very minor cleanups #111606

jyn514 · 2023-05-15T16:44:21Z

add must_use to early_error_no_abort

this was already being used at its only callsite, but this ensures that new code remembers to use it if it's called in the future. found this while investigating stable: Give a better error message for rustc +nightly -Zunstable-options #110090.

remove outdated and incorrect comment in builder.rs. doc_rust_lang_org_channel doesn't exist in rustdoc, it gets it from an env var instead:

rust/src/librustdoc/clean/utils.rs

Lines 569 to 573 in b275d2c

    
           /// A link to `doc.rust-lang.org` that includes the channel name. Use this instead of manual links 
        
           /// so that the channel is consistent. 
        
           /// 
        
           /// Set by `bootstrap::Builder::doc_rust_lang_org_channel` in order to keep tests passing on beta/stable. 
        
           pub(crate) const DOC_RUST_LANG_ORG_CHANNEL: &str = env!("DOC_RUST_LANG_ORG_CHANNEL");

rustbot · 2023-05-15T16:44:27Z

r? @lcnr

(rustbot has picked a reviewer for you, use r? to override)

jyn514 · 2023-05-15T17:33:47Z

compiler/rustc_driver_impl/src/lib.rs

+                // the error code is already going to be reported when the panic unwinds up the stack
+                let _ = early_error_no_abort(ErrorOutputType::default(), &msg);


I tested this does the right thing as follows:

C:\Users\jyn\src\rust2>(rustc -C help || call echo %%^^errorlevel%% 1>&2) | cmd /c exit error: failed printing to stdout: The pipe is being closed. (os error 232) %101%

lcnr · 2023-05-16T07:07:43Z

compiler/rustc_session/src/session.rs

@@ -1741,6 +1741,7 @@ fn early_error_handler(output: config::ErrorOutputType) -> rustc_errors::Handler

 #[allow(rustc::untranslatable_diagnostic)]
 #[allow(rustc::diagnostic_outside_of_impl)]
+#[must_use = "ErrorGuaranteed does nothing unless returned from build_session_options"]


that feels a bit weird to me 🤔 what do you mean with "does nothing unless"?

ErrorGuaranteed never does anything, I thought it is only meant to be used as a token for "we already errored, so feel free to skip some other errors or checks".

ErrorGuaranteed is used in rustc_driver to determine whether to exit with a success status code or not: https://github.com/rust-lang/rust/blob/666d1831c5a4215165f37417bda6667f27ef7790/compiler/rustc_driver_impl/src/lib.rs#L1386-L1399
If someone forgets to return ErrorGuaranteed from run_compiler, we'll exit with a 0 status code even though we emitted an error.

hmm, this feels generally a bit sketchy, I see why you'd want to use #[must_use] now.

could anything that have an TyCtxt instead check for has_errors()? or I guess sess.has_errors()?

ErrorGuaranteed is used in rustc_driver to determine whether to exit with a success status code or not:

can you put that as part of the #[must_use] explanation?

r=me after that

could anything that have an TyCtxt instead check for has_errors()? or I guess sess.has_errors()?

all the callers of early_error and early_error_no_abort happen before we construct a sess (otherwise they could just call sess.err or sess.fatal).

can you put that as part of the #[must_use] explanation?

👍 will do :)

bors · 2023-05-18T11:01:27Z

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

- add `must_use` to `early_error_no_abort` this was already being used at its only callsite, but this ensures that new code remembers to use it if it's called in the future. - remove outdated and incorrect comment in `builder.rs`. `doc_rust_lang_org_channel` doesn't exist in rustdoc, it gets it from an env var instead.

jyn514 · 2023-05-18T13:07:46Z

@bors r=lcnr rollup

bors · 2023-05-18T13:07:48Z

📌 Commit 0426562 has been approved by lcnr

It is now in the queue for this repository.

very minor cleanups - add `must_use` to `early_error_no_abort` this was already being used at its only callsite, but this ensures that new code remembers to use it if it's called in the future. found this while investigating rust-lang#110090. - remove outdated and incorrect comment in `builder.rs`. `doc_rust_lang_org_channel` doesn't exist in rustdoc, it gets it from an env var instead: https://github.com/rust-lang/rust/blob/b275d2c30b6e88cc48747f349f7137076d450658/src/librustdoc/clean/utils.rs#L569-L573

Rollup of 10 pull requests Successful merges: - rust-lang#111491 (Dont check `must_use` on nested `impl Future` from fn) - rust-lang#111606 (very minor cleanups) - rust-lang#111619 (Add timings for MIR passes to profiling report) - rust-lang#111652 (Better diagnostic for `use Self::..`) - rust-lang#111665 (Add more tests for the offset_of macro) - rust-lang#111708 (Give a more useful location for where a span_bug was delayed) - rust-lang#111715 (Fix doc comment for `ConstParamTy` derive) - rust-lang#111723 (style: do not overwrite obligations) - rust-lang#111743 (Improve cgu merging debug output) - rust-lang#111762 (fix: emit error when fragment is `MethodReceiverExpr` and items is empty) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned lcnr May 15, 2023

This comment has been minimized.

Sign in to view

jyn514 force-pushed the nightly-diagnostics branch from 4a1f8c1 to 666d183 Compare May 15, 2023 17:32

jyn514 commented May 15, 2023

View reviewed changes

lcnr reviewed May 16, 2023

View reviewed changes

jyn514 force-pushed the nightly-diagnostics branch from 666d183 to 464fd0c Compare May 18, 2023 13:04

jyn514 force-pushed the nightly-diagnostics branch from 464fd0c to 0426562 Compare May 18, 2023 13:07

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 May 18, 2023

Dylan-DPC mentioned this pull request May 18, 2023

Rollup of 6 pull requests #111732

Closed

matthiaskrgr mentioned this pull request May 19, 2023

Rollup of 8 pull requests #111750

Closed

GuillaumeGomez mentioned this pull request May 19, 2023

Rollup of 10 pull requests #111760

Closed

GuillaumeGomez mentioned this pull request May 19, 2023

Rollup of 10 pull requests #111767

Closed

Dylan-DPC mentioned this pull request May 20, 2023

Rollup of 10 pull requests #111778

Merged

bors merged commit fa11c9e into rust-lang:master May 20, 2023

rustbot added this to the 1.71.0 milestone May 20, 2023

jyn514 deleted the nightly-diagnostics branch June 16, 2023 01:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

very minor cleanups #111606

very minor cleanups #111606

jyn514 commented May 15, 2023 •

edited

Loading

rustbot commented May 15, 2023

This comment has been minimized.

jyn514 May 15, 2023

lcnr May 16, 2023

jyn514 May 18, 2023

lcnr May 18, 2023

lcnr May 18, 2023 •

edited

Loading

jyn514 May 18, 2023 •

edited

Loading

bors commented May 18, 2023

jyn514 commented May 18, 2023

bors commented May 18, 2023

	/// A link to `doc.rust-lang.org` that includes the channel name. Use this instead of manual links
	/// so that the channel is consistent.
	///
	/// Set by `bootstrap::Builder::doc_rust_lang_org_channel` in order to keep tests passing on beta/stable.
	pub(crate) const DOC_RUST_LANG_ORG_CHANNEL: &str = env!("DOC_RUST_LANG_ORG_CHANNEL");

		// the error code is already going to be reported when the panic unwinds up the stack
		let _ = early_error_no_abort(ErrorOutputType::default(), &msg);

very minor cleanups #111606

very minor cleanups #111606

Conversation

jyn514 commented May 15, 2023 • edited Loading

rustbot commented May 15, 2023

This comment has been minimized.

jyn514 May 15, 2023

Choose a reason for hiding this comment

lcnr May 16, 2023

Choose a reason for hiding this comment

jyn514 May 18, 2023

Choose a reason for hiding this comment

lcnr May 18, 2023

Choose a reason for hiding this comment

lcnr May 18, 2023 • edited Loading

Choose a reason for hiding this comment

jyn514 May 18, 2023 • edited Loading

Choose a reason for hiding this comment

bors commented May 18, 2023

jyn514 commented May 18, 2023

bors commented May 18, 2023

jyn514 commented May 15, 2023 •

edited

Loading

lcnr May 18, 2023 •

edited

Loading

jyn514 May 18, 2023 •

edited

Loading