Fix inline record printing in outcome printer #2336
Conversation
| @@ -0,0 +1,6 @@ | |||
| type t0 = T0 { t0: int, }; | |||
| type t1 = A { x: int, } | B | C { c1: string, c2: string, }; | |||
There was a problem hiding this comment.
Doesn't Reason have parens around constructor args?
There was a problem hiding this comment.
I don't think so. Here's an rtop session:
Reason # type a = | A {x:int}: a;
type a = A(): a;
Reason # type a = | A {x:int};
type a = A();
Reason # type a = | A({x:int});;
Error: Syntax error
Reason # type a = | A {x:int}: a;
type a = A(): a;
Reason # type a = | A ({x:int}): a;
Error: Syntax errorThere was a problem hiding this comment.
Maybe it should, though?
There was a problem hiding this comment.
The lack of support for wrapping parens for inline records, when they're required in other cases, is very confusing. It's an inconsistency which would be good to fix.
There was a problem hiding this comment.
Yeah, I just saw someone get pretty confused about this too.
One somewhat related question I have is if there might ever be a change in OCaml that requires syntactically distinguishing values of inline variant records vs. values of variants that happen to contain records as their payload. I doubt it. That tells me that we should probably move to consistent parens everywhere.
If we hypothetically one day figured out a way to be able to drop parens for single argument things, then we would apply it consistently everywhere, not just on variants. Good discussion, thanks.
anmonteiro
force-pushed
the
anmonteiro/print-inline-record-constructors
branch
from
70d1c02 to
6eb0525
Compare
|
@jordwalke rebased on master. |
anmonteiro
force-pushed
the
anmonteiro/print-inline-record-constructors
branch
from
6eb0525 to
f046548
Compare
| type t0 = T0 { t0: int, }; | ||
| type t1 = A { x: int, } | B | C { c1: string, c2: string, }; | ||
| type t2(_) = | ||
| D { x: int, }: t2(int) |
There was a problem hiding this comment.
Great! You have a GADT test case.
| @@ -0,0 +1,6 @@ | |||
| type t0 = T0 { t0: int, }; | |||
| type t1 = A { x: int, } | B | C { c1: string, c2: string, }; | |||
There was a problem hiding this comment.
Yeah, I just saw someone get pretty confused about this too.
One somewhat related question I have is if there might ever be a change in OCaml that requires syntactically distinguishing values of inline variant records vs. values of variants that happen to contain records as their payload. I doubt it. That tells me that we should probably move to consistent parens everywhere.
If we hypothetically one day figured out a way to be able to drop parens for single argument things, then we would apply it consistently everywhere, not just on variants. Good discussion, thanks.
This fixes #1163 and supersedes #2103