Nightly Dotty workflow of 2021-11-04 failed #13878
Comments
|
Dale and I are in the Twilight Zone on this. We are seeing some crazy, inexplicable shit that's making us doubt our own sanity. Stay tuned. |
we've got the compiler giving different results on compiling this code depending on the filename you put it in. we've got the above code in two identical files: but then compiling one of them gives the expected "Widening conversion from Long to Float" warning and the other doesn't. and this is reproducible on both Dale's machine and my machine and it's deterministic. some filenames reproducibly give one result, other filenames reproducibly give the other. |
|
some yak hair may need to hit the barbershop floor before we can even effectively troubleshoot this |
|
I just hit a related problem in my own library. I had to change to explicit |
|
@soronpo I guess I'm not in a position to judge yet whether that might be the same, but I hope you'll circle back and test it again after we merge a fix to this |
|
So I have a small diff of changes that allow me to build bootstrapped jars and cause the reproduction, while being able to actually see all the log messages (including when typerState.fresh changes the reporter to be a store reporter!!), and the behaviour change seems to come from a different order of implicits leading to a different "best implicit" being selected: |
|
Maybe a good time to suggest: (an option to enable) a warning when it's up to luck which implicit is selected. |
|
OK, good news: this is only a warning and not actually impacting which implicit is selected. Of course, if you make all warnings fatal, then it still has a real impact (outside of the checkfile mismatch in our test suite). The problem is that the In terms of "why does the file name change things?" well that's because file names are inserted in the names table: def getFile(name: String): AbstractFile = getFile(name.toTermName)Different names will have different name table hash values, which will impact the hash values and therefore the index of later names, particularly the members of Hashcodes matters because of the use of Set, which brings me to this documentation: /** The set of names of members of this type that pass the given name filter
* when seen as members of `pre`. More precisely, these are all
* of members `name` such that `keepOnly(pre, name)` is `true`.
* @note: OK to use a Set[Name] here because Name hashcodes are replayable,
* hence the Set will always give the same names in the same order.
*/
final def memberNames(keepOnly: NameFilter, pre: Type = this)(using Context): Set[Name] = this match {I'm not sure what "replayable" means, so I can't tell whether the status quo is still following or not that requirement. There's a few ways I've been able to avoid the difference in order with these two files. One is by changing the set used: def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = {
//var names = scala.collection.immutable.ListSet[Name]()
var names = Set[Name]()Another is removing this code, which is forcing names to be inserted (and which is also dead code): val predefClassNames: Set[Name] =
Set("Predef$", "DeprecatedPredef", "LowPriorityImplicits").map(_.toTypeName.unmangleClassName) |
|
We can also add name ordering to the implicit sorting: @@ -1319,12 +1321,15 @@ trait Implicits:
0
len(sym1) - len(sym2)
+ val candidateNameOrdering: Ordering[Candidate] = Ordering.by(c => c.implicitRef.implicitName: Name)
+
/** A relation that influences the order in which eligible implicits are tried.
*
* We prefer (in order of importance)
* 1. more deeply nested definitions
* 2. definitions with fewer implicit parameters
* 3. definitions whose owner has more parents (see `compareBaseClassesLength`)
+ * 4. implicit name ordering
* The reason for (2) is that we want to fail fast if the search type
* is underconstrained. So we look for "small" goals first, because that
* will give an ambiguity quickly.
@@ -1340,7 +1345,8 @@ trait Implicits:
val cmpArity = arity1 - arity2
if cmpArity != 0 then return cmpArity // 2.
val cmpBcs = compareBaseClassesLength(sym1.owner, sym2.owner)
- -cmpBcs // 3.
+ if cmpBcs != 0 then return -cmpBcs // 3.
+ candidateNameOrdering.compare(e1, e2) // 4.
/** Check if `ord` respects the contract of `Ordering`.
*That also nicely shows that even with the wrong order the problem is present: floatWrapper is tried earlier than longWrapper, emitting its lossy warning even though it's not used. |
|
Still unanswered (and I don't intend to spend time on it): why does this only occur (in 3 nightlies now) on jdk 8 and not jdk 16? |
I'm not completely sure this is true. In my case (and I believe this is related to my case) it affected which extension method was selected. This was a change in behavior. I can try and minimize to give a better example. |
|
OK, I understand now why attempted implicits, such as floatWrapper, are leaking warnings despite the implicit being discarded in favour of longWrapper: because of suppressed warnings (aka configurable warnings). In general infos/diagnostics are stored in the reporter, which is on the typerState, where it can then be never unbuffered or force @lrytz free on Monday for some pairing? 😄 |
|
@dwijnand here is a minimized code: import compiletime.ops.int.Max
import scala.annotation.targetName
opaque type Inlined[T] = T
object Inlined:
extension [T](inlined: Inlined[T]) def value: T = inlined
inline given fromValue[T <: Singleton]: Conversion[T, Inlined[T]] =
value => value
@targetName("fromValueWide")
given fromValue[Wide]: Conversion[Wide, Inlined[Wide]] = value => value
def forced[T](value: Any): Inlined[T] = value.asInstanceOf[T]
extension [T <: Int](lhs: Inlined[T])
def max[R <: Int](rhs: Inlined[R]) =
forced[Max[T, R]](lhs.value max rhs.value)The code works in 3.1.1-RC1, but fails on master. |
See https://github.com/lampepfl/dotty/actions/runs/1419651199
The text was updated successfully, but these errors were encountered: