Fix a few AND operator parser bugs and regressions

[psfinaki]

Fixes #16447
Fixes #17134
Fixes #16309

The problem was introduced together with the constraint intersection support here. The parser no longer stops at the ampersand sign in the clause and tries to interpret :? as part of the type.

Checklist

• Test cases added
• Release notes entry updated

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

[psfinaki]

Hey I am getting a bit desperate with this, whatever I do breaks either pattern matching (the original regression) or constraint intersection (the original feature). I feel I am somewhere close but missing something obvious since I haven't properly worked with the parser before.

The current version is the closest I've got so far - it fixes the regression but partially breaks the constraint intersection, in particular this fails to identify the first hash constraint out of multiple hash constraints.

E.g. for

let y (f: #I & #Task<int> & #seq<string>) = ()

this will identify #Task<int> and #seq<string> as a HashConstraint but #I is just LongIdent.

Any ideas, maybe @auduchinok or @kerams?

[brianrourkeboll]

@psfinaki

Would it make sense to define something like this separately:

hashConstraint:
  | HASH atomType
     { SynType.HashConstraint($2, lhs parseState) }

and then use hashConstraint in place of HASH atomType here:

fsharp/src/Compiler/pars.fsy

Lines 6215 to 6217 in 32e2822

	atomType:
	| HASH atomType
	{ SynType.HashConstraint($2, lhs parseState) }

and here:

fsharp/src/Compiler/pars.fsy

Lines 5995 to 6000 in 32e2822

	intersectionType:
	| typar AMP intersectionConstraints %prec prec_no_more_attr_bindings // todo precedence
	{ let constraints, mAmpersands = $3
	SynType.Intersection(Some $1, List.rev constraints, lhs parseState, { AmpersandRanges = rhs parseState 2 :: List.rev mAmpersands }) }
	| HASH atomType AMP intersectionConstraints %prec prec_no_more_attr_bindings // todo precedence

and in place of atomType here (with the recovery/error-reporting adjusted accordingly):

fsharp/src/Compiler/pars.fsy

Lines 2581 to 2596 in 32e2822

	intersectionConstraints:
	| intersectionConstraints AMP atomType %prec prec_no_more_attr_bindings // todo precedence
	{ let constraints, mAmpersands = $1
	match $3 with
	| SynType.HashConstraint _ -> ()
	| ty -> errorR(Error(FSComp.SR.parsConstraintIntersectionSyntaxUsedWithNonFlexibleType(), ty.Range))
	($3 :: constraints), (rhs parseState 2 :: mAmpersands) }
	| atomType
	{ match $1 with
	| SynType.HashConstraint _ -> ()
	| ty -> errorR(Error(FSComp.SR.parsConstraintIntersectionSyntaxUsedWithNonFlexibleType(), ty.Range))
	[ $1 ], [] }

?

[psfinaki]

@brianrourkeboll first of all - thanks for jumping in :) (this was meant to be sent on Tuesday but somehow didn't post here)

Secondly, the approach you suggest seems to do the job!
I was mostly playing with precedences but that didn't satisfy all tests. So thanks for your fresh thoughts.

[psfinaki]

Okay this is green finally :)

[brianrourkeboll]

I think this approach should work — as long as we never need to extend the intersection type syntax to allow anything other than flexible types.

[psfinaki]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[edgarfgp]

@psfinaki Out of curiosity would this help with #16309 ?

[brianrourkeboll]

@edgarfgp

Out of curiosity would this help with #16309 ?

I would guess that the second change here, from atomType to hashConstraint, would address this, yes:

 intersectionType:
-  | typar AMP intersectionConstraints %prec prec_no_more_attr_bindings // todo precedence
+  | typar AMP intersectionConstraints %prec prec_no_more_attr_bindings
     { let constraints, mAmpersands = $3
       SynType.Intersection(Some $1, List.rev constraints, lhs parseState, { AmpersandRanges = rhs parseState 2 :: List.rev mAmpersands }) }

-  | atomType AMP intersectionConstraints %prec prec_no_more_attr_bindings // todo precedence
+  | hashConstraint AMP intersectionConstraints %prec prec_no_more_attr_bindings
     { let constraints, mAmpersands = $3
       SynType.Intersection(None, $1 :: List.rev constraints, lhs parseState, { AmpersandRanges = rhs parseState 2 :: List.rev mAmpersands }) }

[psfinaki]

@edgarfgp looks like it does fix that one as well!

I will add more tests then, thanks for pointing out :)

[edgarfgp]

@edgarfgp looks like it does fix that one as well!

I will add more tests then, thanks for pointing out :)

Awesome to finally tackle all of these issues.

[T-Gro]

(auto merge had some error, merged manually)