-
Notifications
You must be signed in to change notification settings - Fork 762
Generate a typedesc instruction once for record and tuple types #41945
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
Generate a typedesc instruction once for record and tuple types #41945
Conversation
@@ -97,8 +89,8 @@ public Object instantiate(Strand s) { | |||
@Override | |||
public Object instantiate(Strand s, BInitialValueEntry[] initialValues) { | |||
Type referredType = getImpliedType(this.describingType); | |||
if (referredType.getTag() == TypeTags.MAP_TAG) { | |||
return new MapValueImpl(this.describingType, (BMapInitialValueEntry[]) initialValues); | |||
if (referredType.getTag() == TypeTags.MAP_TAG || referredType.getTag() == TypeTags.RECORD_TYPE_TAG) { |
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.
Shall we create a local variable for the result of referredType.getTag()
(or use a switch on that), since we are using it 3 times in the if else block
BIROperand symbolVarOperand = this.env.symbolVarMap.containsKey(typeSymbol.annotations) ? | ||
new BIROperand(this.env.symbolVarMap.get(typeSymbol.annotations)) : | ||
new BIROperand(this.globalVarMap.get(typeSymbol.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.
Assuming we don't put null
to symbolVarMap
shall we change this such that we are not doing the lookup on symbolVarMap
twice
BIROperand symbolVarOperand = this.env.symbolVarMap.containsKey(typeSymbol.annotations) ? | |
new BIROperand(this.env.symbolVarMap.get(typeSymbol.annotations)) : | |
new BIROperand(this.globalVarMap.get(typeSymbol.annotations)); | |
BVarSymbol typeAnnotations = typeSymbol.annotations; | |
BIRVariableDcl localDecl = this.env.symbolVarMap.get(typeAnnotations) | |
BIROperand symbolVarOperand = localDecl != null ? | |
new BIROperand(localDecl) : | |
new BIROperand(this.globalVarMap.get(typeAnnotations)); |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
} else { | ||
createNewTypedescInst(type, type, pos); | ||
return new BIRNonTerminator.NewStructure(pos, toVarRef, this.env.targetOperand, fields); | ||
} |
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.
nit
} else { | |
createNewTypedescInst(type, type, pos); | |
return new BIRNonTerminator.NewStructure(pos, toVarRef, this.env.targetOperand, fields); | |
} | |
} | |
createNewTypedescInst(type, type, pos); | |
return new BIRNonTerminator.NewStructure(pos, toVarRef, this.env.targetOperand, fields); |
} | ||
|
||
for (Map.Entry<BSymbol, BIRGlobalVariableDcl> entry : this.globalVarMap.entrySet()) { | ||
if (isTypeDescSymbol(entry.getKey(), Types.getImpliedType(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.
Can't we separate Types.getImpliedType(type)
into a variable so that function is called only once.
} else { | ||
return new BIRNonTerminator.NewTypeDesc(position, toVarRef, resolveType, closures); | ||
} |
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.
} else { | |
return new BIRNonTerminator.NewTypeDesc(position, toVarRef, resolveType, closures); | |
} | |
} | |
return new BIRNonTerminator.NewTypeDesc(position, toVarRef, resolveType, closures); |
if (getTypedescVariable(listConstructorExprType) != null) { | ||
return new BIRNonTerminator.NewArray(pos, listConstructorExprType, toVarRef, | ||
new BIROperand(getTypedescVariable(listConstructorExprType)), sizeOp, initialValues); | ||
|
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.
globalVar.getBType()); | ||
} | ||
|
||
private boolean containsErrorType(BType 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.
Do we need a separate function for this as this wraps only a single statement
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
Closed PR due to inactivity for more than 18 days. |
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the |
Purpose
Fixes #38844
Fixes #41946
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.
Check List