Error message for E0307 is weird #26485

[jawline]

Print the error message and then what is expected by the repeat count so the output makes more sense when there is an error in the const expression

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[pnkfelix]

Our standard convention for our error messages is to say "expected XXX, found YYY", but your PR is in this case emitting "found YYY, expected XXX"

I am not 100% clear on what your objection is to the error message as it was originally formed.

• Is it that you find the "expected XXX, found YYY" format itself unclear?
• Or is there something specific about this error case that is making this particular message hard to understand?
  • If so, maybe we can tailor it in some way. E.g. would "expected positive integer, found variable (for repeat count)" be clearer?

Also, did you run make check with your patch in place? I would expect a number of our test cases would need to be updated.

[jawline]

Hi,

Sorry I wasn't sure about the convention.

The issue I have with the print out is that the compiler is printing the details of an error in the found which can lead to strange error messages like

a.rs:8:17: 8:38 error: expected constant integer for repeat count, but unsupported constant expr [E0307]

or any of the possible errors in const_eval.rs ConstEvalError::description

The error text just seems a little bit incorrect grammar wise this way around - but if it goes against the convention perhaps it should just be left this way around.

[jawline]

Haven't run make check - defiantly should. Woops

[pnkfelix]

@jawline maybe simplest to try to get the output to look like "error: expected constant integer for repeat count, but found unsupported constant expr" instead?

alternatively, we could try to just emit "unsupported const expr" alone (without saying anything about "expected constant integer") for just that case, and leave the other cases alone?

[jawline]

Sure - I was trying to avoid logic just for this case, but I think adding the 'found' when the error kind is MiscCatchAll might be best.

[jawline]

Modified so the error now reads

src/lib.rs:20:15: 20:23 help: run rustc --explain E0306 to see a detailed explanation
src/lib.rs:21:14: 21:35 error: expected constant integer for repeat count, but found unsupported constant expr [E0307]
src/lib.rs:21 let q = [5; mem::size_of::()];

only for unsupport constant expr

make check reports
summary of 45 test runs: 9078 passed; 0 failed; 89 ignored; 0 measured

[pnkfelix]

@bors r+ affbc72 rollup

[bors]

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

[jawline]

Not entirely sure why Travis is failing in LLVM - haven't touched it. Builds on my machine

[Gankra]

You accidentally commited submodule changes.

[apasel422]

One of these commits should probably have "closes #26485" in it (they could all be squashed, too).

[pnkfelix]

@bors r+ 26e961f rollup

[Gankra]

@pnkfelix presumably this should be squashed? (more commits than lines changed...)

[pnkfelix]

@gankro yeah you're right; I was trying to avoid spending even more time on this than I already had (with back-and-forth regarding squashing and re-r-plus'ing) but obviously now I've lost more time than I saved.

[pnkfelix]

@bors r-

[pnkfelix]

@jawline please squash.

(r=me after said squashing...)

[jawline]

Hi guys,Sorry I missed the comment about squashing before.

I'm having trouble squashing because of the merge conflict solved after. rebase -p -i doesn't seem to work properly because of it, any idea what I need to do to squash it without screwing up the history in this case?

[apasel422]

@jawline With your original commits, you should rebase onto master, then squash them.

[jawline]

@pnkfelix Hi, Sorry for the delay. Squashed most of the commits and added closes

[apasel422]

@jawline It should be possible to get this down to one commit with no merges.

[pnkfelix]

@jawline yes, please rebase to a single commit as requested by @apasel422 (if you need assistance, feel free to ping me on irc and I can try to help)

[jawline]

@pnkfelix Sorry I was a bit of an idiot and missed those. done

[pnkfelix]

@bors r+ a067b45 rollup