Add support for var patterns
#5069
Conversation
| // CHECK:STDOUT: %x.param_patt: %empty_tuple.type = value_param_pattern %x.patt, runtime_param0 | ||
| // CHECK:STDOUT: %y.patt: %empty_tuple.type = binding_pattern y | ||
| // CHECK:STDOUT: %y.param_patt: %empty_tuple.type = ref_param_pattern %y.patt, runtime_param1 | ||
| // CHECK:STDOUT: %.loc4_13: %empty_tuple.type = var_pattern %y.param_patt |
There was a problem hiding this comment.
I wonder if we should compute a "pattern category", and list it here like we do for expression categories, eg
| // CHECK:STDOUT: %.loc4_13: %empty_tuple.type = var_pattern %y.param_patt | |
| // CHECK:STDOUT: %.loc4_13: ref %empty_tuple.type = var_pattern %y.param_patt |
That said, if that information is not useful for check, I suppose we'd be computing it only for the purpose of formatted SemIR, and we can get the same info by looking at the right-hand side of the = :-)
(Just thinking out loud here, no change requested.)
There was a problem hiding this comment.
Yeah, I've been thinking that we should at least be able to talk about "pattern categories", whether or not we need to explicitly represent them in semir.
I've also been thinking about an issue that seems closely related: it kind of seems like we're missing an expression category, for an expression that names an output parameter (i.e., that names a pattern that binds to an initializing expression). We currently treat it as a reference expression (see the TODO at the end of GetExprCategory), but I suspect we're only getting away with that because we don't yet distinguish between objects and storage (e.g. we model initialization as assignment).
| // CHECK:STDOUT: %G.decl: %G.type = fn_decl @G [concrete = constants.%G] {} {} | ||
| // CHECK:STDOUT: } | ||
| // CHECK:STDOUT: | ||
| // CHECK:STDOUT: fn @F(%x.param_patt: %empty_tuple.type, %.loc4_13: %empty_tuple.type); |
There was a problem hiding this comment.
It'd be nice for there to be some mention here that %.loc4_13 is a var parameter. (Does not need to be in this PR.) I'd probably format it before the type, to match ref instructions:
fn @F(%x.param_patt: %empty_tuple.type, %.loc4_13: ref %empty_tuple.type);
There was a problem hiding this comment.
Perhaps we should do the same thing with the return slot? For example, in toolchain/check/testdata/class/init.carbon we have
fn @Make(%n.param_patt: %i32, %next.param_patt: %ptr.e71) -> %return.param_patt: %Class {
But maybe that should be:
fn @Make(%n.param_patt: %i32, %next.param_patt: %ptr.e71) -> out %return.param_patt: %Class {
Or even
fn @Make(%n.param_patt: %i32, %next.param_patt: %ptr.e71, out %return.param_patt: %Class) {
| // CHECK:STDERR: fn F[var u: ()](); | ||
| // CHECK:STDERR: ^~~~~ | ||
| // CHECK:STDERR: | ||
| // CHECK:STDERR: fail_implicit.carbon:[[@LINE+4]]:6: error: found `var` pattern in implicit parameter list [VarInImplicitParams] |
There was a problem hiding this comment.
I think we will want to allow fn F[var self: Self]() eventually.
Might be worth adding a test for that with a TODO?
There was a problem hiding this comment.
Definitely worth adding a test, but it seems like if I remove the new diagnostic, it... just works?
There was a problem hiding this comment.
Oh, nice! I assume we get the "var declaration cannot declare a compile-time binding" error for [var N:! i32] or similar? Do we have a test for that? (Aside: I suppose that error should say "var pattern" rather than "var declaration" now.)
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
No description provided.