Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up error messages for address of to non-function-pointer types. #44613

Merged
merged 5 commits into from
May 28, 2020
Merged

Clean up error messages for address of to non-function-pointer types. #44613

merged 5 commits into from
May 28, 2020

Conversation

333fred
Copy link
Member

@333fred 333fred commented May 28, 2020

  • Correctly error on direct cast to non-func ptr type
  • Use better error message test.

Fixes #44489.

@333fred
Clean up error messages for address of to non-function-pointer types.
ac10559 
* Correctly error on direct cast to non-func ptr type
* Use better error message test.

Fixes dotnet#44489.
@333fred 333fred requested a review from a team as a code owner May 28, 2020 00:58
@333fred 333fred requested a review from AlekseyTs May 28, 2020 00:58
@333fred
Copy link
Member Author

333fred commented May 28, 2020

@dotnet/roslyn-compiler @AlekseyTs for review of this small PR.

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 28, 2020 •
edited
Loading

It looks like there are legitimate test failures. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 28, 2020 •
edited
Loading

Done with review pass (iteration 2) #Closed

@333fred
PR Feedback:
ac66b47 
* Add additional tests.
* Handle new GetTypeInfo case revealed by tests.
* Rename variables for clarity.
@333fred
Copy link
Member Author

333fred commented May 28, 2020

@AlekseyTs addressed feedback.

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

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 28, 2020 •
edited
Loading

Done with review pass (iteration 3) #Closed

@333fred
Copy link
Member Author

333fred commented May 28, 2020

@AlekseyTs this is ready for another look.

AlekseyTs
AlekseyTs approved these changes May 28, 2020
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (iteration 5)

cston
cston reviewed May 28, 2020
View reviewed changes
src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
@@ -1806,6 +1806,8 @@ internal enum ErrorCode
ERR_InvalidFuncPointerReturnTypeModifier = 8797,
ERR_DupReturnTypeMod = 8798,
ERR_AddressOfMethodGroupInExpressionTree = 8799,
ERR_CannotConvertAddressOfToDelegate = 8800,
ERR_AddressOfToNonFunctionPointer = 8801,
Copy link
Member

@cston cston May 28, 2020 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error codes appear to be used already. (Most are used in master and 8801 is used in #44636.) #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm already going to have to resolve conflicts here, one or two more isn't particularly more difficult.

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

cston
cston reviewed May 28, 2020
View reviewed changes
src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -6160,4 +6160,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="OutIsNotValidForReturn" xml:space="preserve">
<value>'RefKind.Out' is not a valid ref kind for a return type.</value>
</data>
<data name="ERR_CannotConvertAddressOfToDelegate" xml:space="preserve">
<value>Cannot convert a &amp; method group '{0}' to delegate type '{0}'.</value>
Copy link
Member

@cston cston May 28, 2020 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convert a & method group [](start = 18, length = 28)

This message uses "convert a & method group" while the message below uses "convert & method group". We should be consistent. Consider dropping the "a" since it's not clear whether it should be "a" or "an". #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll correct this in my IDE pr I have out now.

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

@333fred 333fred merged commit dd42cea into dotnet:features/function-pointers May 28, 2020
@333fred 333fred deleted the void-cast branch May 28, 2020 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cston cston cston approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

Assignees
No one assigned
Labels
Area-Compilers Feature - Function Pointers Adding Function Pointers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@333fred @AlekseyTs @cston