Skip to content

Unintended SafeValue Error #23424

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Unintended SafeValue Error #23424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90cacad
Added test case
JasonH53 Jun 25, 2025
ea4e1e6
Fixed unintentional safe value error
JasonH53 Jul 6, 2025
8c06f71
Merge branch 'main' into init-global-warning
JasonH53 Jul 6, 2025
a901b25
Removed redundant comments
JasonH53 Jul 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Objects.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,7 @@ class Objects(using Context @constructorOnly):

case v @ SafeValue(_) =>
if v.typeSymbol != defn.NullClass then
// selection on Null is sensible on AST level; no warning for it
// selection on Null is sensible on AST level but not in practice
report.warning("[Internal error] Unexpected selection on safe value " + v.show + ", field = " + field.show + ". " + Trace.show, Trace.position)
end if
Bottom
Expand Down Expand Up @@ -1573,7 +1573,13 @@ class Objects(using Context @constructorOnly):
val target = expr.tpe.widenSingleton.classSymbol.asClass
withTrace(trace2) { resolveThis(target, qual.asInstanceOf[ThisValue], klass) }
case _ =>
withTrace(trace2) { select(qual, expr.symbol, receiver = qualifier.tpe) }
qual match {
// Check if expression is a selection of a method
case v: SafeValue if expr.symbol.is(Flags.Method) =>
withTrace(trace2) { call(v, expr.symbol, args = Nil, receiver = qualifier.tpe, superType = NoType) }
case _ =>
withTrace(trace2) { select(qual, expr.symbol, receiver = qualifier.tpe) }
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking, why not always dispatch to call if the symbol is a method.

If that is the case, maybe it's better to fix the extractor Call:

scala3/compiler/src/dotty/tools/dotc/transform/init/Util.scala

Lines 44 to 65 in 35c2ef3

def unapply(tree: Tree)(using Context): Option[(Tree, List[List[Arg]])] =
tree match
case Apply(fn, args) =>
val argTps = fn.tpe.widen match
case mt: MethodType => mt.paramInfos
if (args.size != argTps.size)
report.warning("[Internal error] Number of arguments do not match number of argument types in " + tree.symbol.name)
val normArgs: List[Arg] = args.zip(argTps).map {
case (arg, _: ExprType) => ByNameArg(arg)
case (arg, _) => arg
}
unapply(fn) match
case Some((ref, args0)) => Some((ref, args0 :+ normArgs))
case None => None
case TypeApply(fn, targs) =>
unapply(fn)
case ref: RefTree if ref.tpe.widenSingleton.isInstanceOf[MethodicType] =>
Some((ref, Nil))
case _ => None

Copy link
Author

Choose a reason for hiding this comment

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

Great idea! Would that mean that we should change Call:

 case ref: RefTree if ref.symbol.is(Flags.Method) && ref.tpe.widenSingleton.isInstanceOf[MethodicType] =>
        Some((ref, Nil))

and leaving the def cases unchanged?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can try just keep if ref.symbol.is(Flags.Method).

}

case _: This =>
evalType(expr.tpe, thisV, klass)
Expand Down
7 changes: 7 additions & 0 deletions tests/init-global/pos-tasty/test-safe-value.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package scala.collection.immutable

object A { // These are a safe values, so no warning should be emitted
Node.HashCodeLength
Node.BitPartitionSize
Node.MaxDepth
}