Skip to content

Conversation

@nojaf
Copy link
Contributor

@nojaf nojaf commented Apr 26, 2022

This is a subset, the first step if you will, from #12989.
In 12989 I tried to do too much at the same time and I would like to cut it up into multiple PRs.

This first one refactors SynPat.LongIdent to SynPat.ParameterOwners and take a SynPat as the name identifier.
In practise, this can be SynPat.LongIdent or SynPat.Ident.

Later this could be extended to include SynPat.Operator and SynPat.DotGetOperator.
And after that, the captured SynOperatorName could also be introduced into SynExpr.

@dsyme, @vzarytovskii please let me know your thoughts on this.
I do believe this proposed route should be favoured over the alternative in #13023.
Replacing Ident with SynIdentOrOperatorName takes a performance hit and it is used in a lot of places where operator names will never be valid. So, I would abandon that idea altogether.

//cc @auduchinok

@nojaf nojaf marked this pull request as ready for review April 26, 2022 13:20
@nojaf
Copy link
Contributor Author

nojaf commented May 2, 2022

Closing in favour of #13057.

@nojaf nojaf closed this May 2, 2022
@nojaf nojaf deleted the synpat-parameterowner branch May 16, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant