futhark icon indicating copy to clipboard operation
futhark copied to clipboard

Add noIdentNormalize compilation flag

Open jon-edward opened this issue 1 year ago • 1 comments

Using the compilation flag noidentnormalize, this allows the user to deny identifier normalization to the final output file.

Although this makes no difference to compilation, using this option will keep the output identifier as-is when the source name is already a valid Nim identifier. This makes editor code completion much nicer when using the generated bindings if you're expecting to use a consistent case convention.

Added test and updated README to demonstrate the purpose of the flag.

jon-edward avatar Jun 29 '24 03:06 jon-edward

I was going through active issues and came across #87, better sanitizename. I think this solution will suffer from the same issues as the previously proposed solution, I’ll keep working to allow for hashing.

jon-edward avatar Jun 29 '24 08:06 jon-edward

Worked off of the tnormalize.nim test, and it seems to be working.

/.../futhark/src/futhark.nim(246, 5) Hint: Renaming "myVar" to "myVarconst"
/.../futhark/src/futhark.nim(246, 5) Hint: Renaming "myvar" to "myvarconstC690172C"
/.../futhark/src/futhark.nim(246, 5) Hint: Renaming "MY_VAR" to "MY_VARconst"
/.../futhark/src/futhark.nim(246, 5) Hint: Renaming "MyVar" to "MyVarconst2E4AA817"
/.../futhark/src/futhark.nim(246, 5) Hint: Renaming "My_Var" to "My_Varconst038D4D97"

#67 made reference to a "pretty name" table which was my first thought, also. But (if my assumption is correct) it seems like you can instead add the normalized name to the hashset that's tracking name collisions while returning the case sensitive result from sanitizeName.

Is there any internal reason why sanitizeName's result has to be in usedNames that I'm missing?

jon-edward avatar Jun 29 '24 17:06 jon-edward

Did some minor modifications, and removed the switch. Futhark will now never try to normalize identifiers, only using normalization to be able to do the table lookups. The reason is because sanitizeName also checks usedNames to see whether or not the name is already used and if so pick a more specific name. This is part of the anti-collision strategy. These checks used to be spread out in the code a bit more, but now it's all contained in sanitizeName and a lot easier to reason about.

PMunch avatar Jun 30 '24 19:06 PMunch

Thanks for the merge! I like what you did with double underscore replacement.

jon-edward avatar Jun 30 '24 19:06 jon-edward