Replace weak only lang items with a custom attribute#159344
Conversation
|
Some changes occurred in compiler/rustc_hir/src/attrs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
|
r? @jackh726 rustbot has assigned @jackh726. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
What is the reason for the name field, can't this be a word attribute like |
Calling the Let's do a perf run without it. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Replace weak only lang items with a custom attribute
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9809522): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -4.3%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis perf run didn't have relevant results for this metric. Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 490.561s -> 491.765s (0.25%) |
|
I've switch to deducing the symbol name in the latest commit. Let's see if it has a perf impact. @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Replace weak only lang items with a custom attribute
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (4c682e3): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 1.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis perf run didn't have relevant results for this metric. Binary sizeResults (primary -0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 492.122s -> 488.922s (-0.65%) |
When I added the
invalid_runtime_symbol_definitionsandsuspicious_runtime_symbol_definitionslints in #155521 I added the concept of "weak only" lang item.It's a
LangItemthat has no implementation, this is useful because thecoresymbols where not directly called, they were inserted by the compiler.This mechanism work well, but T-lang approved the extension to "all extern functions referenced by the standard library", which makes the lang item approach impractical as we needs to update 5 files just to declare them. I'm not sure we can ask library contributor to do the dance for each
extern "C"functions.But the most problematic thing is that many extern functions in
stdcome fromlibcnotstd, and adding lang items there is just no feasible I think.Instead this PR proposed that we introduce a proper mechanism for them by adding a attribute
#[rustc_canonical_symbol = "..."], which is encoded and decoded inrmetalike lang items and diagnostics items.This simplifies the declaration as we only need to put the attribute to be effective
#[rustc_canonical_symbol = "open"]. It also opens up many possibilities (none implemented here) like putting them onusestatements (so we don't need to modifylibc) or having the attribute be placed on a module. The table may also be useful on it's own if we want someday to do it for all crates, not just the standard library.Follow up to #155521 and #158522