-
Notifications
You must be signed in to change notification settings - Fork 752
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
[Master] Generate new typedesc instruction for record and tuple when type descriptor resolving #43596
base: master
Are you sure you want to change the base?
Conversation
…ors" This reverts commit 962d8f9.
…onstructors"" This reverts commit d470345.
Add closures before the dependent node in the top-level node list in `ClosureGenerator.java`
Exclude field generation for typedesc when generating fields for user defined types since those fields will be generated when visiting global variables
Add typedesc statement before the var declaration in the init function body
Checkout Java21 branch in full build pipeline
Checkout typeDesc-stmt branch in full build pipeline
[typeDesc-stmt] Fix usage of incorrect branch in full build pipeline
@@ -551,6 +551,11 @@ public static Object getAnnotValue(TypedescValue typedescValue, BString annotTag | |||
if (!(describingType instanceof BAnnotatableType annotatableType)) { | |||
return null; | |||
} | |||
MapValue annotations = ((TypedescValueImpl) typedescValue).annotations; |
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.
Why do we need this change? Are these annotations only in the typedesc, not in the type?
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.
They are in both the places. I think we can remove this for now
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.
Removed this
compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/BIRGen.java
Outdated
Show resolved
Hide resolved
compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/BIRGen.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
private BIRVariableDcl getTypedescVariable(BType type, Location pos) { | ||
Supplier<BIRVariableDcl>[] checks = new Supplier[] { |
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.
Do we really need this? Given that we don't anticipate additions, can just call and see in order?
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.
Yes that was the previous code. But it was lengthy. A con I feel with this approach is its hard to read and debug. I'm +1 to go with any approach. WDYT?
} | ||
} | ||
|
||
// TODO: we need to remove typedesc creating completely from here and handle it in the Desugar phase |
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.
What scenario is this required for atm?
compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/BIRGen.java
Outdated
Show resolved
Hide resolved
c7288dc
to
d70aefa
Compare
d70aefa
to
7e6125c
Compare
Previously we set the annotation for record type only. Now we set for both the type reference type and record type
a97ead4
to
e5f1afc
Compare
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
Purpose
In this PR, I have generated a new 'typedesc' (type descriptor) only once for the tuple and record.
For example:
We have desugared the above program as follows:
We then use the generated 'typedesc' when creating a map value using that type.
Fixes #38844, #41946, #43311