Parens: miscellaneous fixes

[brianrourkeboll]

Another followup to #16079.

• Don't remove parens when there is a SynExpr.Apply in the middle of a chain of dot-gets in argument position (f (x.M.N().O)).
• Don't remove parens from a binary equals expression in method argument position (lest it be confused with named argument assignment: o.M((x = y))).
• Sequential expressions can't be exposed between if and then.
• More match/try-with nuances.
• Disallow removal around typed expressions and patterns (… : ty) in more places.
• Handle Parenthesized/double unit literal () → (()) required in certain scenarios #16254.
• Handle Inconsistent parenthesization requirements for single-argument additional constructors (with new) #16257.
• Handle another tuple pattern scenario.
• Handle dangling expressions in sequential expressions ([x; (if y = 1 then 2 else 3); z]).

[psfinaki]

Thanks! Tests are getting to about 2000 LOC, if you come up with any fresh thoughts on splitting them - go ahead :)

[brianrourkeboll]

Thanks! Tests are getting to about 2000 LOC, if you come up with any fresh thoughts on splitting them - go ahead :)

Fair enough :)

I'll see if I can find time this week to open a test cleanup PR.

[brianrourkeboll]

Also, with the changes in #16248 and those in this PR applied, I can run the bulk fix on FSharp.Core and compile it with no errors or warnings—with the sole exception of this pair of parentheses in local.fs:

fsharp/src/FSharp.Core/local.fs

Line 39 in d441308

[|SR.GetString SR.notEnoughElements; index; (if index=1 then "element" else "elements")|]

Removing those parentheses results in:

FSC : error FS0193: The module/namespace 'Microsoft.FSharp.Collections' from compilation unit 'FSharp.Core' did not contain the namespace, module or type 'SeqModule'

(No line number is given, and there is no warning or error in VS.)

The reason seems to have something to do with the fact that the array comprehension's target type is obj array, which requires that both branches of the if-then-else expression be implicitly upcast to obj. The following lets FSharp.Core compile successfully, for example:

[|SR.GetString SR.notEnoughElements; index; box <| if index=1 then "element" else "elements" |]

It only seems to affect FSharp.Core, though—the following (AST) compiles and runs as you would expect in any other context:

let f x : obj array = [| "a"; 3; (if x = 1 then "element" else "elements") |]
printfn $"f 1 = %A{f 1}"
printfn $"f 2 = %A{f 2}"

let g x : obj array = [| "a"; 3; if x = 1 then "element" else "elements" |]
printfn $"g 1 = %A{g 1}"
printfn $"g 2 = %A{g 2}"

Is there some special way that FSharp.Core is compiled that would affect the way that expression is compiled?

Edit: there is. The error happens sometime during the CoreCompile task, specifically after the Import non-system references phase finishes and before the Typecheck phase timing is reported (although something happening between then and Typechecked would make more sense to me).

Otherwise, I guess I can just never suggest removing parens from around potentially problematic expressions in sequential expressions, although it should be valid in general when the parenthesized expression is the last expression in the sequence (compare the logic in ocamlformat).

[psfinaki]

I'll see if I can find time this week to open a test cleanup PR.

Sure, this can be a followup.

Also, with the changes in #16248 and those in this PR applied, I can run the bulk fix on FSharp.Core and compile it with no errors or warnings

This is awesome!

with the sole exception of this pair of parentheses in local.fs

I guess it is worth creating a separate issue for this. Nothing comes to my mind on that, but this does not sound right or as anything I would want to happen in the compiler.