Don't perform invalid checks in codegen_attrs

[inquisitivecrystal]

The attributes #[track_caller] and #[cmse_nonsecure_entry] are only valid on functions. When validating one of these attributes, codegen_attrs previously called fn_sig, which can only be used on functions, on the item the attribute was attached to, assuming that the item was a function without checking. This led to ICEs in situations where the attribute was incorrectly used on non-functions.

With this change, we skip calling fn_sig if the item the attribute is attached to must be a function but isn't, because check_attr will reject such cases without codegen_attrs's intervention.

As a side note, some of the attributes in codegen_attrs are only valid on functions, but that property isn't actually checked. I'm planning to fix that in a follow up PR since it's a behavior change that will need to be validated rather than an obvious bugfix. Thankfully, all the attributes like that I've found so far are unstable.

Fixes #105594.

r? @cjgillot

[bors]

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

[inquisitivecrystal]

There's a bit of a backwards compatibility hazard here, in that #[track_caller] is currently allowed on macro definitions, match arms, and struct fields for legacy reasons (as in, it shouldn't be valid, but we issue a warning rather than an error to avoid breaking old code). This change doesn't seem to be breaking the legacy cases right now, presumably because this function is never called on them. Macros don't need codegen, and struct fields and match arms aren't items. Still, should I handle the legacy case somehow?

[cjgillot]

There's a bit of a backwards compatibility hazard here, in that #[track_caller] is currently allowed on macro definitions, match arms, and struct fields for legacy reasons (as in, it shouldn't be valid, but we issue a warning rather than an error to avoid breaking old code). This change doesn't seem to be breaking the legacy cases right now, presumably because this function is never called on them. Macros don't need codegen, and struct fields and match arms aren't items. Still, should I handle the legacy case somehow?

The assertion def_kind.has_codegen_attrs at the beginning of the query should be enough to avoid triggering these cases.

@bors r+

[bors]

📌 Commit 47b6426 has been approved by cjgillot

It is now in the queue for this repository.

[bors]

⌛ Testing commit 47b6426 with merge c29f2720036ecf11a7b77c6e666da43436e59cf1...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[cjgillot]

@bors retry

[bors]

⌛ Testing commit 47b6426 with merge f206533...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing f206533 to master...

[rust-timer]

Finished benchmarking commit (f206533): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.1%, 2.5%]	2
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-3.1%	[-3.7%, -2.7%]	3
All ❌✅ (primary)	-1.4%	[-1.4%, -1.4%]	1