fix static_ptr_ty for foreign statics #78101
Merged
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
Even in the PR that introduced this comment, it does not seem like these locals are actually ignored -- just their `source_info` is adjusted: https://github.com/rust-lang/rust/pull/44700/files#diff-ae2f3c7e2f9744f7ef43e96072b10e98d4e3fe74a3a399a3ad8a810fbe56c520R139
rust-highfive
added
the
S-waiting-on-review
|
However, the test suite currently fails with an ICE: Looks like something somewhere is not using |
|
Or maybe it is the other way around -- |
|
I think I found where the wrong type was coming from. |
RalfJung
force-pushed
the
foreign-static
branch
from
ed54d95
to
153e843
Compare
oli-obk
reviewed
Oct 20, 2020
Comment on lines
+155
to
+162
| let static_ty = tcx.type_of(did); | ||
| if tcx.is_mutable_static(did) { | ||
| tcx.mk_mut_ptr(tcx.type_of(did)) | ||
| tcx.mk_mut_ptr(static_ty) | ||
| } else if tcx.is_foreign_item(did) { | ||
| tcx.mk_imm_ptr(static_ty) | ||
| } else { | ||
| tcx.mk_imm_ref(tcx.lifetimes.re_static, tcx.type_of(did)) | ||
| // FIXME: These things don't *really* have 'static lifetime. | ||
| tcx.mk_imm_ref(tcx.lifetimes.re_static, static_ty) |
There was a problem hiding this comment.
This could be deduplicated with the other site by putting it in a function. I see no danger in eagerly normalizing static items' types (so doing it here, too).
There was a problem hiding this comment.
I did that at first, but the lifetime differs. That lead to ICEs.
|
@bors r+ |
|
📌 Commit 153e843 has been approved by |
bors
added
S-waiting-on-bors
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Oct 21, 2020
fix static_ptr_ty for foreign statics Cc rust-lang#74840 This does not fix that issue but fixes a problem in `static_ptr_ty` that we noticed while discussing that issue. I also added and updated a few comments. The one about `internal` locals being ignored does not seem to have been true [even in the commit that introduced it](https://github.com/rust-lang/rust/pull/44700/files#diff-ae2f3c7e2f9744f7ef43e96072b10e98d4e3fe74a3a399a3ad8a810fbe56c520R139). r? @oli-obk
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 21, 2020
Rollup of 7 pull requests Successful merges: - rust-lang#77726 (Add Pin::static_ref, static_mut.) - rust-lang#78002 (Tweak "object unsafe" errors) - rust-lang#78056 (BTreeMap: split off most code of remove and split_off) - rust-lang#78063 (Improve wording of "cannot multiply" type error) - rust-lang#78094 (rustdoc: Show the correct source filename in page titles, without `.html`) - rust-lang#78101 (fix static_ptr_ty for foreign statics) - rust-lang#78118 (Inline const followups) Failed merges: r? `@ghost`
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.
Cc #74840
This does not fix that issue but fixes a problem in
static_ptr_tythat we noticed while discussing that issue. I also added and updated a few comments. The one aboutinternallocals being ignored does not seem to have been true even in the commit that introduced it.r? @oli-obk