Skip to content

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Mar 13, 2025

On Unix the limits can be gargantuan anyway so we're pretty unlikely to hit them, but might still exceed it.
We consult ARG_MAX here to get an estimate.

Fixes #138421

r? @jieyouxu

@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 Mar 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 13, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@rust-log-analyzer

This comment has been minimized.

@weihanglo weihanglo force-pushed the argmax branch 2 times, most recently from 9934c6d to b4e66b7 Compare March 13, 2025 03:28
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

You probably need to declare a libc = "0.2" in cg_ssa.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

jieyouxu commented Mar 13, 2025

... Weird. Let me take a look in a bit. Well of course, it's else if cfg!() not #[cfg(unix)] {}, sorry :D

@jieyouxu
Copy link
Member

jieyouxu commented Mar 13, 2025

I would consider restructuring this bit like

#[cfg(windows)]
{
    // but make sure this returns
}

#[cfg(unix)]
{
    // but make sure this returns
}

false

to avoid having both the cfg!(unix) guard and also #[cfg(unix)] and #[cfg(not(unix))] anyway.

@jieyouxu jieyouxu 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 Mar 13, 2025
@arielb1
Copy link
Contributor

arielb1 commented Mar 13, 2025

Helping @weihanglo work on this

@bors
Copy link
Collaborator

bors commented Mar 14, 2025

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

This also updates the estimate on Windows of the length argument
list to `saturating_add` to avoid overflow.
Though I doubt anyone running rustc outside Unix/Windows
On Unix the limits can be gargantuan anyway so we're pretty
unlikely to hit them, but might still exceed it.
We consult ARG_MAX here to get an estimate.
@weihanglo
Copy link
Member Author

@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 Mar 14, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks!

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 15, 2025

📌 Commit 08166b5 has been approved by jieyouxu

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 Mar 15, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2025
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#138283 (Enforce type of const param correctly in MIR typeck)
 - rust-lang#138439 (feat: check ARG_MAX on Unix platforms)
 - rust-lang#138502 (resolve: Avoid some unstable iteration)
 - rust-lang#138514 (Remove fake borrows of refs that are converted into non-refs in `MakeByMoveBody`)
 - rust-lang#138524 (Mark myself as unavailable for reviews temporarily)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 06b135f into rust-lang:master Mar 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2025
Rollup merge of rust-lang#138439 - weihanglo:argmax, r=jieyouxu

feat: check ARG_MAX on Unix platforms

On Unix the limits can be gargantuan anyway so we're pretty unlikely to hit them, but might still exceed it.
We consult ARG_MAX here to get an estimate.

Fixes rust-lang#138421

r? `@jieyouxu`
@weihanglo weihanglo deleted the argmax branch August 21, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

A way to configure always using @argfile for linker invocations
6 participants