-
-
Notifications
You must be signed in to change notification settings - Fork 45
fix: Wgsl keywords among identifiers #1607
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
|
pkg.pr.new packages benchmark commit |
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.
I wonder if there isn't a more straightforward way to handle the prefixing. RandomNameRegistry already will handle the names since the suffix _{someIdx} should always be enough. And for StrictNameRegistry we could just initialize the _usedNames with the keywords:
export class StrictNameRegistry extends NameRegistryImpl {
/**
* Allows to provide a good fallback for instances of the
* same function that are bound to different slot values.
*/
private readonly _usedNames: Set<string>;
constructor() {
super();
this._usedNames = new Set<string>(keywordsAndReservedTokens);
}
...
}I applied this change and changed makeValid to:
makeValid(primer: string): string {
if (
keywordsAndReservedTokens.has(primer)
) {
return `${this.makeUnique(primer)}`;
}
return primer;
}And it seems to work? What do you think? Did I miss something? This does not handle the _ (but I would argue if someone tries to name his variable this the runtime error is deserved) and __ prefix case but that could be handled inside the primer validation.
Co-authored-by: Iwo Plaza <iwoplaza@gmail.com>
reczkok
left a comment
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.
Looks great 🚀🏗️
Co-authored-by: Konrad Reczko <66403540+reczkok@users.noreply.github.com>
|
@iwoplaza can you take a look at this? I don't know whether |
Changes:
WGSL has a set of words that cannot appear as variable identifiers. Furthermore, "_" and identifiers starting with "__" are invalid as well, those are difficult to rename so we just throw in that case. Otherwise,
makeValidjust callsmakeUniquewhen needed.Identifiers not detected (because they are valid variable names):
u32,vec4f,mat2x2f,int,texture_2detc.,storage,handleetc.,textureSample,sin,dpdxetc.