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

"Expected separator" on hex literal #866

Closed
Nehliin opened this issue May 15, 2021 · 6 comments
Closed

"Expected separator" on hex literal #866

Nehliin opened this issue May 15, 2021 · 6 comments
Labels
area: front-end Input formats for conversion help wanted Extra attention is needed kind: bug Something isn't working lang: WGSL WebGPU shading language

Comments

@Nehliin
Copy link

Nehliin commented May 15, 2021

Hello again, I ran into another issue with my glsl -> wgsl conversion. Wgsl doesn't seem to accept any of the following:
let mask: u8 = 0xFFu;
let mask: u8 = 0xFF;

Both gives the following error even though the values matches the regex in the specification:

  thread 'main' panicked at 'Failed to parse shader: error: expected separator, found 'xFFu'
     ┌─ wgsl:37:21
     │
  37 │     let mask: u8 = 0xFFu;
     │                     ^^^^ expected separator
@kvark kvark added the kind: question Further information is requested label May 18, 2021
@kvark
Copy link
Member

kvark commented May 18, 2021

The error could have been more clear... WGSL doesn't know what u8 is.
What Naga version are you using? Could you try the latest master?

@Nehliin
Copy link
Author

Nehliin commented May 19, 2021

I don't think u8 is the issue here, I got the same error when using u32. I currently run whatever naga version that's used within wgpu 8.1. I've tested with latest naga master now and get the same issue when testing with u32 instead of u8.

@kvark
Copy link
Member

kvark commented May 20, 2021

oh it doesn't expect the u suffix for hex literals, I see

@Nehliin
Copy link
Author

Nehliin commented May 21, 2021

Even without the u suffix I get the same error :/
Failed to parse WGSL code: expected separator, found 'xFF'

@kvark kvark added area: front-end Input formats for conversion help wanted Extra attention is needed kind: bug Something isn't working lang: WGSL WebGPU shading language and removed kind: question Further information is requested labels May 21, 2021
@hcsch
Copy link
Contributor

hcsch commented Aug 10, 2021

I also stumbled across this across this wanting a hexadecimal u32 constant.

My code

let test: u32 = 0x0u;

gets me the following error

error: the type of `test` is expected to be [1]
  ┌─ wgsl:1:5
  │
1 │ let test: u32 = 0x0u;
  │     ^^^^ definition of `test`

Could not parse WGSL

I suppose similar to the above issues, 0 is consumed as a whole number token (i.e. x0u is not included).
Due to the missing u suffix this way, the number literal is interpreted as an i32 literal.

Here are some test cases for all integer and float literals that match the spec (and their current errors for those that fail):

// Regexes for the literals are taken from the working draft at
// https://www.w3.org/TR/2021/WD-WGSL-20210806/#literals

// matches for /^(-?[0-9]*\.[0-9]+|-?[0-9]+\.[0-9]*)((e|E)(\+|-)?[0-9]+)?$/
let dec_float_lit_0 : f32 = -1.;
let dec_float_lit_1 : f32 = -.1;
let dec_float_lit_2 : f32 = 42.1234;
let dec_float_lit_3 : f32 = -1.E3;
let dec_float_lit_4 : f32 = -.1e-5;
let dec_float_lit_5 : f32 = 2.3e+55; // fails "expected floating-point literal, found `2.3e`"

// matches for /^-?0x([0-9a-fA-F]*\.?[0-9a-fA-F]+|[0-9a-fA-F]+\.[0-9a-fA-F]*)(p|P)(\+|-)?[0-9]+$/
let hex_float_lit_0 : f32 = -0xa.p1; // fails "the type of `hex_float_lit_0` is expected to be [1]"
let hex_float_lit_1 : f32 = -0x.fp9; // fails "the type of `hex_float_lit_1` is expected to be [1]"
let hex_float_lit_2 : f32 = 0x2a.4D2P4; // fails "the type of `hex_float_lit_2` is expected to be [1]"
let hex_float_lit_3 : f32 = -0x.1p-5; // fails "the type of `hex_float_lit_3` is expected to be [1]"
let hex_float_lit_4 : f32 = 0xC.8p+55; // fails "the type of `hex_float_lit_4` is expected to be [1]"
let hex_float_lit_5 : f32 = 0x1p1; // fails "the type of `hex_float_lit_5` is expected to be [1]"

// matches for /^-?0x[0-9a-fA-F]+|0|-?[1-9][0-9]*$/
let int_lit_0 : i32 = -0x0; // fails "expected ';', found 'x0'"
let int_lit_1 : i32 = 0;
let int_lit_2 : i32 = 0x2a4D2; // fails "expected ';', found 'x2a4D2'"
let int_lit_3 : i32 = 1092;
let int_lit_4 : i32 = -9923;

// matches for /^0x[0-9a-fA-F]+u|0u|[1-9][0-9]*u$/
let uint_lit_0 : u32 = 0x0u; // fails "the type of `uint_lit_0` is expected to be [1]"
let uint_lit_1 : u32 = 0u;
let uint_lit_2 : u32 = 0x2a4D2u; // fails "the type of `uint_lit_2` is expected to be [1]"
let uint_lit_3 : u32 = 1092u;

Also note that the current draft of the specification does not allow for the f suffix in floating point literals.
The current lexer erroneously accepts the following code:

let dec_float_lit_6 : f32 = 1.337e-42f;

The relevant piece of code seems to be this:

fn consume_number(input: &str) -> (Token, &str) {
//Note: I wish this function was simpler and faster...
let mut is_first_char = true;
let mut right_after_exponent = false;
let mut what = |c| {
if is_first_char {
is_first_char = false;
c == '-' || ('0'..='9').contains(&c) || c == '.'
} else if c == 'e' || c == 'E' {
right_after_exponent = true;
true
} else if right_after_exponent {
right_after_exponent = false;
('0'..='9').contains(&c) || c == '-'
} else {
('0'..='9').contains(&c) || c == '.'
}
};
let pos = input.find(|c| !what(c)).unwrap_or_else(|| input.len());
let (value, rest) = input.split_at(pos);
let mut rest_iter = rest.chars();
let ty = rest_iter.next().unwrap_or(' ');
match ty {
'u' | 'i' | 'f' => {
let width_end = rest_iter
.position(|c| !('0'..='9').contains(&c))
.unwrap_or_else(|| rest.len() - 1);
let (width, rest) = rest[1..].split_at(width_end);
(Token::Number { value, ty, width }, rest)
}
// default to `i32` or `f32`
_ => (
Token::Number {
value,
ty: if value.contains('.') { 'f' } else { 'i' },
width: "",
},
rest,
),
}
}

I imagine it will be rather hard to write a compliant lexer for these literals formats with the current approach.
Perhaps a parser combinator approach, the use of the specified regexes or something similar might make this easier.

@jimblandy
Copy link
Member

At present, this validates fine:

fn f() {
    let mask: u32 = 0xFFu;
    let mask: i32 = 0xFF;
}

The original program, with u32 substituted for u8, fails with a helpful error:

fn f() {
    let mask: u32 = 0xFFu;
    let mask: u32 = 0xFF;
}

Naga correctly identifies the type mismatch between u32 and 0xFF:

[2022-02-24T00:01:23Z ERROR naga::front::wgsl] Given type Scalar { kind: Uint, width: 4 } doesn't match expected Scalar { kind: Sint, width: 4 }
error: the type of `mask` is expected to be `i32`
  ┌─ 866.wgsl:3:9
  │
3 │     let mask: u32 = 0xFF;
  │         ^^^^ definition of `mask`

So I think this is fixed. If not, feel free to re-open this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: front-end Input formats for conversion help wanted Extra attention is needed kind: bug Something isn't working lang: WGSL WebGPU shading language
Projects
None yet
Development

No branches or pull requests

4 participants