fix!: ShapeTag generation for nullable non-blittable types - #99
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
sator-imaging
changed the title
| if (underlying is not INamedTypeSymbol namedUnderlying | ||
| || namedUnderlying.TypeKind != TypeKind.Struct | ||
| || TryGetBlittableStructByteCount(namedUnderlying, out _)) |
There was a problem hiding this comment.
I dislike this. I said target blittable struct. Don't append ? to something.
There was a problem hiding this comment.
Updated the logic to specifically target struct types when checking for blittability before deciding whether to omit the '?' suffix.
| } | ||
| else | ||
| { | ||
| shapeTagBuilder.Append('?'); |
There was a problem hiding this comment.
What's this? I don't need fallback. MUST TARGET THE SPECIFIC SHAPE.
There was a problem hiding this comment.
Refactored the check to explicitly test !IsNonBlittableStruct(underlying), removing fallback branches and directly targeting non-blittable struct types.
There was a problem hiding this comment.
Don't introduce unnecessary code shape change!!
There was a problem hiding this comment.
Refactored the code to avoid unnecessary helpers or branching logic, returning early when the underlying type of Nullable is a non-blittable struct.
sator-imaging
changed the title
ShapeTag generation for nullable non-blittable types
1 participant
Omit the '?' suffix in ShapeTag generation for reference types (string, arrays, classes) and non-blittable structs. Only value types (primitives, enums, blittable structs) retain the '?' suffix when wrapped in Nullable. Update SchemaSignatureTestsModelView shape tag expectation in tests.
PR created automatically by Jules for task 587085618536675106 started by @sator-imaging