Skip to content

Move doc-comment highlight injection from AST to HIR #8059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 17, 2021

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Mar 16, 2021

Fixes #5016

@lnicola
Copy link
Member

lnicola commented Mar 16, 2021

changelog internal Support cfg_attr in doc-comment syntax highlighting

@Veykril
Copy link
Member Author

Veykril commented Mar 16, 2021

image
Gotta say I didn't expect injection to just work in attributes after fixing up some text ranges

@Veykril Veykril marked this pull request as ready for review March 16, 2021 20:08
match comment.text().find(RUSTDOC_FENCE) {
let mut string;
for attr in attributes.by_key("doc").attrs() {
let src = attr.to_src(&owner);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This to_src call actually has O(n^2) runtime as it recollects all doc items every call then takes the nth item from there, should probably look into a better mapping

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm pretty unsure what would make a good API for attributes. Let me know if you have ideas.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, most of our .source calls are quadratic. The fundamental thing here is that we can't ask for sources of a single thing. Rather, we can ask the parent "what are the sources of all your children?", and there needs to be some caching layer there. impl HasSource for Variant is also quadratic, for example, and that's not really right.

Ideally, this caching should be done transparently by salsa, but this is the bit where we need gc unfortunately.

@jonas-schievink jonas-schievink self-requested a review March 16, 2021 23:12
},
// #[cfg_attr(..., doc = "", ...)]
None => {
// We gotta hunt the string token manually here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also something I'd love to fix with a better attributes API

match comment.text().find(RUSTDOC_FENCE) {
let mut string;
for attr in attributes.by_key("doc").attrs() {
let src = attr.to_src(&owner);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, most of our .source calls are quadratic. The fundamental thing here is that we can't ask for sources of a single thing. Rather, we can ask the parent "what are the sources of all your children?", and there needs to be some caching layer there. impl HasSource for Variant is also quadratic, for example, and that's not really right.

Ideally, this caching should be done transparently by salsa, but this is the bit where we need gc unfortunately.

@Veykril
Copy link
Member Author

Veykril commented Mar 17, 2021

bors r=matklad,jonas-schievink

@bors
Copy link
Contributor

bors bot commented Mar 17, 2021

@bors bors bot merged commit 0fbfab3 into rust-lang:master Mar 17, 2021
@Veykril Veykril deleted the comments branch March 17, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doctest syntax highlighting does not work with #[doc] attributes
4 participants