reorder config.toml.example options and add one missing option

[guanqun]

r? @Mark-Simulacrum

[guanqun]

This is based on the discussion in this issue: #65352

[guanqun]

So I just removed the documentation part, internally the flag is still there.

[Mark-Simulacrum]

Perfect, thanks!

@bors r+ rollup=always

cc @rust-lang/compiler on this change but I believe it should be uncontroversial so approving -- this is just hiding the optimize flag from the documentation since it is an "advanced" feature and we don't expect 99.9% of contributors to change it, or indeed really anyone.

[bors]

📌 Commit fb5789f0b23ffde6608f69817130230958d45a5c has been approved by Mark-Simulacrum

[Zoxc]

I personally consider everything in config.toml advanced features and I'd like to keep it.

[pnkfelix]

I am tempted to r- the PR because I personally am in @Zoxc's camp that everything config.toml is "advanced features", and I don't think people should have to go grepping through the rust-build infrastructure to discover options like this ...

... the main reason why I have balked at r-'ing the PR is that I am not aware of anyone who has gotten utility from setting optimize = false... so, is having this big block of text in the config.toml buying us anything ...?

The one thing that I think this big block of text does buy us is this: Clarity as to what are the effects of debug = true.

If you apply this PR, some people might infer (incorrectly) that setting debug = true will disable optiimizations of the stdlib and compiler.

[pnkfelix]

@bors r-

Lets at least touch on this in the Thursday compiler team meeting.

(that is: nominating for discussion at compiler team meeting.)

[Mark-Simulacrum]

Sounds good :)

To be clear I don't personally have much opinion here; I think I agree as to ambiguity of debug. I mainly wanted to land this to avoid further bug reports about optimize = false not working on some platforms, which I closed in the belief that said mode was not supported. One option is to just say "the option exists but we really only support = true" and leave it in.

But I'm happy to discuss at the meeting.

[Zoxc]

I use debug builds sometimes to get accurate stack traces. It's a bit slow, but using a parallel compiler helps a lot.

-Z share-generics is required for debug builds on Windows to not hit PE limitations. We should probably apply that for Windows (or for all platforms?).

[Mark-Simulacrum]

I use debug builds sometimes to get accurate stack traces. It's a bit slow, but using a parallel compiler helps a lot.

When you say "a bit slow" -- prior issues/PRs have noted >10x slowdown which I would imagine makes bootstrapping impossible; does a parallel compiler actually help a lot here?

[Zoxc]

Usually you just need to build libcore or libstd with the debug compiler to debug it.

The parallel compiler helps much more than normally since LLVM is still as fast and the overhead of atomics is less since everything else is slower.

[guanqun]

Thank you @Zoxc for the inputs, I'm fine not to merge this change, then we probably need to re-open this ticket #65352 because if it's exposed, we probably should make it work on Windows platform as well.

[nikomatsakis]

My two cents:

I do not think we should have options that are not commented. I consider config.toml.example the "canonical documentation" for all options available and I would be quite surprised that there are things missing from there.

However, I think we can just add a stronger note on optimize = true to say something like:

# WARNING: Building with optimize = false is NOT SUPPORTED. Due to bootstrapping,
# building without optimizations takes much longer than optimizing. Further, some platforms
# (notably Windows) fail to build without this optimization (c.f. #65352).

[guanqun]

I can try to figure out the missing options.

for the optimize flag, I can simply copy niko's sentences above if everyone is okay with it.

[nikomatsakis]

@guanqun works for me, obviously =)

(to be clear, I also think we can close #65352 if we have no intention to fix it)

[guanqun]

oh, I do spot one missing option, it's the musl-root in the [rust] section. It's added in the commit.

[Mark-Simulacrum]

Could you split the commit into 3:

• reordering the options
• editing the optimize flag text
• adding musl-root

This'll make the commit history a bit better.

Other than that I'm personally probably fine with this, although we might instead prefer to edit the source code to match the ordering in config.toml.example, since that seems to be the "source of truth" more so than the code IMO.

[guanqun]

I thought for a few seconds which would be the "source of truth" as you mentioned above, :) I lean towards the code, but it seems Mark is on the config.toml.example side.

I'll wait for one more day before I update the commit. So my plan would be:

1. recorder the fields based on the config.toml.exmaple's order (according to @Mark-Simulacrum 's suggestion above)
2. split commits into separate functionalities (add the musl-root and modify the optimize comments)

[nikomatsakis]

I mean the code is obviously the real source of truth, but I think people are more likely to peruse this file (certainly I did). It's length also strongly suggests it is exhaustive.

I don't really care if we make the code match the toml or vice versa -- but having them be the same makes sense.

[Mark-Simulacrum]

I mean the code is obviously the real source of truth, but I think people are more likely to peruse this file (certainly I did). It's length also strongly suggests it is exhaustive.

I think this is sort of not true -- in the sense that the code today is just haphazardly ordered whereas config.toml.example has had at least some thought put into it.

[nikomatsakis]

Removing the S-waiting-on-team tag since I think we've decided.

[guanqun]

I've split the commits into three categories:

1. add a warning to rust.optimize option
2. add the missing rust.musl_root option
3. reorders the fields (I'm following the orders in config.toml.example)

@Mark-Simulacrum @nikomatsakis Could you please help review again?

[Mark-Simulacrum]

@bors r+ rollup=always

Thanks for bearing with the process so far!

I'm going to approve this as I believe we're in a "this changes almost nothing" state -- and we had consensus I believe on the warning being added.

[bors]

📌 Commit 5defe06 has been approved by Mark-Simulacrum