Rollup of 5 pull requests #59561
Merged
Rollup of 5 pull requests #59561
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
We were setting the same identifier for both the DW_TAG_structure_type and the DW_TAG_variant_part. This becomes a problem when using thinlto becauses it uses the identifier as a key for a map of types that is used to delete duplicates based on the ODR, so one of them is deleted as a duplicate, resulting in invalid DWARF. The DW_TAG_variant_part isn't a standalone type, so it doesn't need an identifier. Fix by omitting its identifier.
and bump llvm version in test
The existing `KeywordIdents` lint blindly scans the token stream for a macro or macro definition. It does not attempt to parse the input, which means it cannot distinguish between occurrences of `dyn` that are truly instances of it as an identifier (e.g. `let dyn = 3;`) versus occurrences that follow its usage as a contextual keyword (e.g. the type `Box<dyn Trait>`). In an ideal world the lint would parse the token stream in order to distinguish such occurrences; but in general we cannot do this, because a macro_rules definition does not specify what parsing contexts the macro being defined is allowed to be used within. So rather than put a lot of work into attempting to come up with a more precise but still incomplete solution, I am just taking the short cut of not linting any instance of `dyn` under a macro. This prevents `rustfix` from injecting bugs into legal 2015 edition code.
…ed as keyword in test. Back-story: After reflection this morning, I realized that the previous form of this test would allow the macro invocation to treat the `dyn` input as a raw-identifier rather than a keyword, and since the input was discarded by that version of the macro, the test would pass despite the detail that the input `dyn` should not have been parsed as a raw-identifier. This revision fixes that oversight, by actually *using* the macro input to construct a `Box<dyn Trait>` type.
Review feedback asked for the test to be generalized to include macros 2.0; that generalization is dyn-2015-idents-in-decl-macros-unlinted.rs As a drive-by, I also decided to revise the test to make it clear *why* we cannot generally lint these cases. (I already had similar demonstrations in dyn-2015-edition-keyword-ident-lint.rs, but it does not hurt to try to emphasize matters.) I also added some commentary on the cases where we could choose to make the lint smarter, namely the situations where a macro is *definitely* using `dyn` as an identifier (because it is using it as a path component).
miri needs to build std with xargo, which doesn't allow stable/beta: <japaric/xargo#204 (comment)> Therefore, at this time there's no point in making miri available on any but the nightly channel. If we get a stable way to build `std`, like [RFC 2663], then we can re-evaluate whether to start including miri, perhaps still as `miri-preview`. [RFC 2663]: rust-lang/rfcs#2663
…woerister rustc(codegen): uncache `def_symbol_name` prefix from `symbol_name`. The `def_symbol_name` query was an optimization to avoid recomputing the common part of a symbol name, as only the hash needs to be added to it for each symbol. However, rust-lang#57967 will add a new mangling scheme, which doesn't readily support this kind of reuse - while it's plausible, it requires a lot more effort, since you'd have to "symbolically evaluate" mangling, and keep it in a form where the backreference positions can be computed correctly in the final step. So I want to see how much time we're actually saving with this `def_symbol_name` optimization, nowadays. cc @michaelwoerister
…oerister Fix invalid DWARF for enums when using ThinLTO We were setting the same identifier for both the DW_TAG_structure_type and the DW_TAG_variant_part. This becomes a problem when using ThinLTO becauses it uses the identifier as a key for a map of types that is used to delete duplicates based on the ODR, so one of them is deleted as a duplicate, resulting in invalid DWARF. The DW_TAG_variant_part isn't a standalone type, so it doesn't need an identifier. Fix by omitting its identifier. ODR uniquing is [enabled here](https://github.com/rust-lang/rust/blob/f21dee2c6179276321a88a63300dce74ff707e92/src/rustllvm/PassWrapper.cpp#L1101).
…rd-lint-under-macros, r=matthewjasper skip dyn keyword lint under macros This PR is following my own intuition that `rustfix` should never inject bugs into working code (even if that comes at the expense of it failing to fix things that will become bugs). Fix rust-lang#56327
…ursion, r=eddyb Fix infinite recursion Temporary fix for rust-lang#59502. r? @eddyb
manifest: only include miri on the nightly channel miri needs to build std with xargo, which doesn't allow stable/beta: <japaric/xargo#204 (comment)> Therefore, at this time there's no point in making miri available on any but the nightly channel. If we get a stable way to build `std`, like [RFC 2663], then we can re-evaluate whether to start including miri, perhaps still as `miri-preview`. [RFC 2663]: rust-lang/rfcs#2663
|
@bors r+ p=5 |
|
📌 Commit 04ffaca has been approved by |
bors
added
the
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Mar 30, 2019
Rollup of 5 pull requests Successful merges: - #59343 (rustc(codegen): uncache `def_symbol_name` prefix from `symbol_name`.) - #59380 (Fix invalid DWARF for enums when using ThinLTO) - #59463 (skip dyn keyword lint under macros) - #59539 (Fix infinite recursion) - #59544 (manifest: only include miri on the nightly channel) Failed merges: r? @ghost
|
☀️ Test successful - checks-travis, status-appveyor |
This was referenced Mar 30, 2019
Merged
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.
Successful merges:
def_symbol_nameprefix fromsymbol_name. #59343 (rustc(codegen): uncachedef_symbol_nameprefix fromsymbol_name.)Failed merges:
r? @ghost