Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Could you

• add similar test cases for some non path-segment keyword, like struct
• add test cases for --cfg on command line and make sure that they behave identically to cfg! and #[cfg].

After that we should be able to run crater on this.
@rustbot author

Reminder, once the PR becomes ready for a review, use @rustbot ready.

add test cases for --cfg on command line and make sure that they behave identically to cfg! and #[cfg]

--cfg will emit same error but will be suppressed because parse_cfg is called with ParseSess::with_fatal_emitter

@rustbot ready

add test cases for --cfg on command line and make sure that they behave identically to cfg! and #[cfg]

--cfg will emit same error but will be suppressed because parse_cfg is called with ParseSess::with_fatal_emitter

with_fatal_emitter doesn't suppress errors, it makes them fatal.
--cfg parsing has its own error reporting logic in fn parse_cfg in compiler\rustc_interface\src\interface.rs, and the new errors are not emitted there.
@rustbot author

with_fatal_emitter doesn't suppress errors, it makes them fatal.

@petrochenkov I found with_fatal_emitter create dcx with FatalOnlyEmitter, see

and the comment of FatalOnlyEmitter and the impl of FatalOnlyEmitter::emit_diagnostic show only errors with diag.level == Level::Fatal can be emitted, see

I have debug the logic in fn parse_cfg, and the err.emit is called in fact, but it is suppressed by the FatalOnlyEmitter.

Ah, ok, "fatal emitter" means "fatal-only emitter".

In any case, the behavior is not correct.
I guess the assumption when using the fatal emitter was that we always reach the "expected key or key="value"" error at the bottom anyway, but that's not actually the case.
If we get to return (ident.name, meta_item.value_str()), then all the non-fatal errors are lost, which is incorrect.

Thanks, this should be ready for crater now, the remaining minor cleanups can be done later.
@bors try

☀️ Try build successful (CI)
Build commit: 4e55a50 (4e55a504843955c97eb06907c71f6f3e99208021, parent: 4068bafedd8ba724e332a5221c06a6fa531a30d2)

@craterbot check

👌 Experiment pr-146978 created and queued.
🤖 Automatically detected try build 4e55a50
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🚧 Experiment pr-146978 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-146978 is completed!
📊 5 regressed and 3 fixed (720593 total)
📊 1730 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Nice, there are only spurious regressions.
I'll write the change description for the lang team later today and pass it to them.

@petrochenkov Cool! Thanks!

Change introducion

This PR makes syntax accepted by cfg predicates in #[cfg] attributes and cfg! macros stricter.
It also makes predicates in --cfg command line options accept the same syntax as in the attributes and macros (except true and false).

#[cfg] and cfg!

Predicates are generally supposed to use non-keyword identifiers (cfg!(ident), cfg!(ident="value")) + boolean literals, and attributes and macros generally followed this grammar, with one exception.
The exceptions is that path segment keywords (super, self, Self, crate, expanded $crate) were accepted.

This happened because internally cfg predicates were parsed as meta-items (besides boolean literals), and then inappropriate meta-items were rejected after obtaining the parsing result.
Path segment keywords are syntactically valid paths, and therefore valid meta-items, so they were successfully parsed but accidentally not rejected by a post-processing check.

This PR rejects them.

--cfg

Predicate syntax in --cfg was parsed largely incorrectly due to majority of parsing errors being suppressed.
Internally --cfg parsing used so called FatalOnlyEmitter overriding the regular error emitter, and that emitter just ignored all recoverable errors.
Initially a different emitter was introduced in "Hide diagnostics emitted during --cfg parsing" to improve diagnostics (reduce noisiness), but the actual effect was also occasionally accepting invalid syntax. "Report fatal lexer errors in --cfg command line arguments" fixed up the situation a bit, but not completely.
Eventually parser improved and improved its recovery, produced more and more recoverable errors, and FatalOnlyEmitter became more and more wrong, leading to the current situation.
In particular, the --cfg true/--cfg false case considered by lang team in #138632 (comment) was an instance of errors swallowed by FatalOnlyEmitter, it is not syntactically valid (true and false are not even meta-items).

This PR disables the error suppression in --cfg parsing and makes it accept the same syntax as its counterparts in #[cfg] and cfg! (except true and false because those are not predicate definitions).

Breakage

Crater run above (#146978 (comment)) didn't find any regressions from this.