Skip to content

Conversation

@mejrs
Copy link
Contributor

@mejrs mejrs commented Jan 13, 2026

@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2026

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jan 13, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 13, 2026

☔ The latest upstream changes (presumably #151051) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

if parts == &[sym::rustc_dummy] {
// The arguments of rustc_dummy and diagnostic attributes are not validated
// if the arguments are delimited
if parts == &[sym::rustc_dummy] || parts[0] == sym::diagnostic {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm the special casting of this attribute is a little ugly here. Especially since this will affect all diagnostic attributes, and we're introducing a new one soon (#150935) which should not suffer the mistakes from the past

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we perhaps do a crater run and see how bad making a breaking change is for this? Since these attributes are so niche I expect making the breaking change will be fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which should not suffer the mistakes from the past

There were no mistakes, this is the intended behavior. That is, any syntactically valid input is allowed, similar to attribute macros. I do not want to shut the door on being able to use non-metaitem syntax going forward. For example in the future I'd like to expand these attributes to reference types and traits, and something like From<i32> is not valid metaitem syntax.

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to do this in the future, we would need to add support for parsing types and traits to the attribute parser. As I see it, that is even more an argument for removing this special case now, since this special case skips the attribute parser for this attribute.

I.e. If this attribute skips the attribute parser now, we can not use the attribute parser to parse any arguments in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to do this in the future, we would need to add support for parsing types and traits to the attribute parser.

Yes, we would need to add some kind of support for that. But the semantics of the diagnostic namespace are that any such future extensions are ignored by older compilers and do not raise errors.

In other words: to extend the attribute in the future, we must be maximally permissive now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that makes sense, thanks for explaining!
Could you add a comment to https://doc.rust-lang.org/reference/attributes/diagnostics.html#r-attributes.diagnostic.namespace.unknown-invalid-syntax on this check then?

This still doesn't really solve the usecase for diagnostic attributes that do need to have arguments parsed. Ideally the attribute parser would run, but any errors should be warnings instead. Not sure if we should solve this problem in this PR or when it comes up tho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should solve this problem in this PR or when it comes up tho

This will be an issue with the other diagnostic attributes, so I'll see if I can figure something out.

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed the link

Assuming you're talking about the link I asked for here, I don't see it yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 0ee43b1 (I committed it but forgot to push...)

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Some minor comments :)

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@JonathanBrouwer
Copy link
Contributor

(Jana feel free to steal the assign back, since I think you two might know eachother?)

@mejrs
Copy link
Contributor Author

mejrs commented Jan 13, 2026

I've pushed the link , I'll need to think more about a proper fix for the other diagnostic attributes. But that doesn't matter today.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 13, 2026
@JonathanBrouwer
Copy link
Contributor

Don't expect this to have a significant performance impact, but attr parsing has been surprisingly perf sensitive so let's see
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Jan 14, 2026
Port `diagnostic::do_not_recommend` to new attr parsing
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 14, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 14, 2026

☀️ Try build successful (CI)
Build commit: f1840ed (f1840edf4b3cea3df43ffa5d38ac815208afedae, parent: 4931e09e3ac3182d2a00f38cccfdf68e8e385e1c)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f1840ed): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.5% [0.5%, 0.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.8%, secondary -3.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.8% [-0.8%, -0.8%] 1
Improvements ✅
(secondary)
-3.8% [-3.8%, -3.8%] 1
All ❌✅ (primary) -0.8% [-0.8%, -0.8%] 1

Cycles

Results (primary -2.7%, secondary 3.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.7% [3.7%, 3.7%] 1
Improvements ✅
(primary)
-2.7% [-2.7%, -2.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.7% [-2.7%, -2.7%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 474.176s -> 474.127s (-0.01%)
Artifact size: 383.16 MiB -> 383.18 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 14, 2026
@JonathanBrouwer
Copy link
Contributor

@rustbot author
Left one more comment, r=me after

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2026
@JonathanBrouwer
Copy link
Contributor

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 15, 2026

📌 Commit 0ee43b1 has been approved by JonathanBrouwer

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 15, 2026
@JonathanBrouwer
Copy link
Contributor

@bors rollup=iffy
Because of the minor (though "no action needed") perf effect

@rust-bors

This comment has been minimized.

@rust-bors rust-bors bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 15, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 15, 2026

☀️ Test successful - CI
Approved by: JonathanBrouwer
Pushing 22c74ba to main...

@rust-bors rust-bors bot merged commit 22c74ba into rust-lang:main Jan 15, 2026
12 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Jan 15, 2026
@github-actions
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing b5c2a0f (parent) -> 22c74ba (this PR)

Test differences

Show 450 test diffs

Stage 0

  • errors::verify_passes_abi_invalid_attribute_57: [missing] -> pass (J0)
  • errors::verify_passes_abi_ne_57: pass -> [missing] (J0)
  • errors::verify_passes_attr_application_struct_69: [missing] -> pass (J0)
  • errors::verify_passes_attr_application_struct_70: pass -> [missing] (J0)
  • errors::verify_passes_autodiff_attr_4: pass -> [missing] (J0)
  • errors::verify_passes_both_ffi_const_and_pure_21: pass -> [missing] (J0)
  • errors::verify_passes_deprecated_annotation_has_no_effect_39: [missing] -> pass (J0)
  • errors::verify_passes_doc_inline_only_use_18: pass -> [missing] (J0)
  • errors::verify_passes_doc_keyword_attribute_empty_mod_11: [missing] -> pass (J0)
  • errors::verify_passes_doc_masked_not_extern_crate_self_20: pass -> [missing] (J0)
  • errors::verify_passes_duplicate_diagnostic_item_in_crate_49: pass -> [missing] (J0)
  • errors::verify_passes_duplicate_eii_impls_107: pass -> [missing] (J0)
  • errors::verify_passes_eii_fn_with_track_caller_104: [missing] -> pass (J0)
  • errors::verify_passes_eii_impl_not_function_103: [missing] -> pass (J0)
  • errors::verify_passes_function_not_found_in_trait_109: pass -> [missing] (J0)
  • errors::verify_passes_function_not_have_default_implementation_107: [missing] -> pass (J0)
  • errors::verify_passes_functions_names_duplicated_109: [missing] -> pass (J0)
  • errors::verify_passes_functions_names_duplicated_110: pass -> [missing] (J0)
  • errors::verify_passes_inline_ignored_for_exported_67: pass -> [missing] (J0)
  • errors::verify_passes_lang_item_fn_with_target_feature_46: pass -> [missing] (J0)
  • errors::verify_passes_lang_item_fn_with_track_caller_44: [missing] -> pass (J0)
  • errors::verify_passes_lang_item_on_incorrect_target_46: [missing] -> pass (J0)
  • errors::verify_passes_lang_item_on_incorrect_target_47: pass -> [missing] (J0)
  • errors::verify_passes_layout_homogeneous_aggregate_52: [missing] -> pass (J0)
  • errors::verify_passes_layout_of_54: pass -> [missing] (J0)
  • errors::verify_passes_layout_size_52: pass -> [missing] (J0)
  • errors::verify_passes_may_dangle_36: [missing] -> pass (J0)
  • errors::verify_passes_missing_const_stab_attr_79: pass -> [missing] (J0)
  • errors::verify_passes_missing_panic_handler_41: [missing] -> pass (J0)
  • errors::verify_passes_missing_stability_attr_77: [missing] -> pass (J0)
  • errors::verify_passes_mixed_export_name_and_no_mangle_6: [missing] -> pass (J0)
  • errors::verify_passes_object_lifetime_err_68: pass -> [missing] (J0)
  • errors::verify_passes_repr_conflicting_27: pass -> [missing] (J0)
  • errors::verify_passes_rustc_legacy_const_generics_index_23: [missing] -> pass (J0)
  • errors::verify_passes_rustc_legacy_const_generics_only_23: pass -> [missing] (J0)
  • errors::verify_passes_sanitize_attribute_not_allowed_90: pass -> [missing] (J0)
  • errors::verify_passes_unexportable_adt_with_private_fields_99: pass -> [missing] (J0)
  • errors::verify_passes_unexportable_generic_fn_93: [missing] -> pass (J0)
  • errors::verify_passes_unexportable_priv_item_97: [missing] -> pass (J0)
  • errors::verify_passes_unexportable_type_in_interface_96: [missing] -> pass (J0)
  • errors::verify_passes_unexportable_type_repr_96: pass -> [missing] (J0)
  • errors::verify_passes_unknown_feature_alias_81: pass -> [missing] (J0)
  • errors::verify_passes_unnecessary_stable_feature_86: [missing] -> pass (J0)
  • errors::verify_passes_unstable_attr_for_already_stable_feature_77: pass -> [missing] (J0)
  • errors::verify_passes_unused_multiple_38: [missing] -> pass (J0)
  • errors::verify_passes_unused_multiple_39: pass -> [missing] (J0)
  • errors::verify_passes_useless_assignment_65: [missing] -> pass (J0)
  • transmute::verify_lint_undefined_transmute_144: [missing] -> pass (J0)

Stage 1

  • errors::verify_passes_abi_ne_56: [missing] -> pass (J1)
  • errors::verify_passes_attr_application_enum_68: [missing] -> pass (J1)
  • errors::verify_passes_attr_application_struct_enum_union_71: [missing] -> pass (J1)
  • errors::verify_passes_attr_application_struct_union_70: [missing] -> pass (J1)
  • errors::verify_passes_const_continue_attr_5: [missing] -> pass (J1)
  • errors::verify_passes_custom_mir_incompatible_dialect_and_phase_103: pass -> [missing] (J1)
  • errors::verify_passes_doc_fake_variadic_not_valid_14: pass -> [missing] (J1)
  • errors::verify_passes_doc_keyword_attribute_empty_mod_12: pass -> [missing] (J1)
  • errors::verify_passes_doc_keyword_attribute_not_mod_12: [missing] -> pass (J1)
  • errors::verify_passes_doc_masked_not_extern_crate_self_20: pass -> [missing] (J1)
  • errors::verify_passes_eii_impl_not_function_104: pass -> [missing] (J1)
  • errors::verify_passes_feature_previously_declared_60: [missing] -> pass (J1)
  • errors::verify_passes_feature_stable_twice_60: pass -> [missing] (J1)
  • errors::verify_passes_function_not_found_in_trait_109: pass -> [missing] (J1)
  • errors::verify_passes_incorrect_crate_type_65: pass -> [missing] (J1)
  • errors::verify_passes_incorrect_do_not_recommend_args_3: pass -> [missing] (J1)
  • errors::verify_passes_lang_item_fn_with_track_caller_44: [missing] -> pass (J1)
  • errors::verify_passes_lang_item_fn_with_track_caller_45: pass -> [missing] (J1)
  • errors::verify_passes_layout_size_52: pass -> [missing] (J1)
  • errors::verify_passes_missing_lang_item_44: pass -> [missing] (J1)
  • errors::verify_passes_missing_panic_handler_41: [missing] -> pass (J1)
  • errors::verify_passes_missing_stability_attr_77: [missing] -> pass (J1)
  • errors::verify_passes_multiple_rustc_main_62: pass -> [missing] (J1)
  • errors::verify_passes_non_exhaustive_with_default_field_values_9: pass -> [missing] (J1)
  • errors::verify_passes_non_exported_macro_invalid_attrs_36: pass -> [missing] (J1)
  • errors::verify_passes_object_lifetime_err_68: pass -> [missing] (J1)
  • errors::verify_passes_proc_macro_bad_sig_85: [missing] -> pass (J1)
  • errors::verify_passes_repr_align_greater_than_target_max_28: pass -> [missing] (J1)
  • errors::verify_passes_repr_align_should_be_align_99: [missing] -> pass (J1)
  • errors::verify_passes_repr_align_should_be_align_static_101: pass -> [missing] (J1)
  • errors::verify_passes_repr_conflicting_26: [missing] -> pass (J1)
  • errors::verify_passes_rustc_allow_const_fn_unstable_32: pass -> [missing] (J1)
  • errors::verify_passes_rustc_dirty_clean_26: pass -> [missing] (J1)
  • errors::verify_passes_rustc_force_inline_coro_34: pass -> [missing] (J1)
  • errors::verify_passes_rustc_legacy_const_generics_index_24: pass -> [missing] (J1)
  • errors::verify_passes_rustc_legacy_const_generics_index_exceed_25: pass -> [missing] (J1)
  • errors::verify_passes_rustc_legacy_const_generics_only_22: [missing] -> pass (J1)
  • errors::verify_passes_sanitize_attribute_not_allowed_89: [missing] -> pass (J1)
  • errors::verify_passes_sanitize_attribute_not_allowed_90: pass -> [missing] (J1)
  • errors::verify_passes_transparent_incompatible_73: pass -> [missing] (J1)
  • errors::verify_passes_unexportable_generic_fn_93: [missing] -> pass (J1)
  • errors::verify_passes_unexportable_item_92: [missing] -> pass (J1)
  • errors::verify_passes_unexportable_priv_item_97: [missing] -> pass (J1)
  • errors::verify_passes_unexportable_priv_item_98: pass -> [missing] (J1)
  • errors::verify_passes_unexportable_type_repr_95: [missing] -> pass (J1)
  • errors::verify_passes_unknown_feature_alias_81: pass -> [missing] (J1)
  • errors::verify_passes_unknown_lang_item_47: [missing] -> pass (J1)
  • errors::verify_passes_unnecessary_stable_feature_86: [missing] -> pass (J1)
  • errors::verify_passes_unrecognized_argument_58: [missing] -> pass (J1)
  • errors::verify_passes_unused_multiple_38: [missing] -> pass (J1)
  • lints::verify_lint_incorrect_do_not_recommend_args_143: [missing] -> pass (J1)
  • transmute::verify_lint_undefined_transmute_143: pass -> [missing] (J1)

(and 336 additional test diffs)

Additionally, 14 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 22c74ba91873dd013479f86eac3e9ea10593bff9 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-x86_64-apple: 6531.4s -> 8720.6s (+33.5%)
  2. pr-check-1: 1716.1s -> 2053.6s (+19.7%)
  3. aarch64-apple: 10965.5s -> 12791.3s (+16.7%)
  4. pr-check-2: 2343.8s -> 2706.7s (+15.5%)
  5. dist-aarch64-llvm-mingw: 6973.4s -> 5925.8s (-15.0%)
  6. x86_64-gnu-gcc: 3140.9s -> 3588.6s (+14.3%)
  7. armhf-gnu: 4954.3s -> 5558.5s (+12.2%)
  8. aarch64-gnu-llvm-20-2: 2911.6s -> 3245.5s (+11.5%)
  9. dist-i586-gnu-i586-i686-musl: 5252.9s -> 5785.8s (+10.1%)
  10. x86_64-gnu-tools: 3344.0s -> 3681.6s (+10.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (22c74ba): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.1%, 0.8%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.1% [1.1%, 1.1%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.7% [-1.7%, -1.7%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary -2.5%, secondary -2.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.5% [-2.5%, -2.5%] 1
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) -2.5% [-2.5%, -2.5%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 472.792s -> 471.202s (-0.34%)
Artifact size: 383.56 MiB -> 383.57 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) merged-by-bors This PR was explicitly merged by bors. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants