-
Notifications
You must be signed in to change notification settings - Fork 78
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
Adding getMatchFingerPrint and getConcreteMatchFingerPrint to implement a part of the compiled-runtime with more efficiency #1873
Conversation
This is a continuation of usethesource/vallang#210 Since getMatchFingerprint also has to be implemented inside the rascal project, and another method ITree.getConcreteMatchFingerprint too, with similar sensibilities, I believe it is time we also bump the rascal project to 1.0.0. This will allow us to manage the tight integration between the compiler and the run-time with semver. The compiler run-time integration will destabilize the rascal project, from the perspective of generated Java code by the compiler. Also, we will start removing the java JDT support and putting it in a separate project (NOT THIS PR). Such an enormous breaking change must be signalled with a major version imho. In this PR we will add two very brittle methods to the IExternalValue implementations and to ITree. |
but would that mean that we go for 1.0 now, and we'll go for 2.0 soon since we'll be dropping all kinds of things? Maybe you could line out the roadmap a bit? it's a bit unclear for me based on your comments. also fine with having a call about this. |
That's the thinking. Penny for yours? |
My thoughts are that a 1.0.0 release signals a certain kind of stability, while we are heading for a somewhat unstable phase with the compiler coming up. in semver paralance, and what we've been doing, is that for 0.x the minor field should be interpreted as a major release. so it becomes If we want to move towards a different scheme, I'm fine with discussing that, but I feel less agreement on moving to 1.0 than I feel for vallang. and agreed, when I said "dropping things" I should have said, refactoring it to side-projects and similar plans. |
The difference is that these breaking changes in getMatchFingerPrint are invisible to the Java compiler and the JVM linking process. There will be no complaints. Yet the code in the rascal compiler and all the Java code generated by it depends exactly on those integer constants. So that is the reason we could bump to 1.0.0 after we introduce those functions here. 0.x.y semver signals nothing and we have not been clear to anybody about what it meant at all. So after 1.0.0 we can continue signalling non-backward compatible external API and language changes via the major version. Any major version update in vallang will automatically lead to a major version bump here. Any API used by generated code is considered external. All other changes internal to the interpreter are not for major changes, unless the Rascal language has changed. |
Alternatively, we could stick with 0.x.y, but I'm afraid that we will not be able to distinguish ourselves anymore which version of the rascal runtime works with which version of the rascal compiler anymore. |
Another step forward without the semver is to include the entire compiler here in this project, and the run-time. That way one complex tightly coupled dependency is gone and we have simply bundled together the compiler with the run-time it depends on. It is a one-to-one tight coupling, afterall. |
The 1.0.0 code is very stable; in the sense that the interpreter has worked the same for years, and most fixes have been in standard library features. So even if we would not have had this new fingerprinting feature, we could argue that the rascal project is mature now. Even if we release 2.0.0 this month, that does not make the code we use in the interpreter less stable; it means we know exactly what we are breaking and why. |
Codecov Report
@@ Coverage Diff @@
## main #1873 +/- ##
=======================================
Coverage 49% 49%
- Complexity 6147 6161 +14
=======================================
Files 660 661 +1
Lines 58668 58687 +19
Branches 8543 8546 +3
=======================================
+ Hits 28893 28946 +53
+ Misses 27577 27543 -34
Partials 2198 2198
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a few small remarks.
Already looked good to me :) but there is still |
No description provided.