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

Make GenerateType work off of an Incomplete-Member. #14778

Merged
merged 2 commits into from
Oct 28, 2016
Merged

Make GenerateType work off of an Incomplete-Member. #14778

merged 2 commits into from
Oct 28, 2016

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/roslyn-compiler @dotnet/roslyn-ide

@CyrusNajmabadi
Copy link
Member Author

Fixes #13211

Note: this is still an issue because of #7536 @gafter Do we think we can get around to fixing the issue where the compiler does not bind incomplete members?

@CyrusNajmabadi
Copy link
Member Author

@dotnet/roslyn-compiler Please review simple change to SyntaxFacts. Thanks!

@gafter
Copy link
Member

gafter commented Oct 28, 2016

Compiler changes LGTM.

I'll try to fix #7536 sooner rather than later, but C# 7 changes have to take priority.

AlekseyTs
AlekseyTs reviewed Oct 28, 2016
View reviewed changes
src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs
@@ -195,6 +195,9 @@ public static bool IsInTypeOnlyContext(ExpressionSyntax node)

case TypedVariableComponent:
return ((TypedVariableComponentSyntax)parent).Type == node;

case IncompleteMember:
return ((IncompleteMemberSyntax)parent).Type == node;
Copy link
Contributor

Choose a reason for hiding this comment

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

return ((IncompleteMemberSyntax)parent).Type == node; [](start = 23, length = 54)

Does this change the way SemanticModel APIs behave? Please add compiler tests for them and a direct test for IsInTypeOnlyContext API. src\Compilers\CSharp\Test\Symbol\Compilation\SemanticModelGetSemanticInfoTests.cs looks like a good place.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do, thanks @AlekseyTs . If i make those changes, are you ok with this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, other than that, LGTM.

@CyrusNajmabadi CyrusNajmabadi merged commit 659689b into dotnet:master Oct 28, 2016
@CyrusNajmabadi CyrusNajmabadi deleted the generateTypeIncompleteMember branch October 28, 2016 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlekseyTs AlekseyTs AlekseyTs left review comments

Assignees

@gafter gafter

Labels
Area-Compilers cla-already-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CyrusNajmabadi @gafter @AlekseyTs @dnfclas