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

Avoid Compiler compile time dependency on EnsoContext #7299

Merged
merged 8 commits into from
Jul 17, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jul 15, 2023

Pull Request Description

Defining CompilerContext to encapsulate all the existing calls into EnsoContext. Providing implementation of this interface that delegates to EnsoContext. Allows us to compile the Compiler class separately from engine/runtime - after some further work.

Important Notes

The ultimate goal remains to create engine/runtime-compiler project that would be able to properly apply all IRPasses to the IR created by engine/runtime-parser, but it wouldn't need a dependency on the Truffle API and definitely not on the engine/runtime project.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
  • All code has been tested:
    • Unit tests continue to pass.

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 15, 2023
@JaroslavTulach JaroslavTulach requested a review from 4e6 as a code owner July 15, 2023 11:35
@JaroslavTulach JaroslavTulach self-assigned this Jul 15, 2023
@@ -446,7 +409,7 @@ public CompilationStage getCompilationStage() {
*
* @param compilationStage the new compilation stage for the module.
*/
public void unsafeSetCompilationStage(CompilationStage compilationStage) {
void unsafeSetCompilationStage(CompilationStage compilationStage) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to do this change since I joined Enso. Calling a method unsafe, but exposing it to everyone is a poor design practice. Let's hide it!

The only way to call these unsafe method is to:

  • be in the same package
  • have access to CompilerContext.updateModule
  • use ModuleTestUtils in tests

The access control shall make it clear it is not a method for everyone.


CharSequence getCharacters(Module module) throws IOException;

void updateModule(Module module, Consumer<Updater> callback);
Copy link
Member

Choose a reason for hiding this comment

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

This looks good. It will definitelly help us to, at least, make the modifications to modules atomic.

Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

Great to see another iteration of decoupling.

@JaroslavTulach JaroslavTulach merged commit 6864792 into develop Jul 17, 2023
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/CompilerContext branch July 17, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants