Add parsing for function pointer types

[333fred]

@dotnet/roslyn-compiler @AlekseyTs for review.

Latest grammar: https://github.com/dotnet/csharplang/blob/master/proposals/function-pointers.md#function-pointer-syntax

[CyrusNajmabadi]

LGTM. However, there are several things i think can/should be done to make the parser more production-ready for the types of cases we should expect users to be running into. In general, this means not overfitting the syntax model, and it means detecting and being resilient to expected user mistakes. #Resolved

[gafter]

A couple of comments inline. To summarize:

1. Probably check for < as part of knowing we have a function type in IsFunctionPointerStart
2. ScanFunctionPointerType should say not a type if the structure is wrong (i.e. no <)

[CyrusNajmabadi]

c

[333fred]

Pushed an updated that addressed a lot of feedback. There will still be a test failure as I haven't updated the AllInOneCSharp file, I will do this later. #Resolved

[AlekseyTs]

Done with review pass (iteration 4) #Closed

[333fred]

@AlekseyTs addressed feedback, thanks!

---

In reply to: 557336082 [](ancestors = 557336082)

[sharwell]

IDE change (trivial change in features layer) is fine

[AlekseyTs]

Done with review pass (iteration 5) #Closed

[333fred]

@AlekseyTs addressed feedback.

---

In reply to: 558364765 [](ancestors = 558364765)

[AlekseyTs]

CurrentToken.ValueText [](start = 20, length = 22)

I think this change should be backed by a test that observes the difference in behavior.

[333fred]

I think this change should be backed by a test that observes the difference in behavior.

After adding a test, I've gone back to Text instead of ValueText, as we don't want to treat @cdecl as a calling convention, given that it's been escaped.

---

In reply to: 350896749 [](ancestors = 350896749)

[AlekseyTs]

After adding a test, I've gone back to Text instead of ValueText, as we don't want to treat @cdecl as a calling convention, given that it's been escaped.

It feels like we are somewhat inconsistent:

• delegate* @cdecl - return false
• delegate* @cdecl < - return true

Perhaps we should ignore all escaped identifiers? I think it would be fine to add a prototype comment and follow up on this later.

---

In reply to: 350976752 [](ancestors = 350976752,350896749)

[333fred]

It feels like we are somewhat inconsistent

I don't think that that's any more inconsistent than a non-escaped regular identifier that isn't a known convention, like MyType. We special case things that we know are valid calling conventions, but the convention was specifically escaped here so we treat it as any other invalid identifier.

---

In reply to: 351425780 [](ancestors = 351425780,350976752,350896749)

[AlekseyTs]

I don't think that that's any more inconsistent than a non-escaped regular identifier that isn't a known convention, like MyType. We special case things that we know are valid calling conventions, but the convention was specifically escaped here so we treat it as any other invalid identifier.

Then why do we treat @cdecl as possibly valid calling convention when it is followed by <. If grammar doesn't allow escaped identifiers, then we should not accept them.

---

In reply to: 351441102 [](ancestors = 351441102,351425780,350976752,350896749)

[AlekseyTs]

LGTM (iteration 6) modulo the test suggestion.

[gafter]

[AlekseyTs]

        UsingStatement("delegate* @cdecl>", options: TestOptions.RegularPreview,

Consider adding a test for delegate* @cdecl < ...

---

Refers to: src/Compilers/CSharp/Test/Syntax/Parsing/FunctionPointerTests.cs:815 in 27e8523. [](commit_id = 27e8523, deletion_comment = False)

[AlekseyTs]

LGTM (iteration 7).

[333fred]

        UsingStatement("delegate* @cdecl>", options: TestOptions.RegularPreview,

That's in my plan for binding tests, but it's not particularly interesting on a syntax level because it's just like any other invalid calling convention.

---

In reply to: 559191659 [](ancestors = 559191659)

---

Refers to: src/Compilers/CSharp/Test/Syntax/Parsing/FunctionPointerTests.cs:815 in 3231dcc. [](commit_id = 3231dcc, deletion_comment = False)

[AlekseyTs]

        UsingStatement("delegate* @cdecl>", options: TestOptions.RegularPreview,

but it's not particularly interesting on a syntax level because it's just like any other invalid calling convention.

I find the suggested scenario interesting from the parsing point of view because of the contrast to the behavior with scenario in this test. Please consider adding one.

---

In reply to: 559204507 [](ancestors = 559204507,559191659)

---

Refers to: src/Compilers/CSharp/Test/Syntax/Parsing/FunctionPointerTests.cs:815 in 27e8523. [](commit_id = 27e8523, deletion_comment = False)