Open MateuszKubuszok opened 5 months ago
behaviour still present on 3.5.0-RC1-bin-20240508-b10d64e-NIGHTLY
@hamzaremmal do you think that would be appropriate for a spree?
Still not sure if this is appropriate for a spree and how hard that would be to fix. cc @hamzaremmal
Sorry @mbovel, I haven't seen the first ping. I don't think it's suitable for a spree.
It's not a bug in compiler. The actual issue is in macros. This version ( without a branch for module) works fine
Explanation:
The removed from above code:
CaseDef(
Bind(bindName, Ident(sym.companionModule.termRef)),
None,
body
)
generates the following code:
b match
case a$$macro1 @ a => println("a")
case b$$macro1 @ b => println("b")
which is equivavlent to case _ => println("a")
. See this section about varid
at https://www.scala-lang.org/files/archive/spec/3.4/08-pattern-matching.html#variable-patterns
Example in scastie - https://scastie.scala-lang.org/s0mcdSvpTqOJ3fPXDpnEtQ
So in summary, we might have thought that this macro generates this (which would work correctly):
(lower.b: lower) match
case amacro @ lower.a => (...)
case bmacro @ lower.b => (...)
case bmacro @ lower.c => (...)
but it actually interprets it as this:
(lower.b: lower) match
case amacro @ a => (...)
case bmacro @ b => (...)
case bmacro @ c => (...)
This would make a lot of sense, as that explains the different behavior of Upper and lower, however the worrying part are the compiler printouts showing what gets generated:
lower.b match
{
case a$$macro$5 @ lower.a => println("a$")
case b$$macro$5 @ lower.b => println("b$")
case c$$macro$5 @ lower.c => println("c$")
}
which looks correct. I guess there may be a node missing needed for PatternMatcher that does not usually get printed out in the compiler, but gets added in Typer or other phases (like the Typed node) - which is why the macro example doesn't work, but the regular example does.
@jchyb
however the worrying part are the compiler printouts showing what gets generated:
Created Ident
in macros goes with type lower.a
- that's why it's printed like a Select
.
However, it's an Ident
and at PatternMatcher
stage it sees Bind(" a$$macro$5", Ident("a"))
and then it falls into WildcardPattern
- https://github.com/scala/scala3/blob/e5f7272e55ba71f80c553bd66e54df480355420a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala#L210
Yeah, we did tackle this at some point - I think it was a spree. And basically there's some code branch (I think it was within Quotes) that considers that lower
is stable, so the tree representation can be folded down into just an Ident with the correct type. So on the one hand we can't write that correctly in source, on the other hand the prefix is indeed pure, so folding it isn't wrong per se... So I wasn't sure where the correct fix is; that is from the assumption that the macro user's code is reasonable - the workaround is changing the macro code.
@dos65 @dwijnand Thank you for the explanations!
I imagine the best course of action would be to add a check for incorrect/unexpected Idents in -Xcheck-macros (the main fear here is there could be a lot of those in the already published libraries), alternatively we could sanitize Idents into correct forms in Quotes IdentModule.apply() depending on the termRef (this might be unpopular/cause other unexpected issues)
Compiler version
3.3.3
Minimized code
repro.scala
repro.test.scala
Output
Expectation
When
case object
with lowercased names is used match seem to fall through on the firstcase
. For the same macro code the behavior is correct if the name of case object starts with an upper case.I haven't observed such issue with
enum
s.