search

PMunch / futhark

Automatic wrapping of C headers in Nim
MIT License
393 stars 22 forks source link

Add noIdentNormalize compilation flag #110

Closed jon-edward closed 5 months ago

jon-edward commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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?

PMunch commented 5 months ago

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.

jon-edward commented 5 months ago

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