-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Preallocate DefId
s for lang items and use lang items in lowering
#60607
Comments
I'm going to take a look at this as a side-project :) |
So... one thing I just thought about: We could introduce a third variant to rust/src/librustc/middle/lang_items.rs Line 36 in 9ebf478
We can then just create such paths without actually having to do any work. Code further down will have access to a |
Document + Cleanup lang_items.rs Byproduct of work on rust-lang#60607. r? @oli-obk
Document + Cleanup lang_items.rs Byproduct of work on rust-lang#60607. r? @oli-obk
Document + Cleanup lang_items.rs Byproduct of work on rust-lang#60607. r? @oli-obk
Document + Cleanup lang_items.rs Byproduct of work on rust-lang#60607. r? @oli-obk
Currently, rustc uses a heuristic to determine if a range expression is not a literal based on whether the expression looks like a function call or struct initialization. This fails for range literals whose lower/upper bounds are the results of function calls. A possibly-better heuristic is to check if the expression contains `..`, required in range literals. Of course, this is also not perfect; for example, if the range expression is a struct which includes some text with `..` this will fail, but in general I believe it is a better heuristic. A better alternative altogether is to add the `QPath::LangItem` enum variant suggested in rust-lang#60607. I would be happy to do this as a precursor to this patch if someone is able to provide general suggestions on how usages of `QPath` need to be changed later in the compiler with the `LangItem` variant. Closes rust-lang#73553
Change heuristic for determining range literal Currently, rustc uses a heuristic to determine if a range expression is not a literal based on whether the expression looks like a function call or struct initialization. This fails for range literals whose lower/upper bounds are the results of function calls. A possibly-better heuristic is to check if the expression contains `..`, required in range literals. Of course, this is also not perfect; for example, if the range expression is a struct which includes some text with `..` this will fail, but in general I believe it is a better heuristic. A better alternative altogether is to add the `QPath::LangItem` enum variant suggested in rust-lang#60607. I would be happy to do this as a precursor to this patch if someone is able to provide general suggestions on how usages of `QPath` need to be changed later in the compiler with the `LangItem` variant. Closes rust-lang#73553
see also #61019 I have an old branch for this here for anyone interested: Also see this comment: matthewjasper@a227c70#r40107992 |
I am interested in finishing up the work you started @matthewjasper, if you don't already have plans to do so. |
I don't have plans to finish that work any time soon. |
Change heuristic for determining range literal Currently, rustc uses a heuristic to determine if a range expression is not a literal based on whether the expression looks like a function call or struct initialization. This fails for range literals whose lower/upper bounds are the results of function calls. A possibly-better heuristic is to check if the expression contains `..`, required in range literals. Of course, this is also not perfect; for example, if the range expression is a struct which includes some text with `..` this will fail, but in general I believe it is a better heuristic. A better alternative altogether is to add the `QPath::LangItem` enum variant suggested in rust-lang#60607. I would be happy to do this as a precursor to this patch if someone is able to provide general suggestions on how usages of `QPath` need to be changed later in the compiler with the `LangItem` variant. Closes rust-lang#73553
Change heuristic for determining range literal Currently, rustc uses a heuristic to determine if a range expression is not a literal based on whether the expression looks like a function call or struct initialization. This fails for range literals whose lower/upper bounds are the results of function calls. A possibly-better heuristic is to check if the expression contains `..`, required in range literals. Of course, this is also not perfect; for example, if the range expression is a struct which includes some text with `..` this will fail, but in general I believe it is a better heuristic. A better alternative altogether is to add the `QPath::LangItem` enum variant suggested in rust-lang#60607. I would be happy to do this as a precursor to this patch if someone is able to provide general suggestions on how usages of `QPath` need to be changed later in the compiler with the `LangItem` variant. Closes rust-lang#73553
Opened #75145 to resolve this using @matthewjasper's earlier work. |
…efid-for-lang-items, r=petrochenkov Preallocate `DefId`s for lang items Fixes rust-lang#60607 and fixes rust-lang#61019. This PR introduces `QPath::LangItem` to the HIR and uses it in AST lowering instead of constructing a `hir::Path` from a slice of symbols: - Credit for much of this work goes to @matthewjasper, I basically just [rebased their earlier work](matthewjasper@a227c70#diff-c0f791ead38d2d02916faaad0f56f41d). - Changes to Clippy might not be correct, they compile but attempting to run tests through `./x.py` produced failures which appeared spurious, so I didn't run any clippy tests. - Changes to save analysis might not be correct - tests pass but I don't have a lot of confidence in those changes being correct. - I've used `GenericBounds::LangItemTrait` rather than changing `PolyTraitRef`, as suggested by @matthewjasper [in this comment](matthewjasper@a227c70#r40107992) but I'd prefer that be left for a follow-up. - I've split things into smaller commits fairly arbitrarily to make the diff easier to review, each commit should compile but might not pass tests until the final commit. r? @oli-obk cc @matthewjasper
Right now, our lowering code uses string-paths to call e.g.
Iterator::next
when loweringfor
loops:rust/src/librustc/hir/lowering.rs
Lines 4653 to 4655 in c6ac575
This triggers a name resolution call
rust/src/librustc/hir/lowering.rs
Lines 5263 to 5273 in c6ac575
which will then give us a resolved path, but that's
?
andfor
expansions/lowerings)I think it would be neat if we could instead create the
lang_items
table early in the process and just grab the appropriate item out of that table.cc @Centril @eddyb @petrochenkov
As a first experiment to check out the impact we could create a cache for path resolutions so we only run the path resolution code once and just overwrite all
HirId
s in the generated path with newHirId
s whenever we fetch a cached resolution.The text was updated successfully, but these errors were encountered: