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

Reactor classes in the same file with the same name, up to case differences, are prohibited #1741

Merged
merged 11 commits into from
May 18, 2023

Conversation

petervdonovan
Copy link
Collaborator

@petervdonovan petervdonovan commented May 17, 2023

Fixes #1702.

The core issue in #1702 is that we consider reactor classes to have distinct names using a case-sensitive comparison, but we want to generate files corresponding to the reactor classes that will be compared case-insensitively on macOS.

We can't just mangle these file names because users might refer to them in their code.

Therefore, we should avoid creating reactor classes whose names are the same, up to case differences.

This is not really a fix because we still have the problem that we
consider reactor classes to have distinct names using a
**case-sensitive** comparison, but we want to generate files
corresponding to the reactor classes that will be compared
**case-insensitively** on macOS.

However, it does make our generated code "less wrong."
@petervdonovan petervdonovan marked this pull request as draft May 17, 2023 03:13
@petervdonovan petervdonovan marked this pull request as ready for review May 17, 2023 06:33
@cmnrd cmnrd requested review from cmnrd and removed request for cmnrd May 17, 2023 11:16
@petervdonovan petervdonovan marked this pull request as draft May 17, 2023 17:44
The problem here is that we use the uniqueID of the federate both before
and after we transform it into separate programs. After we transform it,
the name of the instance depends on the name of the main reactor, which
might be different (indeed, it must be different in order for us to
eliminate the possibility of name collisions with other parts of the
program). So this is a bit of a hack, but I'm not sure what else to do.
It is bad that in order to make this change it is necessary to edit code
in several places. But it isn't clear how to fix this because different
parts of the code operate on the program at different stages in the
compilation process, either before or after it has been split into many
programs.
This is hacky. The problem is that the generated code does not have a
federated reactor, but we do an AST transformation so that we can
pretend that it is (I do not want to know why). So we end up running the
code generator on a federated program, even though it is supposed to
result on code for a single federate. So if the federate had its name
prefixed once in the main run of the code generator, its name will be
prefixed again in the nested run of the code generator.
@petervdonovan petervdonovan marked this pull request as ready for review May 18, 2023 00:49
@petervdonovan petervdonovan requested a review from edwardalee May 18, 2023 17:21
@petervdonovan
Copy link
Collaborator Author

petervdonovan commented May 18, 2023

I think this is ready for review.
The only test failure is the Zephyr test, which is broken right now, and the Cpp enclave tests, which seem like a separate issue (#1746).

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Would be nice to have a test, but that's not critical.

@petervdonovan
Copy link
Collaborator Author

OK. I will choose to merge this now without testing the validation rule.

@petervdonovan petervdonovan merged commit a2b7f9f into master May 18, 2023
@petervdonovan petervdonovan deleted the fed-naming-collision branch May 18, 2023 18:48
@petervdonovan petervdonovan changed the title Prohibit reactor classes in the same file with the same name, up to case differences Reactor classes in the same file with the same name, up to case differences, are prohibited Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Name collisions between files generated for federated programs
2 participants