Consider making RUSTC_LOG=warn the default (instead of just error).

[eddyb]

We don't use error!/warn!/info! a lot, but they come in handy for "debugging rustc without a debug-assertions build" situations, e.g. we use info! to show the function being codegen'd (as it's low-overhead and really helpful for e.g. LLVM crashes).

While info! makes sense for off-by-default debugging helpers like that, error!/warn! seem closer to user-facing diagnostics, where the main difference is them being usable outside of code which has access to a Session (either due to running before the Session is created, or just in dependencies that have access to log/tracing but not any rustc-specific diagnostics APIs).

However, one surprising aspect is only error! is enabled by default, while warn! is not, meaning that, with the current defaults, one is forced to choose between error! (which would output the word "error" which sounds more dire than "warning") or warn! (which may never be seen by an user).

---

My usecase for warn! is in the upcoming -Z self-profile support for using hardware performance counters (which needs to be implemented in the separate measureme project), e.g. this warn! instructs the user to open an issue if they have an unrecognized (newer) CPU, but there's nothing preventing -Z self-profile from working (if the newer CPUs aren't drastically different), so showing "error" seems quite inappropriate.

Sadly, I don't think log env_logger itself will change this behavior, so even if we change RUSTC_LOG's default, measureme might still have to use the less-appropriate error! if it wants to account for being used in other projects.

cc @Mark-Simulacrum @gdhuper @hawkw

[eddyb]

Sadly, I don't think log env_logger itself will change this behavior, so even if we change RUSTC_LOG's default, measureme might still have to use the less-appropriate error! if it wants to account for being used in other projects.

This might not as big of a problem as I thought, @m-ou-se suggested checking if warn! is enabled (either by the user, or rustc's default if we change that) and emit error! if it's not (and say "warning" in the message).
For convenience I could wrap this in a really_warn! macro.

[Mark-Simulacrum]

warn! then seems not significantly better than just an eprintln - am I missing something there?

[eddyb]

Perhaps, but I would prefer using log/tracing macros for consistency of output, and more generally the ability for consumers of a library to choose where to direct the messages.

This is even more important with tracing, where there is a nesting structure, not just individual messages.

[oli-obk]

Since we use max_level_info for tracing, error! warn! and info! should all show up in regular builds.

[eddyb]

This has gotten worse with tracing, where even error! isn't enabled by default (see #78781 (comment)).

[eddyb]

@oli-obk I assume you mean this?

rust/src/bootstrap/lib.rs

Lines 688 to 695 in d8d1d10

	// If debug logging is on, then we want the default for tracing:
	// https://github.com/tokio-rs/tracing/blob/3dd5c03d907afdf2c39444a29931833335171554/tracing/src/level_filters.rs#L26
	// which is everything (including debug/trace/etc.)
	// if its unset, if debug_assertions is on, then debug_logging will also be on
	// as well as tracing *ignoring* this feature when debug_assertions is on
	if !self.config.rust_debug_logging {
	features.push_str(" max_level_info");
	}

This is just the old equivalent of disabling debug!/trace! unless you have a local built that opts into it.

IIUC, "max" means the limit of what RUSTC_LOG can access (i.e. on nightly RUSTC_LOG=debug can't enable any debug! because max_level_info optimized those out).

What we want is the default level, which doesn't seem to be anything nowadays, with RUSTC_LOG not set.

[eddyb]

Talking to @hawkw, we're in agreement this is very wrong:

rust/compiler/rustc_driver/src/lib.rs

Lines 1255 to 1261 in 55111d6

	pub fn init_env_logger(env: &str) {
	// Don't register a dispatcher if there's no filter to print anything
	match std::env::var(env) {
	Err(_) => return,
	Ok(s) if s.is_empty() => return,
	Ok(_) => {}
	}

Bailing out means the whole output infrastructure isn't registered, so e.g. error! will do absolutely nothing.

[hawkw]

Since we use max_level_info for tracing, error! warn! and info! should all show up in regular builds.

This is not correct. max_level_info means that info and below will not be statically disabled (compiled out). Because you are also using EnvFilter to filter the spans and events that are enabled, error!, warn!, and info! will only be enabled if the EnvFilter is configured to enable them. What max_level_info means is that anything below INFO will be disabled at compile time, and cannot be enabled by the filter configuration.

[oli-obk]

The bail out was done on purpose out of perf concerns. We can certainly bench it and see, but I would prefer to keep it as is and use the appropriate error/warn diagnostics of rustc instead of tracing statements

[eddyb]

Do you know more details? There aren't that many non-debug!/trace! calls and they mainly exist in places that cannot easily get access to the rustc diagnostics infrastructure.

So the perf impact should be minimal, while offering a valuable infrastructure for miscellanea that has no other way to interface with the user.

For something like measureme, we can probably alleviate it by having a fallback to eprintln!, especially since it may be used without any logging infrastructure.

Or maybe the warnings could be returned by value but that's an easy to misuse API, unlike Result for errors.

[oli-obk]

Hmm... maybe it was just a misunderstanding while we were fixing other perf regressions. I'll open a PR later today to test it out.