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

Add supports for type cast and filtering type for field and method owner in global initialization checker #19612

Merged
merged 8 commits into from
Mar 8, 2024

Conversation

EnzeXing
Copy link
Contributor

@EnzeXing EnzeXing commented Feb 5, 2024

This PR adds support for more precise analysis of type casting in global initialization checker, which filters impossible classes after reference casting and preserves only possible classes. It also automatically filter the set of possible classes during field selection or method invocation and only preserves owners of the field/method

case ref: Ref if ref.klass.isSubClass(klass) => ref
case ref: Ref => Bottom
case ValueSet(values) => values.map(v => v.filterClass(klass)).join
case _ => a // TODO: could be more precise for OfArray; possibly add class information for Fun
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed in the meeting, we can further filter OfArray here. Meanwhile, we can remove the old code that partially handles this.

def unapply(tree: Tree)(using Context): Option[(Tree, Type)] =
tree match
case TypeApply(Select(qual, _), typeArg) if tree.symbol.isTypeCast =>
Some(qual, typeArg.head.tpe)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: It's better to changetypeArg to typeArgs.

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

val baseClasses = tpe.baseClasses
if baseClasses.isEmpty then a
else tpe match
case t @ SAMType(_, _) if a.isInstanceOf[Fun] => a // if tpe is SAMType and a is Fun, allow it
Copy link
Contributor

Choose a reason for hiding this comment

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

This case doesn't seem to use the baseclasses. Should all the baseclasses stuff (and the check for empty baseclasses) be pushed down into the second case? Also, perhaps headOption is useful on the baseclasses.

@@ -1549,7 +1572,7 @@ object Objects:
report.warning("The argument should be a constant integer value", arg)
res.widen(1)
case _ =>
res.widen(1)
if res.isInstanceOf[Fun] then res.widen(2) else res.widen(1) // TODO: changing to widen(2) causes standard library analysis to loop infinitely
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the comment because we now know it's not infinite.

val klass = sym.asClass
a match
case Cold => Cold
case ref: Ref if ref.klass.isSubClass(klass) => ref
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with other cases, I'd move the if inside the case body and combine with the following case.

@olhotak olhotak merged commit 8642df1 into scala:main Mar 8, 2024
19 checks passed
@olhotak olhotak deleted the fix-i18882-2 branch March 8, 2024 01:44
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
liufengyun added a commit that referenced this pull request Jun 14, 2024
…20548)

More principled filtering of abstract values in initialization check.

It ports the similar improvement to the global initialization checker
#19612.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants