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

C: Use unique names for variables #2142

Closed
wants to merge 5 commits into from

Conversation

Smit-create
Copy link
Collaborator

@Smit-create
Copy link
Collaborator Author

@certik please let me know if this is the correct approach to move further. There are many tests that still fail and it would require fixes to complete this.

@certik
Copy link
Contributor

certik commented Jul 10, 2023

I think so. This seems what I was thinking.

@certik
Copy link
Contributor

certik commented Jul 10, 2023

To help with the tests, let's hide it behind a compiler option, that way the new name would only be used with the option is given, so current tests should pass.

@Smit-create
Copy link
Collaborator Author

To help with the tests

Let's first fix all the tests such that all of them are working and then we can add it behind a compiler option so that it does not break with the compiler flag.

@certik
Copy link
Contributor

certik commented Jul 10, 2023

Sure. Depends what the failures are --- the issue with unique names is that all the C tests (in tests/ directory) then will fail, as the name is changing with every run (as it should!).

@Smit-create
Copy link
Collaborator Author

I have added the fix, but it's kind of hack-ish as we need to keep in mind about unique_id while emitting the symbol name which can fail easily. How about writing a pass if we have --generate-object-code, then it would replace all the symbol_names with unique names. This would also help with any such backend where there would be linking issues like C, C++, etc.

@certik
Copy link
Contributor

certik commented Jul 10, 2023

I think writing the ASR pass will be better, as it will fix this issue for all the backends. Here is what the pass could do:

  • all module level symbols will receive proper name mangling: prepend module name, and optional unique hash (depending on a compiler option).

I think this might be as easy as just modifying the symbol table, since we don't reference things by name, except in ExternalSymbols, so those would have to be updated too.

@Smit-create
Copy link
Collaborator Author

I'll keep this PR open as all the tests pass successfully. I'll work on writing a pass in a new PR tomorrow morning.

@Smit-create
Copy link
Collaborator Author

Completed in #2149

@Smit-create Smit-create deleted the i-2129 branch July 17, 2023 04:48
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