Introduce a span for module contents #13791
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -227,10 +227,25 @@ impl Clean<Item> for doctree::Module { | |||
| self.view_items.clean().move_iter().collect(), | |||
| self.macros.clean().move_iter().collect() | |||
| ); | |||
|
|
|||
| // determine if we should display the inner contents or | |||
| // the outer `mod` item for the source code. | |||
There was a problem hiding this comment.
I think this comment could be expanded to at least mention that this is handling the difference between mod ...; and mod ... {} explicitly.
There was a problem hiding this comment.
I've added additional comments that show that difference.
this is useful when the module item and module contents are defined from different files (like rustdoc). in most cases the original span for the module item would be used; in other cases, the span for module contents is available separately at the `inner` field.
| let cm = ctxt.sess().codemap(); | ||
| let outer = cm.lookup_char_pos(self.where_outer.lo); | ||
| let inner = cm.lookup_char_pos(self.where_inner.lo); | ||
| if outer.file.deref() as *FileMap == inner.file.deref() as *FileMap { |
There was a problem hiding this comment.
Just to double check: this is working out if the inner span is in the same file as the outer span?
Also, maybe this should be comparing the name, or the byte start position? (e.g. outer.file.start_pos == inner.file.start_pos.)
That feels a little nicer than "relying" on referential equality.
…t-lang#12926. the basic strategy is to distinguish `mod foo;` from `mod foo {...}` by checking if the span for the module item and module contents is in different files. if it's the case, we prefer module contents. it is technically possible to fix rust-lang#12926 without changing the AST, probably by checking the individual items' span. this is not without a problem though, since it is possible that some items inside `mod foo {...}` may have originated from other file (e.g. `include!`). therefore it is better to record both spans explicitly.
bors
added a commit
that referenced
this pull request
Apr 28, 2014
This PR is primarily motivated by (and fixes) #12926. We currently only have a span for the individual item itself and not for the referred contents. This normally does not cause a problem since both are located in the same file; it *is* possible that the contained statement or item is located in the other file (the syntax extension can do that), but even in that case the syntax extension should be located in the same file as the item. The module item (i.e. `mod foo;`) is the only exception here, and thus warrants a special treatment. Rustdoc would now distinguish `mod foo;` from `mod foo {...}` by checking if the span for the module item and module contents is in different files. If it's the case, we'd prefer module contents over module item. There are alternative strategies, but as noted above we will have some corner cases if we don't record the contents span explicitly.
arcnmx
pushed a commit
to arcnmx/rust
that referenced
this pull request
Jan 9, 2023
…e-ranges, r=lnicola fix: merge multiple intersecting ranges Fixes rust-lang#13791 In `check_intersection_and_push()`, there may exist two ranges we should merge with the new one. We've been assuming there should be only one range that intersects, which lead to [this assertion](https://github.com/rust-lang/rust-analyzer/blob/da15d92a3204da419bad70cbfceb2676bfe0b528/crates/text-edit/src/lib.rs#L192) to fail under specific circumstances.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is primarily motivated by (and fixes) #12926.
We currently only have a span for the individual item itself and not for the referred contents. This normally does not cause a problem since both are located in the same file; it is possible that the contained statement or item is located in the other file (the syntax extension can do that), but even in that case the syntax extension should be located in the same file as the item. The module item (i.e.
mod foo;) is the only exception here, and thus warrants a special treatment.Rustdoc would now distinguish
mod foo;frommod foo {...}by checking if the span for the module item and module contents is in different files. If it's the case, we'd prefer module contents over module item. There are alternative strategies, but as noted above we will have some corner cases if we don't record the contents span explicitly.