-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Inlining still breaks type checking #11924
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
Comments
I think we might have to compromise here, at least for inline parameters. If we type ascribe everything it will be a huge pain for meta programming, and the errors we can expect from that will far outweigh the (very rare) breakages we would entail otherwise. |
Is it really necessary to compromise? IMHO mere type ascriptions should never break metaprogramming usages, especially if the ascribed type is the intersection of the expression's type with the expected type. I don't quite see why that would break inline matches or match types. Perhaps the breakage we can see in my PR exposes bugs in these? (I have not had time to investigate, and TBH I probably won't in the near future.) I know the scope of Squid's metaprogramming was smaller and easier to reason about, but in Squid, type ascriptions were always added upon substitutions (unless the types matched already), and code transformations, including quote pattern matching, transparently saw through them. |
They break any use of meta programming that expects (say) a constructor application or a constant, instead of a type ascrption. |
But surely this could be fixed in the implementation, no? Here is an example of how it works in Squid, from the REPL: scala> val c = code"List.fill(42: Int)(0): List[Int]"
c: squid.IR.ClosedCode[List[Int]] = code"scala.collection.immutable.List.fill[scala.Int](((42): scala.Int))(0)"
scala> c match { case code"List.fill(${Const(n)})($e: Int)" => (n, e) }
res8: (Int, squid.IR.Code[Int,Any]) = (42,code"0") Notice that Basically, I treat type ascription as a transparent piece of metadata that never gets in the way of metaprogramming use cases. This way, we retain type-preserving substitution in all cases. |
If we change the implementation's API contract, yes. That's something to consider for the next iteration. |
I think the crucial thing to make progress is really to make type ascriptions transparent in the quoted APIs. So let me reassign to @nicolasstucki. |
Ascriptions are already transparent in the quoted patterns. |
Compiler version
3.0.0-RC2-bin-20210325-ab2664f-NIGHTLY
Problem
It is not enough to special-case
Nothing
(type-ascribing only trees of this type as in #11917). In principle, all inlined parameters may require a type annotation. This is because removing type annotations in Scala can break type checking, as subtyping is not transitive.Minimized code
Expectation
If it type-checks without
inline
, it should type-check withinline
too. In particular, I assume that non-transparentinline
is not supposed to interfere with type checking in any way. Is that a correct assumption?The text was updated successfully, but these errors were encountered: