Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"help" messages in compiler output is ignored #21195

Closed
oli-obk opened this issue Jan 15, 2015 · 9 comments
Closed

"help" messages in compiler output is ignored #21195

oli-obk opened this issue Jan 15, 2015 · 9 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Jan 15, 2015

issue-11714.rs
    Line 14:     ; //~ HELP consider removing this semicolon:
issue-13428.rs
    Line 18:     ;   //~ HELP consider removing this semicolon
    Line 23:     ;   //~ HELP consider removing this semicolon
issue-2354.rs
    Line 11: fn foo() { //~ HELP did you mean to close this delimiter?
liveness-return-last-stmt-semi.rs
    Line 20:     x * 2; //~ HELP consider removing this semicolon
parenthesized-box-expr-message.rs
    Line 12:     box(1 + 1) //~ HELP perhaps you meant `box() (foo)` instead?

Removing these will still pass make check.

@kmcallister kmcallister added A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Jan 16, 2015
@kmcallister
Copy link
Contributor

Perhaps compiletest should yell loudly if there are any unparsed //~ comments?

@oli-obk oli-obk changed the title //~ HELP comments are ignored in unit tests. "help" messages in compiler output is ignored Jan 19, 2015
@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 19, 2015

kmcallister: this already happens, it's the other direction that is not checked

@ftxqxd
Copy link
Contributor

ftxqxd commented Jan 20, 2015

I think this is intentional. It’d be painful to have to annotate every single help/note message in every test that omits them, and it’s very rarely useful to check that a help/note message is not omitted (which is all this would do).

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 20, 2015

almost.
if you add a help message, other help messages should not be ignored anymore, since you are explicitly checking for help messages

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2016

(pointed here from #30778); it seems like a fine idea to me

@fhahn
Copy link
Contributor

fhahn commented Jan 10, 2016

I also think it makes sense to have a way to make tests fail if there are unexpected note/help messages (e.g. making sure a new note/help messages does not show up in unexpected places). I guess we currently do not fail on unexpected help/note messages for historical reasons (as far as I remember, earlier versions of Rust did not have help/note messages). However failing on unexpected help/note messages as default now would require adding a lot of additional annotations.

So providing a way to opt in seems reasonable to me.

@nikomatsakis
Copy link
Contributor

@fhahn opt-in seems good, for the reasons you cite. I've also rarely actually wanted a failure in those cases in the tests I write, at least.

@alexcrichton
Copy link
Member

I personally like the idea that if any help/note are specified then they must be exhaustively specified, and otherwise they're entirely optional.

Seems like a good way to ensure that the diagnostics are kept in check and they don't accidentally blow up to 1k help messages by accident.

@nikomatsakis
Copy link
Contributor

In discussion in the compiler meeting we agreed that "match all help/note if we match any help/note" is a reasonable heuristic and we should just do that.

If we ever want to test that there are NO note/help, we can always add something like #[rustc_error] (or modify #[rustc_error]) to emit a "fake" NOTE.

fhahn added a commit to fhahn/rust that referenced this issue Jan 15, 2016
bors added a commit that referenced this issue Jan 30, 2016
This is a PR for #21195. It changes the way unspecified `help` and `ǹote` messages are handled in compile-fail tests as suggested by @oli-obk in the issue: if there are some `note` or `help` annotations, there must be annotations for all `help` or `note` messages of this test. Maybe it makes also sense to add an option to specify that the this test should fail if there are unspecified `help` or `note` messages.

With this change, the following tests fail:

    [compile-fail] compile-fail/changing-crates.rs
    [compile-fail] compile-fail/default_ty_param_conflict_cross_crate.rs
    [compile-fail] compile-fail/lifetime-inference-give-expl-lifetime-param.rs
    [compile-fail] compile-fail/privacy1.rs
    [compile-fail] compile-fail/svh-change-lit.rs
    [compile-fail] compile-fail/svh-change-significant-cfg.rs
    [compile-fail] compile-fail/svh-change-trait-bound.rs
    [compile-fail] compile-fail/svh-change-type-arg.rs
    [compile-fail] compile-fail/svh-change-type-ret.rs
    [compile-fail] compile-fail/svh-change-type-static.rs
    [compile-fail] compile-fail/svh-use-trait.rs

I'll add the missing annotations if we decide to accept this change.
@bors bors closed this as completed in 36656f8 Jan 30, 2016
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 15, 2016
The opt-in functionality was proposed and discussed in
rust-lang#21195
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2016
Original issue: rust-lang#21195

Relevant PR: rust-lang#30778

Prior to this commit, if a compiletest testcase included the text
"HELP:" or "NOTE:" (note the colons), then it would indicate to the
compiletest suite that we should verify "help" and "note" expected
messages.

This commit updates this check to also check "HELP" and "NOTE" (not the
absense of colons) so that we always verify "help" and "note" expected
messages.
bors added a commit that referenced this issue Mar 17, 2016
…atsakis

Stop ignoring expected note/help messages in compiletest suite.

Original issue: #21195

Relevant PR: #30778

Prior to this commit, if a compiletest testcase included the text
"HELP:" or "NOTE:" (note the colons), then it would indicate to the
compiletest suite that we should verify "help" and "note" expected
messages.

This commit updates this check to also check "HELP" and "NOTE" (not the
absense of colons) so that we always verify "help" and "note" expected
messages.
eddyb added a commit to eddyb/rust that referenced this issue Mar 19, 2016
…ote, r=nikomatsakis

Add comment about opt-in nature of compiletest note/help messages.

The opt-in functionality was proposed and discussed in
rust-lang#21195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants