Migrate rustc_lint lint diagnostics

[Rejyr]

Part 2 of Migrate rustc_lint errors to SessionDiagnostic

r? @davidtwco

TODO

• Refactor some lints manually implementing DecorateLint to use Option<Subdiagnostic>.
• Add #[rustc_lint_diagnostics] to lint functions in context.rs.
• Migrate hidden_unicode_codepoints.rs.
• Migrate UnsafeCode in builtin.rs.
• Migrate the rest of builtin.rs.

[Rejyr]

@rustbot label: +A-diagnostics

[Rejyr]

@davidtwco How would I untangle emit_ffi_unsafe_type_lint and FfiResult? Would I only modify emit_ffi_unsafe_type_lint, create separate LintDiagnostic types for each lint, or just leave it?

[davidtwco]

@davidtwco How would I untangle emit_ffi_unsafe_type_lint and FfiResult? Would I only modify emit_ffi_unsafe_type_lint, create separate LintDiagnostic types for each lint, or just leave it?

I think emit_ffi_unsafe_type_lint/FfiResult might be okay as they are - this lint is using translated diagnostics already, it isn't in a type implementing SessionDiagnostic but that's probably okay for this case.

[Rejyr]

@davidtwco How would I untangle emit_ffi_unsafe_type_lint and FfiResult? Would I only modify emit_ffi_unsafe_type_lint, create separate LintDiagnostic types for each lint, or just leave it?

@davidtwco Same thing for check_panic in non_fmt_panic.rs?

[Rejyr]

@davidtwco How would I migrate usages of struct_lint and struct_lint_level in levels.rs?

[davidtwco]

@davidtwco How would I migrate usages of struct_lint and struct_lint_level in levels.rs?

We have emit_spanned_lint and #[derive(LintDiagnostic)] - I think that covers what you need for struct_lint? struct_lint_level might require a new function which does something similar to emit_spanned_lint - probably just changing its decorate argument to be a impl DecorateLint?

@davidtwco Same thing for check_panic in non_fmt_panic.rs?

I think so, yeah.

[Rejyr]

@davidtwco There's no emit_spanned_lint for LintLevelsBuilder, should I add one? For struct_lint_level, in levels.rs, it only uses LintLevelsBuilder.sess for the Session and LintLevelsBuilder.lint_level(..) or LintLevelsBuilder.provider.get_lint_level(..) for the Level and LintLevelSource, so should I add a method like emit_spanned_lint_level to LintLevelsBuilder?

[davidtwco]

@davidtwco There's no emit_spanned_lint for LintLevelsBuilder, should I add one? For struct_lint_level, in levels.rs, it only uses LintLevelsBuilder.sess for the Session and LintLevelsBuilder.lint_level(..) or LintLevelsBuilder.provider.get_lint_level(..) for the Level and LintLevelSource, so should I add a method like emit_spanned_lint_level to LintLevelsBuilder?

What diagnostic are you trying to migrate where this is necessary?

[davidtwco]

LGTM so far, thanks for your continued work on this.

[Rejyr]

@davidtwco There's no emit_spanned_lint for LintLevelsBuilder, should I add one? For struct_lint_level, in levels.rs, it only uses LintLevelsBuilder.sess for the Session and LintLevelsBuilder.lint_level(..) or LintLevelsBuilder.provider.get_lint_level(..) for the Level and LintLevelSource, so should I add a method like emit_spanned_lint_level to LintLevelsBuilder?

What diagnostic are you trying to migrate where this is necessary?

@davidtwco Mostly the diagnostics in levels.rs. All of them are either struct_lint (one instance) or struct_lint_level (6 instances).
There's a usage of struct_lint in context.rs at line 1013, but it's in the implementation of LintContext for EarlyContext.

[Rejyr]

@davidtwco Also, should I make a new PR for refactoring the diagnostic structs in errors.rs since I've gotten more accustomed to migrating them, or can I add them to this PR?

[davidtwco]

@bors r+

[bors]

📌 Commit 88e5dd2 has been approved by davidtwco

It is now in the queue for this repository.

[bors]

⌛ Testing commit 88e5dd2 with merge c62631d39f8d2ecafc2bf57779be6bc835c116fd...

[bors]

💔 Test failed - checks-actions

[ehuss]

@bors retry

#106682

[bors]

⌛ Testing commit 88e5dd2 with merge bfffe40...

[bors]

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing bfffe40 to master...

[rust-timer]

Finished benchmarking commit (bfffe40): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	1.9%	[1.2%, 2.5%]	2
Improvements ✅ (primary)	-2.9%	[-3.2%, -2.7%]	2
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	-1.7%	[-3.2%, 0.7%]	3

Cycles

This benchmark run did not return any relevant results for this metric.