Comments (9)
Related issue found in OpenCB build.
trait Context
class AutoDerivationMacro extends Reflection:
implicit val context: Context = ???
def usage = summon[Context] // error
trait Reflection:
given Context = context
val context: Context
yields warnings under -source:3.5 or errors in 3.6+:
[warn] ./src/main/scala/test.scala:4:30
[warn] Given search preference for Context between alternatives (AutoDerivationMacro.this.context : Context) and (AutoDerivationMacro.this.given_Context : Context) will change
[warn] Current choice : the first alternative
[warn] New choice from Scala 3.6: none - it's ambiguous
[warn] def usage = summon[Context] // error
[warn] ^
I belive a change to mitigate this issue in compiler codebase was done in the PR introducing the change
from dotty.
The issue seems to affect currently 17 projects in the Open CB:
Project | Version | Build URL | Notes |
---|---|---|---|
cchantep/acolyte | 1.2.9 | Open CB logs | |
com-lihaoyi/pprint | 0.9.0 | Open CB logs | |
foldables-io/skunk-tables | 0.0.3 | Open CB logs | |
ghostdogpr/caliban | 2.8.1 | Open CB logs | |
japgolly/microlibs-scala | 4.2.1 | Open CB logs | |
julianpeeters/dynamical | 0.6.0 | Open CB logs | |
lichess-org/lila | HEAD | Open CB logs | |
polyvariant/smithy4s-caliban | 0.1.0 | Open CB logs | |
sagifogel/proptics | 0.5.2 | Open CB logs | |
scala-cli/scala-cli-signing | 0.1.14 | Open CB logs | |
scala-tsi/scala-tsi | 0.8.3 | Open CB logs | |
thoughtworksinc/dsl.scala | 2.0.0 | Open CB logs | |
tkrs/mess | 0.3.6 | Open CB logs | |
tpolecat/doobie | 1.0.0-RC5 | Open CB logs | |
virtuslab/scala-cli-signing | 0.2.3 | Open CB logs | |
zeal18/zio-mongodb | 0.10.4 | Open CB logs | |
zio/zio-protoquill | 4.8.5 | Open CB logs |
from dotty.
OK, looking at the original code before #21226 it looks to me that unless one of the participants was the Not
given class, both alt1IsGiven and alt2IsGiven were always the same. Which means that given vs implicit did in fact not make a difference in prioritization. Since introducing the prioritization causes a lot of regressions, I think it's better we revert that change.
So this means the only given to de-prioritize is in fact Not
. I wonder whether there is a more organic way to achieve that Not
has low priority than threading alt{1/2}isImplicit
through isAsGoodValueType
?
from dotty.
While we are at it. Can we get rid of alt{1/2}isImplicit
also here?
if Feature.sourceVersion.isAtMost(SourceVersion.`3.4`)
|| oldResolution
|| alt1IsImplicit && alt2IsImplicit
then
// Intermediate rules: better means specialize, but map all type arguments downwards
// These are enabled for 3.0-3.5, and for all comparisons between old-style implicits,
// and in 3.5 and 3.6-migration when we compare with previous rules.
This test is now problematic since it violates the transitivity requirement. If we have a given G and implicits I1, I2 with
I1 < G < I2
where <
means more specific, then from 3.6 on we pick I1 if we compare I1 with I2, yet pick I2 if we compare first I1 with G and then G with I2.
Previously this was not deemed a problem because we thought that the given G would always be picked over both I1 and I2. But that assumption was in fact false, because of the typo in isGiven
.
from dotty.
OK, looking at the original code before #21226 it looks to me that unless one of the participants was the Not given class, both alt1IsGiven and alt2IsGiven were always the same.
This holds only in the case where alt1
is a given, otherwise alt1IsGiven and alt2IsGiven were always the same, i.e. there was a difference (before #21226) only if:
alt1.symbol.is(Given) && (alt1.symbol != defn.NotGivenClass) != (alt2.symbol != defn.NotGivenClass)
Which means there was even less of a difference and I would hence very much agree with
I think it's better we revert that change.
EDIT: even though alt1IsGiven != alt2IsGiven
is rare (and might hence have had little influence), whether or not alt1.symbol.is(Implicit)
had more of an impact: unless I'm mistaken, the behavior before #21226 was:
if alt1.symbol.is(Implicit) then old-style implicit comparison scheme between alts
else
if (both alts are NotGivenClass) then old-style implicit comparison scheme between alts
else if (neither alts are NotGivenClass) then new-style implicit comparison scheme between alts
else the NotGivenClass alt loses
from dotty.
This test is now problematic since it violates the transitivity requirement. If we have a given G and implicits I1, I2 with
I1 < G < I2 where < means more specific, then from 3.6 on we pick I1 if we compare I1 with I2, yet pick I2 if we compare first I1 with G and then G with I2.Previously this was not deemed a problem because we thought that the given G would always be picked over both I1 and I2. But that assumption was in fact false, because of the typo in isGiven.
Making sure I understood correctly, this is now (after #21226) fixed right ?
from dotty.
While we are at it. Can we get rid of alt{1/2}isImplicit also here?
I don't see why not, I'll try it out
from dotty.
Making sure I understood correctly, this is now (after #21226) fixed right ?
Correct.
from dotty.
don't see why not, I'll try it out
But make a separate commit, so we can roll easily back of openCB chokes up.
from dotty.
Related Issues (20)
- Extension method fails to resolve for member type of a structural type
- find less fragile alternative to .tastycheck testing
- The auto-approximated bound for match types leaks type captures
- Auto-approximating bound for match types caused changes in erasure, which are TASTy- and binary incompatible HOT 1
- scaladoc: clauses with evidence parameters as context bounds
- `scala` script uses an old Scala version for printing help messages HOT 1
- Inconsistent handling of match types on dependent types when overriding HOT 3
- Failed to resolve extension method: cannot establish a reference to [...]
- Regression in `katrix/perspective` for extension methods HOT 3
- Compiler crashes when combining lazy val and macros HOT 2
- Regression in `mjakubowski84/parquet4s` for type inference HOT 2
- different TASTy output with scala2-library-tasty
- Regression: tuple inline matching and head/tail HOT 3
- Scaladoc snippet compilation fails to evaluate a macro HOT 1
- `-source:future-migration` generating warnings on `TypeRepr` matches HOT 1
- Passing an inlined result of tuple concatenation to an inline def that calls an intermediate method fails to typecheck
- Higher-kinded type is incompatible to itself HOT 2
- 3.5.0-RC6 Release procedure
- 3.4.3-RC1 Release procedure HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from dotty.