-
Notifications
You must be signed in to change notification settings - Fork 353
Make tokens for << >> >>> #792
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
Conversation
For example, disallow
a < < b
from being recognized as a left-shift.
|
In a larger sense, the spec doesn't talk about whitespace and tokenization. |
|
Tokens should never span white space, and tokens should never be for white space. That is, parsing (grammar time) should not take space into account, only a token stream that has already been normalized WRT space and not include space-based tokens. There has been some conflation of lexing and parsing pointed out in other issues. E.g., this came up with the flawed idea of having |
wgsl/index.bs
Outdated
| <tr><td>`NOT_EQUAL`<td>!== | ||
| <tr><td>`GREATER_THAN`<td>> | ||
| <tr><td>`GREATER_THAN_EQUAL`<td>>= | ||
| <tr><td>`GREATER_GREATER`<td>>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to make these names consistent. I.e. why ARROW and not MINUS_GREATER? Similarly, there is ATTR_LEFT and not BRACKET_LEFT_LEFT.
If GREATER_GREATER is in fact the desired level at which the naming is done, then this would be something to follow-up. I wanted to raise it here in case GREATER_GREATER should be replaced by something like ARITHMETIC_SHIFT_RIGHT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, a more semantic name would be a better choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a1f03bd
|
I'm actually no longer sure this is the right fix. Compare with "old" C++ templates where you had to insert a space between nested templates ends to avoid confusing the tokenizer. E.g. |
|
oh wait: |
It was fixed in C++11. You shouldn't get that error with |
|
But, C++11 is using a more complex parsing scheme than simpler languages, and we shouldn't burden WGSL with that complexity. Not to mention there is a pre-processor in the middle of the tokenization for C++. That's also why I originally raised the white-space issues in my review: that's all covered by the pre-processing rules, but WGSL doesn't have a pre-processor, leaving a vacuum to fill regarding white space. |
|
100% agreed we don't want those parsing complexities. |
| <tr><td>`GREATER_THAN_EQUAL`<td>>= | ||
| <tr><td>`LOGICAL_SHIFT_RIGHT`<td>>> | ||
| <tr><td>`ARITH_SHIFT_RIGHT`<td>>>> | ||
| <tr><td>`LESS_THAN`<td>< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to consider call this ANGLE_BRACKET_LEFT and ANGLE_BRACKET_RIGHT to separate terminology between lexing and parsing.
That would be more consistent with BRACE_LEFT instead of BEGIN_SCOPE, BANG instead of NOT, BRACKET_LEFT instead of BEING_ARRAY_INDEX, etc.
|
Coming back to this to implement in Tint and it actually makes other bits of parsing more complicated. With this change, anywhere there is a GREATER_THAN (say This is doable, but is something other implementations will have to guard against as well. |
|
I'm sure (and from some discussion above) you don't want this complexity in a simple portable language. I think you'll find more corner cases, and different implementations might find different ones. |
|
There are 3 options I see. You either accept this complexity, allow '> > >' to mean '>>>' or change how the parser/lexer works so the parser asks for specific tokens from the lexer instead of what's next. Having the grammar as written in this CL is probably the right thing as '> > >' meaning '>>>' would be weird. But, implementers need to make sure they deal with the side effects. |
Make tokens for << >> >>>
For example, disallow
a < < b
from being recognized as a left-shift.
Make tokens for << >> >>>
For example, disallow
a < < b
from being recognized as a left-shift.
This implements the F32Interval class and fundamental accuracy functions described in my prototype, gpuweb/cts#1402, for reworking how floating point tests are written. This also adds in testing that did not appear in my prototype. There are a couple of functions that are not currently used that I have included in this patch that have suppressions for unused-vars on them. I included them in this patch since they are logically grouped with this code. These surpressions will be removed in future patches. Review discovered a bug in oneULP's implementation, which is fixed in this PR. Issue gpuweb#792
Also includes a fix for f64, which was pushing inputs through a Float32Array. This has been corrected to use a Float64Array. Issue gpuweb#792
Allows passing in an F32Interval instead of an number[], so that cases can bypass error calculation, instead of complicating error calculations further. Issue gpuweb#792
Also remove usage of this infra by integer tests that shouldn't have been using it to begin with. Fixes gpuweb#792
For example, disallow
from being recognized as a left-shift.