This change partially implements [PR #7362], which revises how objects
are destroyed. It is a partial implementation for two reasons:
1. This change moves `Destroy.Op`'s current behaviour into
`Destroy.SubobjectDestroy`, but it doesn't add support for objects with
non-trivial destruction.
2. `Destroy.SubobjectDestroy` is a workaround for `require impls
SubobjectDestroy`. We aren't able to use the latter until the dependents
add their requirements' implementations to their own witness tables.
[PR #7362]: https://github.com/carbon-language/carbon-lang/pulls/7362
The type `type` is now a `FacetType` inst with no constraints. This
brings the model implemented in the toolchain into better alignment with
the language design. The `SemIR::TypeType` struct remains as a scope for
holding the `TypeInstId`, `ConstantId`, and `TypeId` constants, but is
not an `InstKind` anymore.
The `TypeType` inst looks a lot like singletons, but there are many
`FacetType` insts so it doesn't quite fit that model. So we put it
alongside singletons with a fixed inst id but refer to it as a more
general "builtin" inst that is not a singleton.
`Namespace::PackageInstId` is similar, and we group it with `TypeType`
conceptually as another builtin instruction with a fixed id.
No conversion is needed anymore to use a `type` as a facet, since types
also have a `FacetType` type. This simplifies and removes a number of
helpers and branches throughout the code.
The `TypeType` inst is now part of the constant store, so we end up
printing it in the constants block in every test. But it's also named
`type` rather than `%type` to preserve the majority of existing
formatting behaviour, though this does look different from other
constants.
Assisted-by: Opus 5 was used to generate a first draft and validate the
refactoring. Though nearly everything non-trivial the tool wrote has
been modified or rewritten.
Use a single `SemIR::Function` per `Core` interface method, whether it's
generated locally or imported. This prevents generating duplicate
functions, which lead to different types when the witness appears in a
`FacetValue` as part of a specific for a class.
We use a `CanonicalValueStore` of `GeneratedFunction` objects that allow
finding an existing FunctionId for a `Generated` special function before
(re-)generating it. Mangling for `Generated` functions is also moved to
use the values from the `GeneratedFunction`'s canonicalization key, so
that we have a consistent source of truth for the unique ID of a
`Generated` function across all files.
New tests are in
`toolchain/check/testdata/impl/custom_witness/destroy.carbon`.
Don't wait until we reach the end of the eval block to set the value
block on the specific. This is a prerequisite for allowing template
actions to read from the specific.
Building a `for` loop looks `Core.Iterate` up twice: once for
`NewCursor` to make the cursor, and again for `Next` to advance it. A
range that implements neither failed both lookups and reported both, so
a loop over something that isn't iterable produced two errors saying the
same thing about the same expression.
The second lookup is skipped when the first already failed, which is
what `BuildBinaryOperator`'s `diagnose` parameter is for. The
`ErrorInst` it returns instead does not reach the produced SemIR: the
loop is abandoned on the error either way.
Assisted-by: Claude Code
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implementing interface modifiers causes an infinite loop when generating
fingerprints because the witness value generates a fingerprint that's
dependent on something dependent on the witness value. We've debugged
this to the witness table's `elements_id` field.
This hack is a workaround for creating a new block type whose value is
not codependent with its identity.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
When implementing a named constraint, like `impl as N`, any accesses
through `Self` in the named constraint need to get the value of the
associated constant from the impl's witness table. This isn't possible
immediately, since the impl does not even exist until the declaration is
complete. We use the same model as for accesses found directly in the
impl declaration, but applied to the point where accesses in the named
constraint are substituted during identify to point at the impl's self
type. To get there, we need LookupImplWitness instructions in the named
constraint, when re-evaluated during construction of their enclosing
specific, to evaluate to ImplSelfWitness when they are a reference to
the type and interface being implemented.
During impl lookup, for each (generic) impl candidate, we form a
specific for that impl by deducing its generic arguments. Then we
compare the query interface against the impl's specific interface. That
comparison needs the deduced arguments applied to the impl's specific
interface. Previously we were doing this by getting the impl's
constraint facet type with the impl's specific applied (via
`GetConstantValueInSpecific()`) and then identifying that facet type
with the impl's deduced self.
Identify is a fairly expensive operation. It runs subst, trying to
replace `.Self` references. It walks named constraints. It collects
require declarations. We're looking at making it do _more_ in the future
too, including rewrite constraint resolution and collecting rewrite and
same-type constraints. For this reason we have a cache to make it cheap
on the second run, but it's still a very heavyweight operation to
involve in impl lookup, when all we want is to apply the impl's specific
to its target interface.
We almost have all the information we need to avoid the identification
step. We have the impl's specific after deduction. And we have the
SpecificInterface that the impl is targeting in the `Impl` struct. When
we form the specific for the impl itself, we resolve the declaration
block and form new constant values for all instructions in there, but
that does not cover the SpecificInterface that we're storing in the
`Impl` struct. So we add a new instruction to the impl's eval block,
which will be symbolic when the impl is generic and the target interface
depends on a generic parameter. And we store the `InstId` in the `Impl`
struct. This allows us to gets its constant value later with the impl's
specific applied. From that constant value we can then pull out the
SpecificInterface that the impl is targeting.
Point symbolic witnesses into `.Self` written inside an impl decl at the
impl that is being declared. This is tricky because the impl does not
yet exist. So we use a new instruction `ImplSelfWitness` which _will_ be
replaced by the `ImplWitness` once it becomes available. The
`ImplSelfWitness` acts like a symbolic witness, except it does not
perform lookup, since we know which impl we will get a witness from.
This prevents us from finding other impls when performing lookups into
`.Self` in an impl decl, which produces incorrect/incoherent results.
Split up the CppRange interface into smaller parts, with the intent of
improving the diagnostic quality and making the implementation easier to
understand.
This also makes the implementation details of the CppRange machinery
private, which breaks one of the existing tests; that test is split into
two files, one which tests the low-level machinery works, and another
that tests the resulting prelude behavior.
This change exposes a crash in `where` expression handling, where we
would perform a substitution that creates a new `SpecificConstant` that
refers into a region of a generic that has never been resolved. Fix that
by resolving the definition region of a generic if eval sees a
`SpecificConstant` that refers into it. This is usually not necessary
because something else should have resolved that region first, but that
doesn't happen here.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
Destroy local variables and temporaries at each `}`, and when branching
with `break` and `continue`. In `for` statements, destroy loop variables
along with anything created within the loop at the end of each loop
iteration, and destroy the cursor and range object when the loop
terminates.
Assisted-by: Gemini via Antigravity
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Implement the toolchain side of proposal #7254, removing the `:!`
binding
syntax for generic and template parameters in favor of the keywords
`generic`,
`template`, and `runtime` plus contextual defaults for phase.
For valid programs this is semantics-preserving: each binding resolves
to the
same phase, and produces the same SemIR, as it did under `:!`/`:`. The
parser
derives a binding's phase from its syntactic context plus any explicit
phase
keyword; new diagnostics and error recovery for misused keywords are
described
below.
Implementation details for each component:
- Lexer: remove the `:!` (`ColonExclaim`) token, move its virtual
parse-node
budget onto `:`, and add the `generic` and `runtime` keywords.
- Parser: thread a `BindingContext` (`ExplicitParam`, `DeducedParam`, or
`CompileTimeEntityParam`) from declaration introducers down through
parameter
lists to each binding pattern, using a one-token lookahead to
distinguish a
name-qualifier parameter list from a declaration's own final list.
Parameters
of a compile-time entity (`class`, `interface`, `constraint`, `choice`,
`alias`, `export`, `namespace`) and deduced `[]` parameters default to
checked
generic; explicit function parameters and local bindings default to
runtime.
`HandleBindingPattern` resolves the phase from that context plus the
keyword: a
`generic` keyword needs no node of its own (the phase is carried by the
binding's node kind), while a `runtime` keyword is preserved as a
`RuntimeBindingName` node so `check` can name it in a diagnostic. A
phase
keyword that is merely redundant with the contextual default is
diagnosed
here, without invalidating the parse tree.
- Check: a phase keyword that is invalid for its context (for example
`runtime`
on a checked-generic parameter) is diagnosed here, and recovers by
building an
error binding that still introduces the name so that later uses of it do
not
produce cascading errors.
The removed `:!` syntax is now rejected as an ordinary parse error.
The `form`/`:?`/`->?` ("extended types") portion of proposal #7254 is
left for a
separate change.
Assisted-by: Claude Code
`LookupImplWitness` instructions inside the impl declaration can't use
the impl they are apart of. Previously we had an heuristic in eval which
would try to prevent finding the impl for a lookup from inside that
impl. But it breaks when the `.Self` is replaced in a generic impl with
a symbolic, and then that symbolic is replaced in a specific. The
specific's decl block contains that `LookupImplWitness` instruction and
it tries to use the impl it came from. This causes the same specific to
be formed again, but now it exists, so it's used as-is but it has no
decl block yet, and so we crash.
Now we ban an impl while we resolve its specific, both deduction of its
arguments and from any other substitution. The prevents instructions
from inside the impl (which are evaluated when resolving the specific)
from finding their own impl. We do so by adding the ImplId to a stack on
the Context, and then skipping such impls when looking for candidates
during eval.
This fixes a crash, which was demonstrated by the new test being added.
It also makes another todo test pass.
There's a whole lot of other semir churn, which seems to be mostly
reordering of constants. There are some fingerprint changes in
constants, but it appears they are the same canonical instructions, so
they don't represent a behaviour change. For example in
`toolchain/check/testdata/for/actual.carbon` the `%N.patt` constant has
been given its fingerprint suffix now as `%N.patt.aa5`. But they are
both this instruction, so it is just a formatting change:
```
inst6100001A: {kind: SymbolicBindingPattern, arg0: entity_name61000002, type: type(inst61000018)}
- name: `N`
- type: type(inst61000018): <pattern for Core.IntLiteral>; {kind: PatternType, arg0: inst(IntLiteralType), type: type(TypeType)} (concrete)
- value: symbolic_constant61000001
```
We used to have to batch these at the end of pattern traversal in order
to avoid accidentally adding them to a block that was speculatively
pushed for an expression within a pattern, but that's no longer a
concern with the more precise handling of those speculative blocks in
#7445.
In #7436 we stopped substituting `.Self` when collecting witnesses out
of a facet type. While this was correct, it did not capture all the
cases that need to avoid substituting `.Self`. And it poisoned the
`IdentifiedFacetType` cache by not replacing `.Self` but storing the
result in the cache. This led to incoherent behaviour, where the result
of an impl lookup would change depending on which ones had been done
previously.
Now we use a flag to track for each `.Self` if we're currently
type-checking inside the scope where it was introduced in a facet type.
While inside that scope, identify should not replace the `.Self`. Any
use of it should remain as-is since we don't yet know what value will
replace it. We call this state "frozen" since it should not be modified
by identify. This requires a substitution step when we leave the scope
that introduced the `.Self`, to remove the flag. The flag is set in the
`EntityName` of the `SymbolicBinding`, and is part of the canonical
value, since `.Self` can become part of types, which are constants, and
the flag needs to follow it for correct behaviour.
We also have to ensure the flag is the same when doing comparison with
constants from inside a facet type and constants from outside. For
instance in `(Z where .Z1 = ()) where .Z2 = .Z1`, when we arrive at the
second `.Z1` its `.Self` will be frozen, while the `.Z1 = ()` contains a
non-frozen `.Self`. So we add the frozen flag to the first when storing
it in `where_stack` in order to compare the constant values of the two
`.Z1`.
The `WhereExpr` requirement inst kinds now have an `InstConstantKind` of
`AlwaysUnique` instead of `Never`. This allows us to add them to the
usual InstBlocks, and in an `eval fn` body they have a constant value,
so eval does not fail when trying to call that function. We have to be
careful to not consider `AlwaysUnique` as being actually concrete
though, since their constant value erases `.Self`-dependence. This
allows us to stop special casing them when thawing the requirements
block in a `WhereExpr`, and we can just thaw each `InstId` in the block
in a straightforward manner.
We add the new flag to the instruction's fingerprint and name in
formatted semir.
This adds SemIR structs and implements building `observe` lists, as well
as naming, formatting, and importing `observe` declarations.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
In some cases the pattern block can depend on the initializer, so it
must be sequenced after it. See #7469 for a more detailed explanation of
why this is necessary.
The bulk of this change is changing most pattern insts to be `Always`
rather than `AlwaysUnique` constants, so that they can be wrapped in
`SpecificConstant`s to perform substitution. That then lets thunking
rely much more on `SpecificConstant` wrappers instead of deep-copying
the inst tree with modified types.
This approach to thunking should scale better, particularly as things
like form generics make function signatures more complex, because we can
leverage the existing support for constant evaluation and substitution.
Unfortunately, applying this approach to binding patterns will require
more work; see the TODO near the top of `thunk.cpp` for details.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Implements proposal #7016: `self` moves from the deduced implicit list
(`fn F[self: Self]()`) to the front of the explicit list. Its type may
be written explicitly (`fn F(self: Self)`) or omitted, in which case it
defaults to `Self` (`fn F(self)`, `fn F(ref self)`); `self` in the
implicit list is rejected.
Throughout checking, `self` is modeled as the first explicit parameter.
Because a method is just a function whose first parameter is `self`, it
can also be called as an ordinary function with the receiver passed
explicitly (`Type.M(obj, ...)`), not only as `obj.M(...)`. A new
`SemIR::CallArgParamPatterns` helper chooses the parameters matched
against the explicit arguments, excluding a leading `self` only when it
is supplied as a method-call receiver; arity checking, conversion, and
generic deduction use it. The resulting SemIR and lowering are
unchanged: `self` is still `call_param0`, and witnesses, thunks, and
vtables are unaffected.
An omitted `self` type is parsed as a `SelfBindingPattern` node with no
type expression; checking synthesizes the `Self` type so it behaves
exactly like `self: Self`. However, the exact spelling used must match
between a forward declaration and a definition, following #3763's rules
around declaration matching.
Generated functions, thunks, and C++ interop import/export build `self`
as the first explicit parameter, and the `self`-type override (e.g.
Derived->Base for a virtual override) applies to the explicit `self`.
Placement is validated by new diagnostics: `SelfInImplicitParamList`,
`SelfNotFirstParam`, and `SelfOutsideParamList`. The benchmark source
generator and the documentation adopt the `(self)` shorthand; the
prelude, the examples, and the test data are migrated in the following
commits.
Assisted-by: Claude Code with Claude Opus 4.7
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
We exposed `Core.IntLiteral()`, `Core.FloatLiteral()`,
`Core.CharLiteral()`, and `Core.Bool()` as functions as a workaround,
because we had no way to provide the type names without parentheses that
the design requests. But now we can do so, by using an alias. Switch all
of these over from being functions to simply being names of the
corresponding types.
Assisted-by: Gemini via Antigravity
Fix import logic to make all imported packages be children of the
`NameScopeId::Package` scope. Previously, indirectly-imported packages
would end up as children of their importing package's scope, which
resulted in them not being treated as packages at all, and in particular
not being fingerprinted as packages.
Fixing that caused a failure in the fingerprinting logic as we started
to encounter packages with no correspoding import scopes. Instead of
looking for import scopes, use a simpler mechanism to map packages to
their package names, and clean up.
Unfortunately the latter change churns all the fingerprints again :(
Hopefully this is the last time for a while.
Implement support for floating-point <-> integer type conversions as
described in #820 and #845, extended to support `unsafe as` conversions
for the conversions that can't be expressed as either implicit
conversions or `as` conversions.
One tricky part here is conversions from floating-point literals to
integer types. Such literals may have both a very large mantissa and a
corresponding somewhat large negative exponent, and still produce a
result that is in the range of values that a small integer type can
represent. In order to support that while avoiding building very large
2^N or 10^N constants in general, we first compute a conservative
approximation of the number of bits necessary to represent the integer
result, with an early exit if the number is either definitely too large
or definitely zero. The remaining cases have a reasonable bound on the
size of integer necessary to compute the base^exponent multiplicand.
Assisted-by: Gemini via Antigravity
This change adds rudimentary support for C++ ranges in Carbon range-for
loops. Since C++ ranges are exposed through `Core.Iterate`, C++ ranges
have the same limitations as Carbon ranges (e.g. can't return
references).
`Iterate.CursorType` now requires `Destroy`, since types that implement
`CppRangeForIterate` can't be used in range-for loops unless their
cursor type can be verifiably destructible.
When we import from another library in the same package, its entities
end up with our library as their parent scope, resulting in cross-file
fingerprint mismatches. Instead, only include the library ID when
fingerprinting either a package-private entity or an `ImportIRId` that
refers to a particular `SemIR::File`.
Include the library name in the fingerprint of an entity declared
`private` at namespace scope. Include the entity's fingerprint in the
mangling of a library-private entity.
This fixes miscompiles if two libraries in the same package declare
`private` entites with the same name. We can't fix this with internal
linkage because library-private entities can be reachable through
generics defined in the API file of the library.
Assisted-by: Gemini via Antigravity
As suggested in [1], replace `FieldInitializerMap` with a `FieldStore`.
The corresponding `FieldId` is now stored in `FieldDecl`. To make room
for the `FieldId`, the `ElementIndex` is now stored in the `Field`,
along with the initializer.
In import_ref.cpp, resolving `FieldDecl` initializers is now supported,
and in convert.cpp `LoadImportRef` is called to do so. The
`field_initializer_import.carbon` test now passes.
Printing a `FieldDecl` instruction now prints the initializer as well,
if present. See field_initializer.carbon for an example.
[1]:
https://github.com/carbon-language/carbon-lang/pull/7238#discussion_r3283217158
Fixes link failures when referencing a symbol involving a fingerprint
from a different package.
Previously we included the `Namespace`'s `import_id` as part of its
fingerprint, which caused local and imported namespaces to get different
fingerprints. We now store the `import_id` on the `NameScope` instead of
on the `Namespace` inst to avoid this problem.
Also, when we reach a package-level `NameScopeId`, consistently
fingerprint it as a (package name, library name) pair. Previously the
fingerprinting depended on whether it was imported or not, as an
imported `NameScopeId` had a parent scope (the current package). We need
to include the library name here so that private entities with the same
name in different libraries have different fingerprints.
When a class extends an interface, referring to a member name of the
interface as an unqualified name should refer to the class's
corresponding associated entity value, not to the associated entity
itself. Similarly, in an `impl`, unqualified names of associated
entities should refer to the `impl`'s corresponding value for that
entity.
To support this, we treat `impl`s as `extend`ing their implemented facet
type, and we make lookups into an extended facet type use the `Self`
type of the extending `impl` or `class` if lookup finds an associated
entity. We already did the latter if the extending entity was an
interface; this extends the existing support for these other cases.
Class vars are still restricted to simple `name: type` bindings, not
full patterns. This is now handled in the check phase instead of during
parsing.
This is in preparation for supporting `static var`.
Fixes mangling collisions when two thunks with the same name (eg, `Op`)
are created in the same context, which in turn would lead to LLVM
verifier failures and miscompiles.
To support this, add a new value store to track a little more
information about thunks beyond what's in the `Function`.
A `where` expression nested inside a `T impls X` constraint makes
`.Self` ambiguous on the right-hand side of the `where` if `T` is
anything other than `.Self`. After the `where`, the value of a `.Self`
could be `T` or could be the value of `.Self` before the `impls`
constraint: the so-called top-level value of `.Self`.
Implicit use of `.Self` in designators is always allowed, and they are
bound (and replaced by a reference) to the inner-most possible value of
`.Self`. On the right-hand side of the nested `where` above, they have
the value `T as X`.
`.Self impls ...` is also always allowed, since it acts more as a
keyword here, and it always refers to the inner-most possible value of
`.Self`.
Any other explicit use of `.Self` is diagnosed when ambiguous, in any
kind of constraint. This is done in the handling of `WhereExpr` since it
has enough context to allow `.Self impls` (which is an explicit use)
while disallowing other explicit uses. And because it has non-canonical
instructions to work with, so it is able to diagnose errors with precise
locations.
Since `.Self` is no longer going to be marked with depth modifiers, the
eval of `WhereExpr` does not need an input facet value instruction
representing `.Self` to compare with, as they are now going to all be
equivalent. So revert it back to just looking for the `PeriodSelf` name
id, through a shared helper being introduced as `IsPeriodSelf`. And drop
the period self InstId from the `WhereExpr` instruction. This causes
most of the formatted SemIR changes.
Move helpers for working with and replacing `.Self` to their own file,
out of the `facet_type.h` header/cpp files. These are working with
`.Self` facet values more than facet types, though `.Self` is a name
that only exists inside the scope of a facet type.
See
[here](https://docs.google.com/document/d/1rWcueFwIfZox6GKVGxiUG4cBzjrZ6djXiIDGyJDtrE4/edit?tab=t.0)
for the design doc.
This also removes the default value of the `result_type_inst_id`
parameter of `HandleAction`, moves it before the action in the parameter
list, and documents it. This solves two problems:
- The default made it easy to forget, leading to unnecessary
`TypeOfInst` instructions.
- When it was present, putting it after the fairly "bulky" action
argument tended to make the callsite harder to read.
This is only fixing the decision about *whether* to produce a witness.
Implementation of the witness is still a TODO, though where a body is
generated, it should also precisely reflect where one _needs_ to be
generated.
Note the tests:
- toolchain/lower/testdata/function/generic/import_core_witness.carbon
- toolchain/lower/testdata/function/generic/import_unused_def.carbon
These tests can probably be produced _without_ Core.Destroy, but I found
the essence of them while trying to build //examples with Core.Destroy
and a simpler minimization wasn't striking me.
Assisted-by: Google Antigravity with Gemini
---------
Co-authored-by: jonmeow <jperkins@google.com>
This will be used for const-evaling functions. Splitting into a separate
commit since it touches a lot of test files, and a couple fail_todo
tests are no longer failing.
EvalOrAddInst has to create a non-canonical instruction for evaluating a
few typed insts, such as LookupImplWitness which uses an InstId to
provide a location for diagnostics.
But the output of the function is a ConstantId. We do not have access to
the non-canonical InstId after the function returns. But if the constant
value was symbolic, it was being attached to the inst, and the inst
would be added to the eval block of the enclosing generic. This
needlessly added semir for a symbolic value.
The ConstantId returned by EvalOrAddInst can be used immediately, such
as to evaluate an ImplWitnessAccess. In that case, the final evaluated
result is all we need to keep in semir.
If the ConstantId needs to be replaced by specifics, it is only as part
of some other instruction, since ConstantIds themselves are not modified
by specifics, instructions are. In that case, the canonical instruction
in the constant value would have been added to some other (now symbolic)
instruction, which would be replaced by a specific.
This has no functional change, but it reduces runtime overhead and semir
output for LookupImplWitness and ImplWitnessAccess.
Performing a lookup against `Self` inside the definition of the named
constraint leads to cycles, as described in the document [Self
contradictions in Named
Constraints](https://docs.google.com/document/d/17rn2XmME8o2MM4OJqatSVuMa1iYZ1PAgcNrf0PXR9Q4/edit?tab=t.0).
To prevent those cycles, this change introduces a large refactoring of
impl lookup.
The impl lookup done inside eval is reduced to only performing
monomorphization. That is it:
- Only looks for an provides final witnesses.
- Is not allowed to identify the facet type of the query self.
- Returns either a final witness or None (or an error)
The paths for finding non-final witnesses are now done outside of eval,
directly in the initial `LookupImplWitness()` function. If no final
witness it found through eval, the resulting non-final
`LookupImplWitness` instruction witness is returned. It does not produce
cycles to identify the facet type of query self outside of eval, since
that does not result in repeating the identification when resolving
specifics of the named constraint or require decl.
Move the ArrayStack for Context::require_impls_stack into a new class
which tracks a NamedConstraintId (or InterfaceId) for each frame of
RequireImplsIds, so that in type completion we always can find the
correct frame for a given named constraint which is still being defined,
in order to find the RequireImplsIds in the in-progress definition.
Previously we forced a temporary materialization, resulting in it being
treated as an ephemeral reference expression. This change allows
```carbon
var x: Class = {} as Class;
```
even when `Class` is not copyable.
This case is redundant: when deducing against a runtime parameter
pattern, the type is all that matters, and the type is added to the
deduction earlier. Additionally deducing the same argument against
parameter's subpattern just creates duplicate work, because the
subpattern has the same type.