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

[Master] Generate new typedesc instruction for record and tuple when type descriptor resolving #43596

Open
wants to merge 88 commits into
base: master
Choose a base branch
from

Conversation

rdulmina
Copy link
Contributor

Purpose

In this PR, I have generated a new 'typedesc' (type descriptor) only once for the tuple and record.
For example:

type Foo record {||};
public function main() {
    Foo f = {};
    record {|int a;|} r = {a: 1};
}

We have desugared the above program as follows:

type Foo record {||};
typedesc<Foo> $typedesc$Foo = Foo;
public function main() {
    Foo f = {};
    typedesc<record {| int a; |}> $typedesc$$anonType$_0 = record {| int a; |}
    record {|int a;|} r = {a: 1};
}

We then use the generated 'typedesc' when creating a map value using that type.

Fixes #38844, #41946, #43311

chiranSachintha and others added 30 commits August 16, 2024 17:05
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
@rdulmina rdulmina added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label Nov 18, 2024
@@ -551,6 +551,11 @@ public static Object getAnnotValue(TypedescValue typedescValue, BString annotTag
if (!(describingType instanceof BAnnotatableType annotatableType)) {
return null;
}
MapValue annotations = ((TypedescValueImpl) typedescValue).annotations;
Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this

}

private BIRVariableDcl getTypedescVariable(BType type, Location pos) {
Supplier<BIRVariableDcl>[] checks = new Supplier[] {
Copy link
Member

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?

Copy link
Contributor Author

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
Copy link
Member

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?

@rdulmina rdulmina force-pushed the typeDesc-stmt branch 3 times, most recently from c7288dc to d70aefa Compare November 19, 2024 07:22
Previously we set the annotation for record type only. Now we set for both the type reference type and record type
Copy link

sonarcloud bot commented Nov 20, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Generate new typedesc instruction for record and tuple when type descriptor resolving
3 participants