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.

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

Expression variables declared in a local function parameter default are given a scope #16467

Merged
merged 3 commits into from
Jan 13, 2017

Conversation

gafter
Copy link
Member

@gafter gafter commented Jan 12, 2017

Customer scenario

If you declare an expression variable (e.g. out variable) inside the default value
expression of a parameter to a local function, the IDE crashes.

Bugs this fixes:

Fixes #16167

Workarounds, if any

Don't make that mistake.

Risk

Small. Corrects a small oversight in the implementation of the interaction of new language features.

Performance impact

Trivial.

Is this a regression from a previous update?

No.

Root cause analysis:

Missing test for interaction between multiple new language features. Lack of dedicated testing team. Lack of coordinated testing role for new features and interactions between new features.

How was the bug found?

Customer reported. It occurred accidentally when the customer pasted XML inside a method body instead of inside the doc comment.

@jcouv @cston previously reviewed this fix in #16315; this moves it to the rc3 branch.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM

@gafter
Copy link
Member Author

gafter commented Jan 12, 2017

@MattGertz This is for rc3 shiproom.

@cston
Copy link
Member

cston commented Jan 12, 2017

LGTM

@MattGertz
Copy link
Contributor

Dogfood-reported, so OK to take to RC3. (Please don't add the pending label; I will do that.)

@MattGertz
Copy link
Contributor

@natidea

@natidea
Copy link
Contributor

natidea commented Jan 12, 2017

@gafter please review the "Ask Mode" tab in 367236

@jaredpar
Copy link
Member

Need clarity on the following before we can merge this change

#16315 (comment)

@natidea
Copy link
Contributor

natidea commented Jan 12, 2017

Shiproom moved to RTW. Please retarget PR

parameter binders with appropriate containing symbol
and flags.
@gafter gafter changed the base branch from dev15-rc3 to master January 12, 2017 22:54
@gafter
Copy link
Member Author

gafter commented Jan 12, 2017

@cston @AlekseyTs Please re-review one additional commit.

@gafter
Copy link
Member Author

gafter commented Jan 12, 2017

@jaredpar Pending your approval.

{
public static void Main(int arg)
{
void Local2(bool b = M(M(out int z1), z1), int s2 = z1) {}
Copy link
Contributor

@AlekseyTs AlekseyTs Jan 12, 2017

Choose a reason for hiding this comment

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

void Local2(bool b = M(M(out int z1), z1), int s2 = z1) {} [](start = 8, length = 58)

Consider adding a test where a variable declared within a default parameter value is referenced within the local function that owns the parameter. #Closed

@AlekseyTs
Copy link
Contributor

LGTM, please consider adding a test suggested in #16467 (review)

@AlekseyTs
Copy link
Contributor

LGTM

@gafter
Copy link
Member Author

gafter commented Jan 13, 2017

@MattGertz @natidea This is for RTM shiproom.

@gafter gafter modified the milestones: 2.0 (RTM), 2.0 (RC.3) Jan 13, 2017
@gafter
Copy link
Member Author

gafter commented Jan 13, 2017

I see this has been pre-approved for proactive integration into RTM per https://devdiv.visualstudio.com/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_workitems?id=367236

@gafter gafter merged commit 3e580ad into dotnet:master Jan 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VS crashes if xml is pasted in C# editor
8 participants