-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
resolve: Account for new importable entities #59047
Conversation
let module = self.new_module(parent, | ||
ModuleKind::Def(def, ident.name), | ||
def_id, | ||
expansion, | ||
span); | ||
self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); | ||
} | ||
Def::Variant(..) | Def::TyAlias(..) | Def::ForeignTy(..) => { | ||
Def::Variant(..) | Def::TyAlias(..) | Def::ForeignTy(..) | Def::Existential(..) | | ||
Def::TraitAlias(..) | Def::PrimTy(..) | Def::ToolMod => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand but to make sure, could you educate me as to why ToolMod
uses the TypeNS
as opposed to MacroNS
(the documentation on hir::def::Def
barely exists...)?
Otherwise the PR looks good; r=me with green travis :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ToolMod
is very similar to Mod
living in type namespace, it's a container for other items.
The difference is that contained items are limited to tool attributes and nested tool modules + the number of contained items is infinite (i.e. no name checks are performed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah; that makes sense, thanks!
Btw.. @oli-obk suggested we beta-backport fixing the problem? |
Nominating for backport since it makes developing stage 0 less annoying (if you get the ICE you don't get the error that is the actual problem, thus not being able to figure out how to work around the ICE). |
@bors r=Centril |
📌 Commit 967e7f4 has been approved by |
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
@@ -639,24 +639,24 @@ impl<'a> Resolver<'a> { | |||
// but metadata cannot encode gensyms currently, so we create it here. | |||
// This is only a guess, two equivalent idents may incorrectly get different gensyms here. | |||
let ident = ident.gensym_if_underscore(); | |||
let def_id = def.def_id(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part of the diff, I presume isn't meaningful -- that is, we're now getting the def_id
as part of the match, but in each case, it's the same def_id
it was before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the def-ids are the same.
resolve: Account for new importable entities Fixes the ICE encountered in rust-lang#58837 r? @Centril
⌛ Testing commit 47ee538 with merge 7ea8bbe61b4e5c57f9643d296326a9b4cbb34605... |
@bors retry |
⌛ Testing commit 47ee538 with merge 3375b8236553a7aa2d6a9f94486ac5cdea03975a... |
💔 Test failed - checks-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry Seems unrelated to this PR... |
☀️ Test successful - checks-travis, status-appveyor |
…crum Bump bootstrap compiler to 2019-03-20 Includes #59295 and by extension #59047, which unblocks #58253, #58837, and possibly #59336, and so therefore: @bors p=50 r? @Mark-Simulacrum cc @pietroalbini
Fixes the ICE encountered in #58837
r? @Centril