Skip to content

Commit

Permalink
Merge pull request #40964 from prakanth97/issue_40914
Browse files Browse the repository at this point in the history
[Master] Reduce creating unwanted type definitions for const annotations
  • Loading branch information
hasithaa authored Jul 17, 2023
2 parents 4e224cd + 24d343b commit d7ad710
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,7 @@ CompiledJarFile generate(BIRPackage module, boolean isEntry) {
// generate the shutdown listener class.
new ShutDownListenerGen().generateShutdownSignalListener(moduleInitClass, jarEntries);

removeSourceAnnotationTypeDefs(module.typeDefs);
// desugar the record init function
rewriteRecordInits(module.typeDefs);

Expand All @@ -811,6 +812,10 @@ CompiledJarFile generate(BIRPackage module, boolean isEntry) {
return new CompiledJarFile(getModuleLevelClassName(module.packageID, MODULE_INIT_CLASS_NAME, "."), jarEntries);
}

private void removeSourceAnnotationTypeDefs(List<BIRTypeDefinition> typeDefs) {
typeDefs.removeIf(def -> Symbols.isFlagOn(def.flags, Flags.SOURCE_ANNOTATION));
}

private BIRFunction getMainFunction(BIRPackage module) {
BIRFunction mainFunc = null;
if (module.packageID.skipTests) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,22 @@ public static BIRNode.BIRAnnotationAttachment createBIRAnnotationAttachment(
}

public static BIRNode.ConstValue getBIRConstantVal(BLangConstantValue constValue) {
BType constValType = constValue.type;
int tag = constValue.type.tag;
if (tag == TypeTags.INTERSECTION) {
constValue.type = ((BIntersectionType) constValue.type).effectiveType;
tag = constValue.type.tag;
boolean isIntersection = false;
if (constValType.tag == TypeTags.INTERSECTION) {
constValType = ((BIntersectionType) constValType).effectiveType;
tag = constValType.tag;
isIntersection = true;
}

if (tag == TypeTags.RECORD) {
Map<String, BIRNode.ConstValue> mapConstVal = new HashMap<>();
((Map<String, BLangConstantValue>) constValue.value)
.forEach((key, value) -> mapConstVal.put(key, getBIRConstantVal(value)));
return new BIRNode.ConstValue(mapConstVal, ((BRecordType) constValue.type).getIntersectionType().get());
return isIntersection ?
new BIRNode.ConstValue(mapConstVal, ((BRecordType) constValType).getIntersectionType().get())
: new BIRNode.ConstValue(mapConstVal, constValType);
}

if (tag == TypeTags.TUPLE) {
Expand All @@ -239,9 +244,9 @@ public static BIRNode.ConstValue getBIRConstantVal(BLangConstantValue constValue
for (int exprIndex = 0; exprIndex < constantValueList.size(); exprIndex++) {
tupleConstVal[exprIndex] = getBIRConstantVal(constantValueList.get(exprIndex));
}
return new BIRNode.ConstValue(tupleConstVal, ((BTupleType) constValue.type).getIntersectionType().get());
return new BIRNode.ConstValue(tupleConstVal, ((BTupleType) constValType).getIntersectionType().get());
}

return new BIRNode.ConstValue(constValue.value, constValue.type);
return new BIRNode.ConstValue(constValue.value, constValType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Stack;
import java.util.function.BiFunction;

import static org.ballerinalang.model.symbols.SymbolOrigin.SOURCE;
import static org.ballerinalang.model.symbols.SymbolOrigin.VIRTUAL;

/**
Expand Down Expand Up @@ -564,12 +566,13 @@ private BLangConstantValue calculateBooleanComplement(BLangConstantValue value)

BLangConstantValue constructBLangConstantValueWithExactType(BLangExpression expression,
BConstantSymbol constantSymbol, SymbolEnv env) {
return constructBLangConstantValueWithExactType(expression, constantSymbol, env, new Stack<>());
return constructBLangConstantValueWithExactType(expression, constantSymbol, env, new Stack<>(), false);
}

BLangConstantValue constructBLangConstantValueWithExactType(BLangExpression expression,
BConstantSymbol constantSymbol, SymbolEnv env,
Stack<String> anonTypeNameSuffixes) {
Stack<String> anonTypeNameSuffixes,
boolean isSourceOnlyAnon) {
this.currentConstSymbol = constantSymbol;
BLangConstantValue value = constructBLangConstantValue(expression);
constantSymbol.value = value;
Expand All @@ -580,6 +583,8 @@ BLangConstantValue constructBLangConstantValueWithExactType(BLangExpression expr

this.anonTypeNameSuffixes = anonTypeNameSuffixes;
updateConstantType(constantSymbol, expression, env);
Optional.ofNullable(isSourceOnlyAnon ? createdTypeDefinitions.get(constantSymbol.type.tsymbol) : null)
.ifPresent(typeDefinition -> typeDefinition.symbol.flags |= Flags.SOURCE_ANNOTATION);
return value;
}

Expand Down Expand Up @@ -667,8 +672,8 @@ private void updateConstantType(BConstantSymbol symbol, BLangExpression expr, Sy
return;
}

if (resolvedType.getKind() == TypeKind.INTERSECTION && isListOrMapping(type.tag)) {
expr.setBType(((BIntersectionType) resolvedType).effectiveType);
if (resolvedType.getKind() == TypeKind.RECORD && isListOrMapping(type.tag)) {
expr.setBType(resolvedType);
symbol.type = resolvedType;
symbol.literalType = resolvedType;
symbol.value.type = resolvedType;
Expand Down Expand Up @@ -846,9 +851,23 @@ private BType createRecordType(BLangExpression expr, BConstantSymbol constantSym
}

createTypeDefinition(recordType, pos, env);
BIntersectionType intersectionType = ImmutableTypeCloner.getImmutableIntersectionType(pos, types,
recordType, env, symTable, anonymousModelHelper, names, new HashSet<>());
return intersectionType;
updateRecordFields(recordType, pos, env);
recordType.tsymbol.flags |= Flags.READONLY;
recordType.flags |= Flags.READONLY;
return recordType;
}

private void updateRecordFields(BRecordType recordType, Location pos, SymbolEnv env) {
BTypeSymbol structureSymbol = recordType.tsymbol;
for (BField field : recordType.fields.values()) {
field.type = ImmutableTypeCloner.getImmutableType(pos, types, field.type, env,
pkgID, env.scope.owner, symTable, anonymousModelHelper, names,
new HashSet<>());
Name fieldName = field.symbol.name;
field.symbol = new BVarSymbol(field.symbol.flags | Flags.READONLY, fieldName,
pkgID, field.type, structureSymbol, pos, SOURCE);
structureSymbol.scope.define(fieldName, field.symbol);
}
}

private boolean populateRecordFields(BLangExpression expr, BConstantSymbol constantSymbol, Location pos,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.ballerina.tools.diagnostics.DiagnosticCode;
import io.ballerina.tools.diagnostics.Location;
import org.ballerinalang.model.TreeBuilder;
import org.ballerinalang.model.elements.AttachPoint;
import org.ballerinalang.model.elements.Flag;
import org.ballerinalang.model.elements.PackageID;
import org.ballerinalang.model.symbols.SymbolKind;
Expand Down Expand Up @@ -2552,7 +2553,7 @@ public void populateAnnotationAttachmentSymbol(BLangAnnotationAttachment annotat
attachedType = Types.getReferredType(attachedType);
attachedType = attachedType.tag == TypeTags.ARRAY ? ((BArrayType) attachedType).eType : attachedType;
}

boolean isSourceOnlyAnon = isSourceAnonOnly(annotationSymbol.points);
BConstantSymbol constantSymbol = new BConstantSymbol(0, Names.EMPTY, Names.EMPTY, env.enclPkg.packageID,
attachedType, attachedType, env.scope.owner,
annotationAttachment.pos, VIRTUAL);
Expand All @@ -2575,7 +2576,7 @@ public void populateAnnotationAttachmentSymbol(BLangAnnotationAttachment annotat
}
} else {
constAnnotationValue = constantValueResolver.constructBLangConstantValueWithExactType(expr, constantSymbol,
env, anonTypeNameSuffixes);
env, anonTypeNameSuffixes, isSourceOnlyAnon);
}

constantSymbol.type = constAnnotationValue.type;
Expand All @@ -2588,6 +2589,15 @@ public void populateAnnotationAttachmentSymbol(BLangAnnotationAttachment annotat
constantSymbol);
}

private boolean isSourceAnonOnly(Set<AttachPoint> attachPoints) {
for (AttachPoint attachPoint : attachPoints) {
if (!attachPoint.source) {
return false;
}
}
return true;
}

public Set<BVarSymbol> getConfigVarSymbolsIncludingImportedModules(BPackageSymbol packageSymbol) {
Set<BVarSymbol> configVars = new HashSet<>();
populateConfigurableVars(packageSymbol, configVars);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public class Flags {
public static final long ENUM_MEMBER = INFER << 1; // 41
public static final long QUERY_LAMBDA = ENUM_MEMBER << 1; // 42
public static final long EFFECTIVE_TYPE_DEF = QUERY_LAMBDA << 1; // 43
public static final long SOURCE_ANNOTATION = EFFECTIVE_TYPE_DEF << 1; // 44

public static long asMask(Set<Flag> flagSet) {
long mask = 0;
Expand Down
1 change: 1 addition & 0 deletions docs/bir-spec/src/main/resources/kaitai/bir.ksy
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,7 @@ types:
'type_tag_enum::type_tag_decimal': decimal_constant_info
'type_tag_enum::type_tag_boolean': boolean_constant_info
'type_tag_enum::type_tag_nil': nil_constant_info
'type_tag_enum::type_tag_record': map_constant_info
'type_tag_enum::type_tag_intersection': intersection_constant_info
instances:
type:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,13 @@ private static void assertConstantValue(Bir.ConstantValue actualConstantValue, O
Assert.assertEquals(booleanConstantInfo.valueBooleanConstant() == 1, expectedValue);
break;
case TYPE_TAG_RECORD:
Bir.MapConstantInfo actualMapConst =
Bir.MapConstantInfo actualMapConst;
if (constantValueInfo instanceof Bir.MapConstantInfo) {
actualMapConst = (Bir.MapConstantInfo) constantValueInfo;
} else {
actualMapConst =
(Bir.MapConstantInfo) ((Bir.IntersectionConstantInfo) constantValueInfo).constantValueInfo();
}
Map<String, BIRNode.ConstValue> expectedMapConst = (Map<String, BIRNode.ConstValue>) expectedValue;
Assert.assertEquals(actualMapConst.mapConstantSize(), expectedMapConst.size());
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import io.ballerina.compiler.api.SemanticModel;
import io.ballerina.compiler.api.impl.values.BallerinaConstantValue;
import io.ballerina.compiler.api.symbols.AnnotationAttachmentSymbol;
import io.ballerina.compiler.api.symbols.IntersectionTypeSymbol;
import io.ballerina.compiler.api.symbols.MemberTypeSymbol;
import io.ballerina.compiler.api.symbols.RecordTypeSymbol;
import io.ballerina.compiler.api.symbols.Symbol;
Expand Down Expand Up @@ -80,14 +79,11 @@ public void testValuesInConstantAnnotationAttachment() {
ConstantValue constVal = annotAttachment.attachmentValue().get();

// Test type-descriptor
assertEquals(constVal.valueType().typeKind(), TypeDescKind.INTERSECTION);
assertEquals(((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(0).typeKind(),
TypeDescKind.RECORD);
assertEquals(((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(1).typeKind(),
TypeDescKind.READONLY);
assertEquals(constVal.valueType().typeKind(), TypeDescKind.RECORD);
RecordTypeSymbol recTypeSymbol =
(RecordTypeSymbol) ((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(0);
assertEquals(recTypeSymbol.signature(), "record {|1 id; record {|1 a; 2 b;|} perm;|}");
(RecordTypeSymbol) constVal.valueType();
assertEquals(recTypeSymbol.signature(), "record {|readonly 1 id; readonly record " +
"{|readonly 1 a; readonly 2 b;|} perm;|}");

// Test const value
assertTrue(constVal.value() instanceof HashMap);
Expand All @@ -101,9 +97,7 @@ public void testValuesInConstantAnnotationAttachment() {

assertTrue(valueMap.get("perm") instanceof BallerinaConstantValue);
BallerinaConstantValue permValue = (BallerinaConstantValue) valueMap.get("perm");
assertEquals(permValue.valueType().typeKind(), TypeDescKind.INTERSECTION);
assertEquals(((IntersectionTypeSymbol) permValue.valueType()).effectiveTypeDescriptor().typeKind(),
TypeDescKind.RECORD);
assertEquals(permValue.valueType().typeKind(), TypeDescKind.RECORD);
assertTrue(permValue.value() instanceof HashMap);
HashMap permMap = (HashMap) permValue.value();
assertEquals(((BallerinaConstantValue) permMap.get("a")).value(), 1L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import io.ballerina.compiler.api.symbols.AnnotationSymbol;
import io.ballerina.compiler.api.symbols.ConstantSymbol;
import io.ballerina.compiler.api.symbols.Documentation;
import io.ballerina.compiler.api.symbols.IntersectionTypeSymbol;
import io.ballerina.compiler.api.symbols.Qualifier;
import io.ballerina.compiler.api.symbols.RecordTypeSymbol;
import io.ballerina.compiler.api.symbols.Symbol;
Expand Down Expand Up @@ -160,14 +159,11 @@ public void testValuesInConstantAnnotationAttachment() {
ConstantValue constVal = annotAttachment.attachmentValue().get();

// Test type-descriptor
assertEquals(constVal.valueType().typeKind(), TypeDescKind.INTERSECTION);
assertEquals(((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(0).typeKind(),
TypeDescKind.RECORD);
assertEquals(((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(1).typeKind(),
TypeDescKind.READONLY);
assertEquals(constVal.valueType().typeKind(), TypeDescKind.RECORD);
RecordTypeSymbol recTypeSymbol =
(RecordTypeSymbol) ((IntersectionTypeSymbol) constVal.valueType()).memberTypeDescriptors().get(0);
assertEquals(recTypeSymbol.signature(), "record {|1 id; record {|1 a; 2 b;|} perm;|}");
(RecordTypeSymbol) constVal.valueType();
assertEquals(recTypeSymbol.signature(), "record {|readonly 1 id; readonly record " +
"{|readonly 1 a; readonly 2 b;|} perm;|}");

// Test const value
assertTrue(constVal.value() instanceof HashMap);
Expand All @@ -182,9 +178,7 @@ public void testValuesInConstantAnnotationAttachment() {
assertTrue(valueMap.get("perm") instanceof BallerinaConstantValue);
BallerinaConstantValue permValue =
(BallerinaConstantValue) valueMap.get("perm");
assertEquals(permValue.valueType().typeKind(), TypeDescKind.INTERSECTION);
assertEquals(((IntersectionTypeSymbol) permValue.valueType()).effectiveTypeDescriptor().typeKind(),
TypeDescKind.RECORD);
assertEquals(permValue.valueType().typeKind(), TypeDescKind.RECORD);
assertTrue(permValue.value() instanceof HashMap);
HashMap permMap = (HashMap) permValue.value();
assertEquals(((BallerinaConstantValue) permMap.get("a")).value(), 1L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void testDisplayAnnotOnObjectAndMemberFunction() {

@Test
public void testDisplayAnnotOnRecord() {
TypeDefinition typeDefinition = result.getAST().getTypeDefinitions().get(23);
TypeDefinition typeDefinition = result.getAST().getTypeDefinitions().get(13);
List<? extends AnnotationAttachmentNode> annot = typeDefinition.getAnnotationAttachments();
Assert.assertEquals(annot.size(), 1);
Assert.assertEquals(annot.get(0).getExpression().toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.wso2.ballerinalang.compiler.semantics.model.symbols.BTypeDefinitionSymbol;
import org.wso2.ballerinalang.compiler.semantics.model.symbols.BVarSymbol;
import org.wso2.ballerinalang.compiler.semantics.model.types.BField;
import org.wso2.ballerinalang.compiler.semantics.model.types.BIntersectionType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BRecordType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BTupleMember;
import org.wso2.ballerinalang.compiler.semantics.model.types.BTupleType;
Expand Down Expand Up @@ -278,10 +277,7 @@ public void testParamAnnotAttachmentsViaBir() {
Assert.assertEquals(mapValue.size(), 1);
Assert.assertEquals(mapValue.get("i").value, 1L);
BType type = constAttachmentSymbol.attachmentValueSymbol.type;
Assert.assertEquals(type.tag, TypeTags.INTERSECTION);
BIntersectionType intersectionType = (BIntersectionType) type;
BType effectiveType = intersectionType.effectiveType;
Assert.assertEquals(effectiveType.tag, TypeTags.RECORD);
Assert.assertEquals(type.tag, TypeTags.RECORD);

params = detachMethod.symbol.params;
annotationAttachmentSymbols = params.get(0).getAnnotations();
Expand All @@ -303,8 +299,6 @@ public void testParamAnnotAttachmentsViaBir() {
Assert.assertEquals(mapValue.size(), 1);
Assert.assertTrue(members.remove(mapValue.get("i").value));
type = constAttachmentSymbol.attachmentValueSymbol.type;
Assert.assertEquals(type.tag, TypeTags.INTERSECTION);
type = ((BIntersectionType) type).effectiveType;
Assert.assertEquals(type.tag, TypeTags.RECORD);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ public void testRecordTypeResolvingWithTypeInclusion() {
BRunUtil.invoke(recordFieldRes, "testRecordTypeResolvingWithTypeInclusion");
}

@Test
public void testAnnotWithRecordTypeDefinition() {
BRunUtil.invoke(compileResult, "testAnnotWithRecordTypeDefinition");
}

@AfterClass
public void tearDown() {
compileResult = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,29 @@ public function returnTupleWithSingletonType2() returns X2 {
return ["x", 2, ()];
}

type Data record {|
string value;
|};

const annotation Data dataAnon on type;

@dataAnon {
value: "T1"
}
type Person2 record {|
string name;
|};

function testAnnotWithRecordTypeDefinition() {
Person2 foo = {name: "James"};
typedesc<any> t = typeof foo;
Data? annon = t.@dataAnon;
assertEquality("{\"value\":\"T1\"}", annon.toString());

Data data = {value: "T2"};
assertEquality("{\"value\":\"T2\"}" , data.toString());
}

function assertTrue(any|error actual) {
assertEquality(true, actual);
}
Expand Down

0 comments on commit d7ad710

Please sign in to comment.