Resolve issue #22187 (ICE compiling libcore with RUST_LOG=debug). #22376
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @huonw (who looked at this earlier) |
|
Needs a rebase |
|
FYI - rebased. |
| // pointer is valid. Furthermore, we know that the `ArcInner` structure itself is `Sync` | ||
| // because the inner data is `Sync` as well, so we're ok loaning out an immutable pointer | ||
| // to these contents. | ||
| // This unsafety is ok because while this arc is alive we're guaranteed |
There was a problem hiding this comment.
As much as I prefer 80 character line comments to 100 character ones, I object to people touching files solely for such changes. Please remove.
|
@aidancully sorry for the noise at the start; I was misled by the diff and thought you were making very broad changes on your own. Please rebase with just the one commit; if you want assistance you can ping me on irc. (And if you also change the occurrences of "{:?}" to "{}", I'll r+) |
|
@pnkfelix: sorry for taking so long to address this, real life has interfered for some time. I don't want to let this drop, though, and will try to find time to address your concerns.
|
|
@pnkfelix, I believe I've addressed the concerns? Please let me know if this version of the patch won't work. |
|
☔ The latest upstream changes (presumably #23955) made this pull request unmergeable. Please resolve the merge conflicts. |
aidancully
force-pushed
the
pr22187
branch
2 times, most recently
from
ed8637e to
3afa481
Compare
|
I've re-rebased, so this should be merge-able again. |
|
@bors r+ |
|
📌 Commit 3afa481 has been approved by |
|
⌛ Testing commit 3afa481 with merge d29518c... |
|
💔 Test failed - auto-mac-64-nopt-t |
| // defined in the local crate. Returns Option<_> because this function | ||
| // can be called while the trait_defs map is being built, so that | ||
| // looking up the trait_def in the local crate can sometimes fail. | ||
| fn try_lookup_trait_def<'tcx>(tcx: &ctxt<'tcx>, did: ast::DefId) |
There was a problem hiding this comment.
Ugh I totally overlooked the fact that you seem to have defined the same function twice here.
There was a problem hiding this comment.
sorry, that was probably a rebase problem, i'll fix and update.
…bug). The issue is that the Repr logic assumes that any trait_defs to be represented will be non-local. This will not be the case when compiling libcore: the compiler has a default predicate on many types that they will be Sized, and the Sized trait is defined in libcore. If we allow that a Repr'ed trait_def must come from a foreign crate, then, because the Repr logic can be used while the trait_defs table is being built, there will be a chance that a trait_def lookup will fail on Repr. Handle both of these issues by defining a new try_lookup_trait_def for use by the Repr logic that will attempt to look up a trait_def in the local crate, and returns success/failure in an Option<>. Callers are then responsible for making sure the trait_def is actually present in defining the representation.
|
@aidancully did you pull in an older version? It seems like the |
|
@pnkfelix: |
|
@aidancully (just transcribing from my suggestion on IRC) : I'd use format!("local_trait_not_ready({})", trait_ref.def_id.local_id()) |
|
r? @pnkfelix |
|
Thanks for the help, I couldn't reproduce this issue in current Rust, I think due to a6c295c. Since the patch seems to have been obsoleted, I'm closing the PR. |
The issue is that the Repr logic assumes that any trait_defs to
be represented will be non-local. This will not be the case when
compiling libcore: the compiler has a default predicate on many
types that they will be Sized, and the Sized trait is defined
in libcore.
If we allow that a Repr'ed trait_def must come from a foreign
crate, then, because the Repr logic can be used while the
trait_defs table is being built, there will be a chance that
a trait_def lookup will fail on Repr.
Handle both of these issues by defining a new try_lookup_trait_def
for use by the Repr logic that will attempt to look up a trait_def
in the local crate, and returns success/failure in an Option<>.
Callers are then responsible for making sure the trait_def is
actually present in defining the representation.
r? huonw