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

Add noIdentNormalize compilation flag #110

Merged
merged 7 commits into from
Jun 30, 2024
Merged

Conversation

jon-edward
Copy link
Contributor

@jon-edward jon-edward commented Jun 29, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

jon-edward commented Jun 29, 2024

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 PMunch merged commit 7410c6c into PMunch:master Jun 30, 2024
@PMunch
Copy link
Owner

PMunch commented Jun 30, 2024

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
Copy link
Contributor Author

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

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants