Skip to content

compiler: Ease off the accelerator on unsupported_calling_conventions #142353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Jun 11, 2025

This is to give us more time to discuss #142330 without the ecosystem having an anxiety attack. I have withdrawn unsupported_calling_conventions from report-in-deps

I believe we should consider this a simple suspension of the decision in #141435 to start this process, rather than a reversal. That is, we may continue with linting again. But I believe we are about to get a... reasonable amount of feedback just from currently available information and should allow ourselves time to process it.

@rustbot
Copy link
Collaborator

rustbot commented Jun 11, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added 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. labels Jun 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 11, 2025

These commits modify compiler targets.
(See the Target Tier Policy.)

@workingjubilee
Copy link
Member Author

cc @RalfJung

@workingjubilee
Copy link
Member Author

oh I should bless tests, sec...

@@ -95,8 +95,7 @@ impl AbiMap {
return AbiMapping::Invalid;
}

(ExternAbi::Cdecl { .. }, Arch::X86) => CanonAbi::C,
(ExternAbi::Cdecl { .. }, _) => return AbiMapping::Deprecated(CanonAbi::C),
Copy link
Member

Choose a reason for hiding this comment

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

I think this should remain deprecated. Just disabling report_in_deps is enough IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright.

@workingjubilee workingjubilee force-pushed the warn-less-about-cdecl-and-other-abis branch from 8c797db to 8f0c09a Compare June 11, 2025 16:58
@RalfJung
Copy link
Member

Thanks! r=me when CI is green.

@workingjubilee
Copy link
Member Author

@bors r=RalfJung

@bors
Copy link
Collaborator

bors commented Jun 11, 2025

📌 Commit 8f0c09a has been approved by RalfJung

It is now in the queue for this repository.

@bors bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 12, 2025
…cl-and-other-abis, r=RalfJung

compiler: Ease off the accelerator on `unsupported_calling_conventions`

This is to give us more time to discuss rust-lang#142330 without the ecosystem having an anxiety attack. I have withdrawn `unsupported_calling_conventions` from report-in-deps

I believe we should consider this a simple suspension of the decision in rust-lang#141435 to start this process, rather than a reversal. That is, we may continue with linting again. But I believe we are about to get a... reasonable amount of feedback just from currently available information and should allow ourselves time to process it.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 12, 2025
…cl-and-other-abis, r=RalfJung

compiler: Ease off the accelerator on `unsupported_calling_conventions`

This is to give us more time to discuss rust-lang#142330 without the ecosystem having an anxiety attack. I have withdrawn `unsupported_calling_conventions` from report-in-deps

I believe we should consider this a simple suspension of the decision in rust-lang#141435 to start this process, rather than a reversal. That is, we may continue with linting again. But I believe we are about to get a... reasonable amount of feedback just from currently available information and should allow ourselves time to process it.
@fee1-dead fee1-dead assigned RalfJung and unassigned fee1-dead Jun 12, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 12, 2025
…cl-and-other-abis, r=RalfJung

compiler: Ease off the accelerator on `unsupported_calling_conventions`

This is to give us more time to discuss rust-lang#142330 without the ecosystem having an anxiety attack. I have withdrawn `unsupported_calling_conventions` from report-in-deps

I believe we should consider this a simple suspension of the decision in rust-lang#141435 to start this process, rather than a reversal. That is, we may continue with linting again. But I believe we are about to get a... reasonable amount of feedback just from currently available information and should allow ourselves time to process it.
bors added a commit that referenced this pull request Jun 12, 2025
Rollup of 9 pull requests

Successful merges:

 - #138016 (Added `Clone` implementation for `ChunkBy`)
 - #140770 (add `extern "custom"` functions)
 - #141162 (refactor  `AttributeGate` and `rustc_attr!` to emit notes during feature checking)
 - #141474 (Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints)
 - #141947 (Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field.)
 - #142135 (docs: autogenerate compiler flag stubs based on -Zhelp)
 - #142252 (Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations)
 - #142337 (miri: add flag to suppress float non-determinism)
 - #142353 (compiler: Ease off the accelerator on `unsupported_calling_conventions`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 12, 2025
…cl-and-other-abis, r=RalfJung

compiler: Ease off the accelerator on `unsupported_calling_conventions`

This is to give us more time to discuss rust-lang#142330 without the ecosystem having an anxiety attack. I have withdrawn `unsupported_calling_conventions` from report-in-deps

I believe we should consider this a simple suspension of the decision in rust-lang#141435 to start this process, rather than a reversal. That is, we may continue with linting again. But I believe we are about to get a... reasonable amount of feedback just from currently available information and should allow ourselves time to process it.
@RalfJung
Copy link
Member

@bors p=1
since people are now seeing these warnings on their nightly builds and the playground

@workingjubilee
Copy link
Member Author

see #142433

@rust-log-analyzer

This comment has been minimized.

@workingjubilee workingjubilee added the CI-spurious-fail-npm we forgot to cache our installs from npm label Jun 12, 2025
@ChrisDenton ChrisDenton reopened this Jun 12, 2025
@rust-log-analyzer

This comment has been minimized.

@ChrisDenton
Copy link
Member

It looks like there is a genuine tidy error:

/checkout/tests/ui/linkage-attr/raw-dylib/windows/unsupported-abi.rs: revision [unspecified] should specify `needs-llvm-components:` as it has `--target` set

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 12, 2025
@ChrisDenton ChrisDenton force-pushed the warn-less-about-cdecl-and-other-abis branch from 5636649 to 5fdca31 Compare June 12, 2025 23:12
@rust-log-analyzer

This comment has been minimized.

@ChrisDenton ChrisDenton force-pushed the warn-less-about-cdecl-and-other-abis branch from 5fdca31 to 97c3860 Compare June 12, 2025 23:17
@ChrisDenton ChrisDenton force-pushed the warn-less-about-cdecl-and-other-abis branch from 97c3860 to 9f50246 Compare June 12, 2025 23:22
@ChrisDenton
Copy link
Member

I just applied the tidy suggestion and blessed. No other changes. We missed today's nightly but I really do want this to make the next one if it's at all possible.

@bors r=ChrisDenton,RalfJung p=2

@bors
Copy link
Collaborator

bors commented Jun 12, 2025

📌 Commit 9f50246 has been approved by ChrisDenton,RalfJung

It is now in the queue for this repository.

@bors bors 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 Jun 12, 2025
@workingjubilee
Copy link
Member Author

ah shit, I always forget the llvm components thing

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

⌛ Testing commit 9f50246 with merge ed44c0e...

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

☀️ Test successful - checks-actions
Approved by: ChrisDenton,RalfJung
Pushing ed44c0e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 13, 2025
@bors bors merged commit ed44c0e into rust-lang:master Jun 13, 2025
11 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 13, 2025
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 bb3a3c5 (parent) -> ed44c0e (this PR)

Test differences

Show 3 test diffs

Stage 1

  • [ui] tests/ui/linkage-attr/raw-dylib/windows/unsupported-abi.rs: ignore (only executed when the operating system is windows) -> pass (J2)

Stage 2

  • [ui] tests/ui/linkage-attr/raw-dylib/windows/unsupported-abi.rs: ignore (only executed when the operating system is windows) -> pass (J0)
  • [ui] tests/ui/linkage-attr/raw-dylib/windows/unsupported-abi.rs: ignore (only executed when the architecture is x86_64) -> pass (J1)

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard ed44c0e3b3a4f90c464361ec6892c1d42c15ea8f --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. x86_64-apple-2: 3906.0s -> 4793.3s (22.7%)
  2. dist-aarch64-apple: 5101.4s -> 6038.3s (18.4%)
  3. dist-apple-various: 5999.0s -> 6953.1s (15.9%)
  4. i686-gnu-2: 5382.7s -> 6191.7s (15.0%)
  5. i686-gnu-nopt-1: 6956.3s -> 8001.6s (15.0%)
  6. x86_64-apple-1: 7365.6s -> 6424.0s (-12.8%)
  7. dist-x86_64-apple: 9724.6s -> 8564.2s (-11.9%)
  8. x86_64-gnu-debug: 5206.0s -> 5806.5s (11.5%)
  9. i686-gnu-1: 7181.9s -> 8002.1s (11.4%)
  10. armhf-gnu: 4471.3s -> 4979.7s (11.4%)
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 (ed44c0e): 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)

Results (secondary 1.4%)

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)
8.5% [8.5%, 8.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.6% [-5.6%, -5.6%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary 2.7%, secondary -2.3%)

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

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

Binary size

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

Bootstrap: 755.435s -> 756.165s (0.10%)
Artifact size: 372.24 MiB -> 372.25 MiB (0.00%)

@workingjubilee workingjubilee deleted the warn-less-about-cdecl-and-other-abis branch June 14, 2025 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) CI-spurious-fail-npm we forgot to cache our installs from npm merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants