carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

Named parameters in choice types

Open JamesJCode opened this issue 3 years ago • 10 comments

https://carbon.godbolt.org/z/nbjT4z6ov

Choice type declaration example (reduced to issue - associated data declaration) from language design (link) fails to compile in explorer:

package sample api;

choice IntResult {
  Success(value: i32)
  Failure
}

Results in:

COMPILATION ERROR: <source>:4: syntax error, unexpected COLON, expecting COMMA or RIGHT_PARENTHESIS Compiler returned: 1

JamesJCode avatar Jul 20 '22 10:07 JamesJCode

Your version is missing a comma, so just to copy what's in the doc:

choice IntResult {
  Success(value: i32),
  Failure(error: String),
  Cancelled
}

The names aren't supported syntax in explorer, but it does allow:

choice IntResult {
  Success(i32),
  Failure(String),
  Cancelled
}

https://carbon.godbolt.org/z/5Ghbnjha3

I'm assigning this to @geoffromer to decide which he thinks should really work best -- this area isn't fully designed yet and he was thinking it over.

jonmeow avatar Jul 20 '22 21:07 jonmeow

Yep sorry, errant comma was a copy-pasta. The issue is the name not being supported as you show.

JamesJCode avatar Jul 21 '22 00:07 JamesJCode

A potential alternate design could be to take the Rust approach and allow both

choice IntResult {
  Success(i32),
  Failure(error: String),
  // Alternate syntax
  // Failure { error: String },
  Cancelled
}

or perhaps to only offer unnamed fields and allow struct types to function as named fields (Standard ML does this)

choice IntResult {
  Success(i32),
  Failure({.error: String}),
  Cancelled
}

tkadur avatar Jul 21 '22 03:07 tkadur

I suspect we'd recommend Success(_: i32) if we allow names.

Using struct types probably should work. It would likely be a little extra overhead syntactically though, probably both in terms of setting and getting the value.

jonmeow avatar Jul 21 '22 11:07 jonmeow

Another alternative design we should consider: allow either a tuple type or a struct type after the name of an alternative:

choice IntResult {
  Success(i32),
  Failure{.error: String},
  Cancelled
}

(with matching syntax where a choice value is created or pattern matched).

zygoloid avatar Oct 14 '22 19:10 zygoloid

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Jan 13 '23 02:01 github-actions[bot]

I'm assigning this to @geoffromer to decide which he thinks should really work best -- this area isn't fully designed yet and he was thinking it over.

I'm afraid I don't remember what this was referring to. #2187 pinned down some aspects of the design a little better, but nothing that's relevant to this issue. As far as I know, the fact that Explorer doesn't allow (much less require) names for the alternative parameters is just a bug, not an alternative design that's under consideration.

geoffromer avatar Jan 13 '23 21:01 geoffromer

I'm assigning this to @geoffromer to decide which he thinks should really work best -- this area isn't fully designed yet and he was thinking it over.

I'm afraid I don't remember what this was referring to. #2187 pinned down some aspects of the design a little better, but nothing that's relevant to this issue. As far as I know, the fact that Explorer doesn't allow (much less require) names for the alternative parameters is just a bug, not an alternative design that's under consideration.

I think the lack of names is the design alternative brought up by zygoloid, https://github.com/carbon-language/carbon-lang/issues/1452#issuecomment-1279398988. That is, if a user wants named parameters, use a struct type.

(even if it wasn't under consideration before, it probably should be now)

jonmeow avatar Jan 17 '23 17:01 jonmeow

I understand this issue report to be about two closely related problems:

  • The Explorer does not support the declaration syntax for alternatives that's in the current language design.
  • The Explorer supports a declaration syntax for alternatives that is not in the current language design.

In both cases, we can either update the Explorer to match the language design, or update the language design to match the Explorer. Presumably the default should be to treat the design as ground truth and update the Explorer, and so far I haven't seen any clear rationale for why we should override that default in either case.

zygoloid's suggestion concerns a third declaration syntax (and an associated callsite syntax) that is not currently supported by either the language design or the Explorer. Whatever the merits of that suggestion, adopting it will not resolve either of the two halves of this issue, so I think it's out of scope.

geoffromer avatar Jan 18 '23 19:01 geoffromer

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. \n\n\n This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Apr 19 '23 01:04 github-actions[bot]