Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Move from calling Create to calling a specific constructor#53262

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@333fred
Copy link
Member

@333fred333fred commentedMay 7, 2021•
edited
Loading

Couple of smaller changes:

  1. Call a constructor, rather than a create method.
  2. Move from calling types builders to handlers, to keep in sync withAPI Proposal: C# 10 interpolated strings support, part 3 runtime#51962.

@ghostghost added the Area-Compilers labelMay 7, 2021
@333fred333fred changed the titleconstructor call Move from calling Create to calling a specific constructorMay 7, 2021
@333fred333fred marked this pull request as ready for reviewMay 7, 2021 21:49
@333fred333fred requested a review froma team as acode ownerMay 7, 2021 21:49
@333fred
Copy link
MemberAuthor

@chsienki@AlekseyTs for review.

1 similar comment
@333fred
Copy link
MemberAuthor

@chsienki@AlekseyTs for review.

@chsienkichsienki self-assigned thisMay 11, 2021
Copy link
Member

@chsienkichsienki left a comment•
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@333fred
Copy link
MemberAuthor

@AlekseyTs for review.

/// <summary>
/// Helper method to create a synthesized constructor invocation.
/// </summary>
privateBoundExpressionMakeClassCreationExpression(
Copy link
Contributor

@AlekseyTsAlekseyTsMay 12, 2021•
edited by 333fred
Loading

Choose a reason for hiding this comment

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

MakeClassCreationExpression

The naming might be somewhat confusing. The helper isn't just for classes right? Structures are good too? You are probably follwong the pattern of BindClassCreationExpression name, however that function wasn't meant to be used as a general helper. Consider using name that simply indicates that an instance is created by using constructor. Then arguments start making more sense, etc. "MakeConstructorInvocation"? #Pending

}

privateBoundExpressionMakeBadExpressionForObjectCreation(SyntaxNodenode,TypeSymboltype,AnalyzedArgumentsanalyzedArguments,InitializerExpressionSyntaxinitializerOpt,SyntaxNodetypeSyntax,BindingDiagnosticBagdiagnostics)
/// <param name="typeSyntax">If <paramref name="initializerOpt"/> is not null, this should not be null.</param>
Copy link
Contributor

@AlekseyTsAlekseyTsMay 12, 2021•
edited by 333fred
Loading

Choose a reason for hiding this comment

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

If is not null, this should not be null.

Shouldn't be null if <paramref name="initializerOpt"/> is not null.? #Pending


BoundCall?createExpression=BindCreateCall(unconvertedInterpolatedString.Syntax,interpolatedStringBuilderType,arguments,diagnostics);
// PROTOTYPE(interp-string): Support optional out param for whether the builder was created successfully and passing in other required args
BoundExpression?createExpression=MakeClassCreationExpression(interpolatedStringBuilderType,arguments,unconvertedInterpolatedString.Syntax,diagnostics);
Copy link
Contributor

@AlekseyTsAlekseyTsMay 12, 2021•
edited by 333fred
Loading

Choose a reason for hiding this comment

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

MakeClassCreationExpression

BindCreateCall used to report an error unless it was producing a BoundCall node. Should we be doing something similar here? For BoundDynamicObjectCreationExpression, for example? #Pending

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I have a prototype comment in LocalLowering to handle dynamic at a later point. I'll add assert for now that it either HasErrors or is a constructor.


BoundCall?createExpression=BindCreateCall(unconvertedInterpolatedString.Syntax,interpolatedStringBuilderType,arguments,diagnostics);
// PROTOTYPE(interp-string): Support optional out param for whether the builder was created successfully and passing in other required args
BoundExpression?createExpression=MakeClassCreationExpression(interpolatedStringBuilderType,arguments,unconvertedInterpolatedString.Syntax,diagnostics);
Copy link
Contributor

@AlekseyTsAlekseyTsMay 12, 2021•
edited
Loading

Choose a reason for hiding this comment

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

MakeClassCreationExpression

Does this handle optional parameters? #Closed

Copy link
Contributor

@AlekseyTsAlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 2)

@333fred
Copy link
MemberAuthor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@333fred333fredenabled auto-merge (squash)May 12, 2021 21:33
@333fred333fred merged commita4ebff3 intodotnet:features/interpolated-stringMay 12, 2021
@333fred333fred deleted the constructor-call branchMay 13, 2021 18:53
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@AlekseyTsAlekseyTsAlekseyTs approved these changes

@chsienkichsienkichsienki approved these changes

Assignees

@chsienkichsienki

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@333fred@AlekseyTs@chsienki
[8]ページ先頭
©2009-2025 Movatter.jp