Rollup of 3 pull requests#159827
Closed
jhpratt wants to merge 7 commits into
Closed
Conversation
also add a test for cross-crate static initializers
The string `#[target_features]` is used in various error messages as well as the compiler's code. But there is no such attribute, which I found very confusing. I think it means "one or more target features", e.g. covering both `#[target_feature(foo)]` and `#[target_feature(foo, bar, baz)]`. We already use `#[target_feature(..)]` for that meaning in several places. So this commit changes all `#[target_features]` occurrences to `#[target_feature(..)]`. It also changes a few `#[target_feature]` (no plural) occurrences to `#[target_feature(..)]` for consistency with other mentions nearby in error messages.
…ls, r=bjorn3 reuse regular exported_non_generic_symbols logic in Miri This is some gnarly code we have duplicated in Miri, let's try to reuse the version from rustc. r? @bjorn3
…=BoxyUwU when bailing on ambiguity, don't force other results to ambig A smaller version of rust-lang#149904 which doesn't cause the perf issue in bevy, or at least in the minimization While not necessarily necessary for rust-lang/trait-system-refactor-initiative#257 due to rust-lang#158643. This does fix the underlying issues there. This change does affect `binius_field`, the remaining issues are tracked in rust-lang/trait-system-refactor-initiative#274. The core idea is that for - A - B - A (cycle with initial result) - B <--- We normally run A until we reach a fixpoint. We don't do so in case trying to do so encounters overflow or the result of `A` ends up ambiguous to avoid performance issues. We want to generally keep the provisional cache entries which depend on A around. Evaluating these goals depends on the current provisional result of A. This means it is fine to keep them around if A reached a fixpoint. If we bail without reaching a fixpoint, the provisional result used while computing B differs from the final result of A. This means the entry for B is not valid and we'd need to discard it. Discarding all nested provisional cache entries when not reaching a fixpoint causes perf issues. In case the final result of A is ambiguous, then this PR keeps nested cache entries which are ambiguous and don't have any inference constraints around. That is sound, as having more goals be ambiguous is never an issue, the trait solver can always say ":shrug: idk". It also shouldn't cause any incorrect ambiguity errors issues, as changing the provisional result of A to be ambiguity should not change B to go from being ambiguous to something else. The current implementation mutated the result of all nested provisional cache entries to be ambiguous, which resulted in incorrect ambiguity errors in rust-lang/trait-system-refactor-initiative#257. This PR differs from that by dropping goals whose result isn't already ambiguous. That's means we don't keep quite as many cache entries around, which is potentially worse for perf, but from what I can tell this doens't cause issues in practice. r? BoxyUwU
…ssages, r=oli-obk Avoid `#[target_features]` The string `#[target_features]` is used in various error messages as well as the compiler's code. But there is no such attribute, which I found very confusing. I think it means "one or more target features", e.g. covering both `#[target_feature(foo)]` and `#[target_feature(foo, bar, baz)]`. We already use `#[target_feature(..)]` for that meaning in several places. So this commit changes all `#[target_features]` occurrences to `#[target_feature(..)]`. It also changes a few `#[target_feature]` (no plural) occurrences to `#[target_feature(..)]` for consistency with other mentions nearby in error messages. r? @oli-obk
Member
Author
|
@bors r+ rollup=never p=5 |
Contributor
This comment has been minimized.
This comment has been minimized.
Contributor
|
💔 Test for d2c9471 failed: CI. Failed job:
|
Contributor
|
This pull request was unapproved due to being closed. |
Collaborator
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
This file contains hidden or 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
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:
#[target_features]#159809 (Avoid#[target_features])r? @ghost
Create a similar rollup