Improve diagnostic for const ctors in array repeat expressions #113925

clubby789 · 2023-07-21T16:02:03Z

rustbot · 2023-07-21T16:02:08Z

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

compiler/rustc_middle/src/traits/mod.rs

estebank · 2023-07-24T11:56:14Z

compiler/rustc_middle/src/traits/mod.rs

+/// Whether a value can be extracted into a const.
+/// Used for diagnostics around array repeat expressions.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)]
+pub enum IsConstable {


compiler/rustc_hir_typeck/src/expr.rs

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

clubby789 · 2023-07-24T17:20:45Z

Updated to be a structured suggestion, but I'm not sure about the placement of it (the line before the span of the expression being repeated), since the line before could be part of some other expression. It probably doesn't matter oto much since this is a HasPlaceholders suggestion

estebank

Oh wow! Thank you for attempting to turn this into a structured suggestion in this same PR, was expecting it to be addressed as follow up work :)

I think we can make this very robust, by changing RepeatElementCopy to carry more info.

compiler/rustc_hir_typeck/src/expr.rs

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

clubby789 · 2023-07-24T19:35:25Z

While working on this, it looks like we get weird highlighting when the span we're suggesting gets moved to a newline. Putting the const after works okay but results in weirder looking code

estebank · 2023-07-24T19:45:11Z

The highlighting is likely a bug in the suggestion calculation. What you could do instead is use span_extend_to_line (and check that there's no code before it (like if we had the span for bar() in foo(); bar();) and insert the whole line to side step the issue (but ideally we should fix the underlying problem, fixing the coloring, at some point).

In any case, try to get the ascii output itself to look ok and rustfix suggestions to apply neatly, and we can deal with the coloring separately.

Edit: could it be that there's a tab or some other whitespace hidden in there? I could imagine inconsistent handling of \t in this part of the snippet machinery.

clubby789 · 2023-07-24T20:02:44Z

Ah, looks like it's a bug with indentation. Fixing the indentation makes the highlighting work properly

clubby789 · 2023-07-24T20:07:35Z

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+                    err.multipart_suggestion(help_msg, vec![
+                        (elt_stmt_span.shrink_to_lo(), format!("const ARRAY_REPEAT_VALUE: {elt_type} = {snip};\n{indentation}")),
+                        (elt_span, "ARRAY_REPEAT_VALUE".to_string())
+                    ], Applicability::MachineApplicable);


Made this MachineApplicable because ARRAY_REPEAT_VALUE is unlikely to conflict unless there's multiple instances of this in the same scope, and I'm reasonably sure the HIR walking should give us a span it's safe to put a new item on. This can be changed to MaybeIncorrect if there's any weird counterexamples

estebank · 2023-07-25T17:23:17Z

tests/ui/consts/const-fn-in-vec.rs

@@ -1,7 +1,11 @@
+static _MAYBE_STRINGS: [Option<String>; 5] = [None; 5];


If we annotate this file with // run-rustfix, does the test continue to pass? If so, let's add that to protect against involuntary regressions in the suggestions. (You might have to allow some lints, potentially.)

Ah unfortunately no, because the const uses the same name which conflicts. It might be a good idea to maybe make the identifier more unique (do we have a way of 'identifier-izing' an arbritrary string?). Something like Enum::Variant to ENUM_VARIANT might help

estebank · 2023-07-25T17:25:13Z

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+                {
+                    let help_msg = format!(
+                        "consider creating a new `const` item and initializing it with {value_kind} \
+                        to be used in the repeat position");


Suggested change

to be used in the repeat position");

to be used in the repeat position"

);

estebank

r=me after addressing the nitpicks.

estebank · 2023-07-25T17:26:10Z

r? @estebank

@bors r+

bors · 2023-07-25T17:26:12Z

📌 Commit 86b1122 has been approved by estebank

It is now in the queue for this repository.

clubby789 · 2023-07-25T17:33:54Z

I'd like to address the nitpicks, having a rustfix example seems like a good idea
@bors r-

Dylan-DPC · 2023-11-04T17:34:08Z

@clubby789 any updates on this?

estebank · 2023-11-07T19:57:17Z

@Dylan-DPC lets see if we can land as is and @clubby789 can open a follow up PR at their leisure.

@bors r+

bors · 2023-11-07T19:57:20Z

📌 Commit 86b1122 has been approved by estebank

It is now in the queue for this repository.

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#113925 (Improve diagnostic for const ctors in array repeat expressions) - rust-lang#116399 (Small changes w/ `query::Erase<_>`) - rust-lang#117625 (Fix some clippy perf lints) - rust-lang#117655 (Method suggestion code tweaks) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#113925 - clubby789:const-ctor-repeat, r=estebank Improve diagnostic for const ctors in array repeat expressions Fixes rust-lang#113912

rustbot assigned eholk Jul 21, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 21, 2023

compiler-errors reviewed Jul 21, 2023

View reviewed changes

compiler/rustc_middle/src/traits/mod.rs Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the const-ctor-repeat branch from 7127f5c to 22ce932 Compare July 21, 2023 23:55

estebank reviewed Jul 24, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved

estebank reviewed Jul 24, 2023

View reviewed changes

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved

estebank approved these changes Jul 24, 2023

View reviewed changes

rust-cloud-vms bot force-pushed the const-ctor-repeat branch from 22ce932 to 8dccd6a Compare July 24, 2023 17:19

estebank reviewed Jul 24, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the const-ctor-repeat branch from 8dccd6a to 7857b27 Compare July 24, 2023 20:06

clubby789 commented Jul 24, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Improve diagnostic for const ctors in array repeat expressions

86b1122

rust-cloud-vms bot force-pushed the const-ctor-repeat branch from 7857b27 to 86b1122 Compare July 24, 2023 20:31

estebank reviewed Jul 25, 2023

View reviewed changes

estebank approved these changes Jul 25, 2023

View reviewed changes

rustbot assigned estebank and unassigned eholk Jul 25, 2023

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 Jul 25, 2023

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 25, 2023

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 7, 2023

matthiaskrgr mentioned this pull request Nov 7, 2023

Rollup of 4 pull requests #117692

Merged

bors merged commit b724d9c into rust-lang:master Nov 8, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Nov 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve diagnostic for const ctors in array repeat expressions #113925

Improve diagnostic for const ctors in array repeat expressions #113925

clubby789 commented Jul 21, 2023

rustbot commented Jul 21, 2023

estebank Jul 24, 2023

clubby789 commented Jul 24, 2023

estebank left a comment

clubby789 commented Jul 24, 2023

estebank commented Jul 24, 2023 •

edited

clubby789 commented Jul 24, 2023

clubby789 Jul 24, 2023

This comment has been minimized.

estebank Jul 25, 2023

clubby789 Jul 25, 2023

estebank Jul 25, 2023

estebank left a comment

estebank commented Jul 25, 2023

bors commented Jul 25, 2023

clubby789 commented Jul 25, 2023

Dylan-DPC commented Nov 4, 2023

estebank commented Nov 7, 2023

bors commented Nov 7, 2023

		@@ -1,7 +1,11 @@
		static _MAYBE_STRINGS: [Option<String>; 5] = [None; 5];

	to be used in the repeat position");
	to be used in the repeat position"
	);

Improve diagnostic for const ctors in array repeat expressions #113925

Improve diagnostic for const ctors in array repeat expressions #113925

Conversation

clubby789 commented Jul 21, 2023

rustbot commented Jul 21, 2023

estebank Jul 24, 2023

Choose a reason for hiding this comment

clubby789 commented Jul 24, 2023

estebank left a comment

Choose a reason for hiding this comment

clubby789 commented Jul 24, 2023

estebank commented Jul 24, 2023 • edited

clubby789 commented Jul 24, 2023

clubby789 Jul 24, 2023

Choose a reason for hiding this comment

This comment has been minimized.

estebank Jul 25, 2023

Choose a reason for hiding this comment

clubby789 Jul 25, 2023

Choose a reason for hiding this comment

estebank Jul 25, 2023

Choose a reason for hiding this comment

estebank left a comment

Choose a reason for hiding this comment

estebank commented Jul 25, 2023

bors commented Jul 25, 2023

clubby789 commented Jul 25, 2023

Dylan-DPC commented Nov 4, 2023

estebank commented Nov 7, 2023

bors commented Nov 7, 2023

estebank commented Jul 24, 2023 •

edited