paulk-asert commented on PR #3055:
URL: https://github.com/apache/groovy/pull/3055#issuecomment-6565954109
Thanks both for the reviews. The branch has been rebased onto current master
and now has two commits: the squashed implementation (74ed5d53d2) and a
follow-up that reads record components through their accessors (b11e3fc2dc).
This comment describes the PR as it stands. The original description above
tells how it got here, and its public API list is still accurate.
## What this PR provides
**The language feature (GEP-19)**
- A `case` label can be a pattern, and so can the right operand of
`instanceof`:
- type patterns with a binding: `case String s ->`
- record patterns: `case Point(int x, var y) ->`, `o instanceof Point(var
x, _)`
- list patterns with an optional rest: `case [1, var second, var... rest]
->`
- map patterns over named entries: `case [name: var n, age: _] ->`
- `var x` bindings, `_` wildcards, constants, and nested patterns anywhere
a component is expected
- A `when` guard refines an arm.
- Pattern labels are arrow-only. Colon labels, `isCase` labels and the
existing `case Foo:` class labels keep their meaning, and a switch can mix them
with pattern arms.
- Plain `o instanceof T t` keeps JEP 394 flow scoping.
- Primitive type patterns follow JEP 507.
**Type checking, under `@TypeChecked` / `@CompileStatic`**
- A pattern switch is exhaustive when an arm is unconditional for the
selector type or the arms cover a sealed hierarchy.
- A non-exhaustive switch expression is an error, as in Groovy 6.
- A non-exhaustive switch statement with a pattern label is also an error,
as in Java (JEP 441). A statement without pattern labels needs no `default`, so
existing code keeps compiling.
- The type checker also reports a pattern that can't match the selector, a
record pattern of the wrong arity, and an arm dominated by an earlier one.
**The AST**
- A pattern is a `MatchPattern`: a sealed model of records in
`org.apache.groovy.ast.pattern`, `@Incubating`. Its bindings are the declaring
`VariableExpression`s.
- A `PatternExpression` holds the `MatchPattern`. It is the case label or
the `instanceof` operand, and plain `o instanceof T t` is one too.
- `GroovyCodeVisitor.visitPatternExpression` is the default traversal.
**When lowering happens**
- `PatternLowering` runs as the first operation of instruction selection.
Until then, everything sees only the pattern as written: resolution, scoping,
every semantic-analysis and canonicalization transform, compilation customizers
(including `SecureASTCustomizer`), the console AST browser and the language
server.
- After lowering, the type checker and class generation see only ordinary
AST:
- a per-switch subject variable
- arm labels that are boolean tests
- the JEP 394 `instanceof` shape for type patterns
- a second per-switch slot, so an `Iterable` or array selector is
materialised at most once
- A transform that creates a pattern during or after instruction selection
calls `PatternLowering.lower(ClassNode, SourceUnit)`. An unlowered pattern that
reaches class generation is a `GroovyBugError`.
**How components are read**
- A record whose components are known at compile time is read through its
accessors. That covers native Groovy records and Java records, including
precompiled ones.
- Accessors are called in component order, and only as far as the match
proceeds.
- Under `@CompileStatic` an arm compiles to a type test and
`invokevirtual` accessor calls, with no runtime support call.
- Emulated records and any other value with `toList()` are deconstructed at
run time. `toList()` is called once per arm tried.
- Generated code links only to `RecordPatternSupport`, `ListPatternSupport`
and `MapPatternSupport`, which carry `@GroovyABI(since = "7.0.0")`.
**Documentation and compatibility**
- The language guide has a new section, "Pattern matching in switch and
instanceof", and its examples run as spec tests.
- COMPATIBILITY.md records the labels whose meaning changes (`case Foo()`,
`case []`, `case [:]`, and `_` in a list or map label) and the new AST shape of
`instanceof T name`.
- ARCHITECTURE.md notes the lowering phase.
- apache/groovy-website#88 brings GEP-19 in line with this implementation
and is meant to be merged at the same time as this PR.
## Review points
**@blackdrag**
1. **Lifecycle of the pattern and the lowered code.** Lowering has moved
from the start of semantic analysis to the first operation of instruction
selection, so everything before it sees only the `PatternExpression` and
everything after it sees only the lowered code. Pattern, guard and source are
final, so a transform that wants a different match builds a new node. One
residual remains: after lowering, the node holds both forms and `setLowered` is
public, so a transform running inside instruction selection could still put
them out of step. The window is now narrow and documented, but it isn't closed.
2. **Tooling sees the semantic form.** Until lowering, the default
`visitPatternExpression` visits the source, the guard and the literals inside
the pattern, never generated code. `SecureASTCustomizer` checks the pattern,
its literals and its binding types, and it no longer sees calls to the runtime
support classes. The console AST browser shows `Pattern - Point(var x, int y)
when (x > 0)`, and the script view renders patterns as written.
3. **`MissingMethodException`.** The catch is gone. The runtime path asks
the meta-class whether the value responds to `toList()` and invokes it only if
it does. A non-List result is reported, and an exception thrown inside a user's
`toList()` propagates unchanged. There are tests for all three cases.
4. **Iterable materialisation.** A switch with list-pattern arms
materialises its selector into the per-switch slot the first time a list arm is
tried, and every later list arm reuses it. A test counts `iterator()` calls for
switch statements and expressions, in dynamic and static code.
5. **`@GroovyABI`.** `RecordPatternSupport`, `ListPatternSupport` and
`MapPatternSupport` now carry `@GroovyABI(since = "7.0.0")`.
**@daniellansun**
1. **`@since`.** Every new type now says 7.0.0.
2. **Compatibility notes.** See above. `case List l` wasn't valid syntax
before patterns, so it isn't a carve-out and has no entry.
3. **Constant-true label.** `lowerArm` now has a comment explaining why list
and map arms use a constant-true label.
4. **`elementsOrNull`.** The javadoc now says a `List` is matched in place
and can observe concurrent modification, and that an array or `Iterable` is
copied once per switch. I went with documentation rather than a defensive copy,
so the common `List` case still pays nothing extra.
5. **Direct accessors.** Done in b11e3fc2dc, in dynamic and static code
alike.
- A component read is an accessor call such as `__$$rp1.x()`, not a list
built through `RecordPatternSupport`.
- A failing component test leaves the later accessors uncalled.
- A primitive component matched by its own type binds directly, with no
test.
- Emulated records (`@RecordOptions(mode = EMULATE)`), other `toList()`
deconstructables and wrong-arity patterns still use the runtime path.
- Bytecode-level tests cover source, precompiled and emulated records,
and the short-circuit order.
## Still open
`SwitchBootstraps.typeSwitch` on a 21+ target, plus three language questions
from #2933:
- whether a typed component on a reference-typed record component should
match `null` (Java's rule, which would let `case Box(Object o)` cover `Box`)
- whether dominance should be a warning or an error
- whether `case null` should trigger exhaustiveness
The pattern-statement exhaustiveness error described above just needs
confirming rather than deciding.
## Verification
core 19,438 tests, groovy-console 228, groovy-macro 66 and groovy-lsp 346,
all passing. `lintGate` is clean. japicmp against 5.1.3 shows the new API is
additive only. (Note - this should really check against 6 - I'll update that
separately)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
