-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Handle private properties in the compiled model #33409
Conversation
87952ec
to
8048cab
Compare
public static string GetUnsafeAccessorName(MemberInfo member) | ||
{ | ||
StringBuilder stringBuilder = new(); | ||
stringBuilder.Clear().Append("UnsafeAccessor_"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this our naming convention, or a convention recommended from somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's a specific recommended naming convention for unsafe accessor (see API docs with samples), this is also what I used on the query side when generating these.
No strong feelings of course (this is all "internal" generated code), it just seemed like a code idea to identify what these methods are etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that it generates very long names. Not a big deal, but given the size of generated code has been an issue it was something that occurred to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made it consistent with what query generates. The idea is to have a unique name that still means something. If Shay agrees we can just use the member name + uniquefier to keep it short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, am fine with whatever - remember this is just generated code so nothing's very important here (except for uniqueness).
{ | ||
if (assignmentKind is not SyntaxKind.SimpleAssignmentExpression) | ||
{ | ||
throw new NotSupportedException("Compound assignment not supported"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is from Shay's PR, I was reading this as "not implemented", we could handle compound assignments by calling the getter and then the setter
Add full support for service properties
Use a consistent pattern for unsafe accessors
Store the unsafe accessor names in an annotation
Refactor the different mechanisms for tracking variables in the generated code into a single bidirectional dictionary
Fixes #29761
Fixes #24904
Fixes #24900