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

emit error with span for empty asserts #56491

Merged
merged 1 commit into from Dec 11, 2018

Conversation

Projects
None yet
6 participants
@euclio
Contributor

euclio commented Dec 4, 2018

Fixes #55547.

@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Dec 4, 2018

r? @aturon

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

@euclio

This comment has been minimized.

Contributor

euclio commented Dec 4, 2018

@rust-highfive rust-highfive assigned estebank and unassigned aturon Dec 4, 2018

@@ -24,6 +24,12 @@ pub fn expand_assert<'cx>(
tts: &[TokenTree],
) -> Box<dyn MacResult + 'cx> {
let mut parser = cx.new_parser_from_tts(tts);

if parser.token == token::Eof {
cx.span_err(sp, "requires a boolean expression");

This comment has been minimized.

@estebank

estebank Dec 4, 2018

Contributor

Thanks for the change!

Could you change this to something closer to

cx.struct_span_err(sp, "macro call requires boolean expression as argument")
    .span_label(sp, "boolean expression required")
    .emit();

Ideally you'd add a Applicability::MaybeIncorrect suggestion as well showing what the correct code would be, but that'd be overkill for this.

This comment has been minimized.

@euclio

euclio Dec 4, 2018

Contributor

Fixed.

@euclio euclio force-pushed the euclio:assert-error branch from 01eaf4d to a367cec Dec 4, 2018

--> $DIR/assert.rs:2:5
|
LL | assert!(); //~ ERROR requires a boolean expression
| ^^^^^^^^^^ boolean expression required

This comment has been minimized.

@estebank

estebank Dec 10, 2018

Contributor

It'd be nice to point at the (empty) argument list, but this is already quite an improvement.

@estebank

This comment has been minimized.

Contributor

estebank commented Dec 10, 2018

@bors r+ rollup

@bors

This comment has been minimized.

Contributor

bors commented Dec 10, 2018

📌 Commit a367cec has been approved by estebank

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 10, 2018

Rollup merge of rust-lang#56491 - euclio:assert-error, r=estebank
emit error with span for empty asserts

Fixes rust-lang#55547.

bors added a commit that referenced this pull request Dec 10, 2018

Auto merge of #56688 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 5 pull requests

Successful merges:

 - #56491 (emit error with span for empty asserts)
 - #56633 (Fix right arrow size for crate filter)
 - #56641 (fix span for invalid number of parameters in trait method)
 - #56656 (Fix typo)
 - #56661 (Add regression test for ICE)

Failed merges:

r? @ghost

@bors bors merged commit a367cec into rust-lang:master Dec 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@euclio euclio deleted the euclio:assert-error branch Dec 11, 2018

euclio added a commit to euclio/rust that referenced this pull request Dec 16, 2018

remove usage of `panictry!` from libsyntax_ext
This commit completely removes usage of the `panictry!` macro from
libsyntax_ext. The macro causes parse errors to be fatal, so removing it
allows compilation to collect more errors in parsing before bailing out.

Furthermore, this commit adds spans to diagnostics emitted by empty
extensions if they were missing, à la rust-lang#56491.

euclio added a commit to euclio/rust that referenced this pull request Dec 16, 2018

make `panictry!` private to libsyntax
This commit completely removes usage of the `panictry!` macro from
outside libsyntax. The macro causes parse errors to be fatal, so using
it in libsyntax_ext caused parse failures *within* a syntax extension to
be fatal, which is probably not intended.

Furthermore, this commit adds spans to diagnostics emitted by empty
extensions if they were missing, à la rust-lang#56491.

euclio added a commit to euclio/rust that referenced this pull request Dec 17, 2018

make `panictry!` private to libsyntax
This commit completely removes usage of the `panictry!` macro from
outside libsyntax. The macro causes parse errors to be fatal, so using
it in libsyntax_ext caused parse failures *within* a syntax extension to
be fatal, which is probably not intended.

Furthermore, this commit adds spans to diagnostics emitted by empty
extensions if they were missing, à la rust-lang#56491.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment