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

uuid! macro not reporting precise diagnostics #585

Closed
KodrAus opened this issue Feb 11, 2022 · 3 comments · Fixed by #591
Closed

uuid! macro not reporting precise diagnostics #585

KodrAus opened this issue Feb 11, 2022 · 3 comments · Fixed by #591

Comments

@KodrAus
Copy link
Member

KodrAus commented Feb 11, 2022

It looks like we're not getting precise spans in our uuid! macro, because the subspan method isn't considering our indexes to be within the span:

error: invalid character: expected an optional prefix of `urn:uuid:` followed by [0-9a-zA-Z], found `v` at 15
 --> src\main.rs:1:26
  |
1 | const UUID: uuid::Uuid = uuid::uuid!("67e55044-10b1-v26f-9247-bb680e5fe0c8");
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `uuid::uuid` (in Nightly builds, run with -Z macro-backtrace for more info)

This might have changed at some point since we first wrote this code. I'm not actually sure how we're meant to be offsetting those subspans, since we can't tell what the real byte offset of a span is.

I'll do some digging and see if I can find out what's going on there. The proc-macro infrastructure is a bit tricky to trace through.

If we disable the macro-diagnostics feature then we end up with:

error: any use of this value will cause an error
 --> src\main.rs:1:26
  |
1 | const UUID: uuid::Uuid = uuid::uuid!("67e55044-10b1-v26f-9247-bb680e5fe0c8");
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ index out of bounds: the length is 1 but the index is 1
  |
  = note: `#[deny(const_err)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>
  = note: this error originates in the macro `uuid::uuid` (in Nightly builds, run with -Z macro-backtrace for more info)

I think there's nothing we have to do here and the error is only confusing because of the future-compatibility lint template. If you try parse an invalid UUID then we want your compilation to fail.

@Nugine
Copy link
Contributor

Nugine commented Feb 11, 2022

If we bump MSRV to 1.57, we can replace the const_err trick with const panic and provide a more friendly message to tell people what happens.

uuid/src/macros.rs

Lines 19 to 26 in a1fd80c

Err(_) => {
// here triggers const_err
// const_panic requires 1.57
#[allow(unconditional_panic)]
let _ = ["invalid uuid representation"][1];
loop {} // -> never type
}

@KodrAus
Copy link
Member Author

KodrAus commented Feb 11, 2022

I'm very tempted to stabilize on at least 1.56.0 (I've been playing with it in #587 (comment)) so we can bump to the 2021 edition, but at this stage it might be a bit much of a jump for some people.

@KodrAus
Copy link
Member Author

KodrAus commented Feb 28, 2022

I'd like to try figure out the issue with spans before 1.0 if I can so will take a look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants