Backlog/lib/sdl3/parser/PARSER_FIX_PLAN.md

12 KiB

SDL3 Parser Fix Plan - Final Version

Executive Summary

Fix the SDL3 C header parser to generate valid, idiomatic Zig code matching existing conventions in the codebase.

Issues Identified

Priority Issue Impact Status
CRITICAL Flag definitions not captured Generated flags are empty/unusable Not Fixed
CRITICAL Invalid Zig identifiers (start with numbers) Generated code doesn't compile Not Fixed
HIGH Incorrect naming conventions Doesn't match existing codebase style Not Fixed

Root Cause Analysis

Issue 1: Empty Flag Structures

Problem: Parser generates:

pub const GPUTextureUsageFlags = packed struct(u32) {
    pad0: u31 = 0,
    rsvd: bool = false,
};

Expected:

pub const GPUTextureUsageFlags = packed struct(u32) {
    textureusageSampler: bool = false,
    textureusageColorTarget: bool = false,
    // ... 7 flags total
    pad0: u24 = 0,
    rsvd: bool = false,
};

Root Cause:

  • scanFlagTypedef() in patterns.zig:379
  • After reading typedef Uint32 SDL_GPUTextureUsageFlags;, scanner position is at newline
  • Loop tries matchPrefix("#define ") which fails immediately (looking at \n, not #)
  • Returns empty flags array

Source Header:

typedef Uint32 SDL_GPUTextureUsageFlags;

#define SDL_GPU_TEXTUREUSAGE_SAMPLER (1u << 0)
#define SDL_GPU_TEXTUREUSAGE_COLOR_TARGET (1u << 1)
// ...

Issue 2: Invalid Identifiers

Problem: Parser generates:

pub const GPUIndexElementSize = enum(c_int) {
    16bit,  // ERROR: Can't start with number!
    32bit,
};

pub const GPUTextureType = enum(c_int) {
    2d,      // ERROR: Can't start with number!
    2dArray,
    3d,
    // ...
};

Root Cause:

  • detectCommonPrefix() strips SDL_GPU_INDEXELEMENTSIZE_ from SDL_GPU_INDEXELEMENTSIZE_16BIT
  • Leaves 16BIT which becomes 16bit (invalid)
  • Need to keep type name prefix to avoid numeric start

Issue 3: Naming Convention Mismatch

Current parser output:

  • SDL_GPU_PRIMITIVETYPE_TRIANGLELISTtrianglelist
  • SDL_GPU_LOADOP_LOADload

Existing codebase:

  • SDL_GPU_PRIMITIVETYPE_TRIANGLELISTprimitivetypeTrianglelist
  • SDL_GPU_LOADOP_LOADloadopLoad

Pattern Rule: After stripping SDL_GPU_, use everything up to last underscore as lowercase prefix, then camelCase the remainder.

Example: PRIMITIVETYPE_TRIANGLELIST

  • Before last _: PRIMITIVETYPEprimitivetype (all lowercase)
  • After last _: TRIANGLELISTTrianglelist (capitalize first letter, rest lowercase)
  • Result: primitivetypeTrianglelist

Solution Design

Fix 1: Add Whitespace Skipping to Flag Scanner

File: patterns.zig Function: scanFlagTypedef() at line ~375-396 Change: Add helper function and use it before the #define scanning loop

// New helper function (add after skipLine())
fn skipWhitespace(self: *Scanner) void {
    while (self.pos < self.source.len) {
        const c = self.source[self.pos];
        if (c == ' ' or c == '\t' or c == '\n' or c == '\r') {
            self.pos += 1;
        } else {
            break;
        }
    }
}

Modification to scanFlagTypedef():

// Now collect following #define lines
var flags = try std.ArrayList(FlagValue).initCapacity(self.allocator, 10);

// Skip any whitespace/newlines before looking for #define
self.skipWhitespace();  // <-- ADD THIS LINE

// Look ahead for #define lines
while (!self.isAtEnd()) {
    const define_start = self.pos;
    if (!self.matchPrefix("#define ")) {
        self.pos = define_start;
        break;
    }
    // ... rest unchanged
}

Fix 2: Rewrite Naming Convention Logic

File: naming.zig Functions: Rewrite detectCommonPrefix() and enumValueToZig()

Strategy:

  1. Only strip the SDL_GPU_ or SDL_ prefix (not the type name)
  2. Split at last underscore to separate type from value
  3. Type part = all lowercase
  4. Value part = capitalize first letter only
  5. Concatenate

New Implementation:

/// Detect common prefix in a list of names
/// For SDL3, this should only strip the SDL_GPU_ or SDL_ prefix,
/// NOT the type name portion
pub fn detectCommonPrefix(names: []const []const u8, allocator: Allocator) ![]const u8 {
    if (names.len == 0) return try allocator.dupe(u8, "");
    
    // For SDL3, we want to find the "SDL_GPU_" or "SDL_" prefix
    // but NOT include the type name part
    
    const first = names[0];
    
    // Find "SDL_GPU_" or "SDL_" prefix
    if (std.mem.startsWith(u8, first, "SDL_GPU_")) {
        return try allocator.dupe(u8, "SDL_GPU_");
    } else if (std.mem.startsWith(u8, first, "SDL_")) {
        return try allocator.dupe(u8, "SDL_");
    }
    
    return try allocator.dupe(u8, "");
}

/// Convert enum value name to Zig using the "last underscore" rule
/// SDL_GPU_PRIMITIVETYPE_TRIANGLELIST -> primitivetypeTrianglelist
/// SDL_GPU_TEXTURETYPE_2D_ARRAY -> texturetype2dArray
pub fn enumValueToZig(c_name: []const u8, prefix: []const u8, allocator: Allocator) ![]const u8 {
    // Remove SDL_GPU_ or SDL_ prefix
    var name = c_name;
    if (std.mem.startsWith(u8, name, prefix)) {
        name = name[prefix.len..];
    }
    
    // Find last underscore: splits type name from value
    // e.g., "PRIMITIVETYPE_TRIANGLELIST" -> "PRIMITIVETYPE" + "TRIANGLELIST"
    const last_underscore = std.mem.lastIndexOfScalar(u8, name, '_');
    
    if (last_underscore) |pos| {
        const type_part = name[0..pos];      // "PRIMITIVETYPE"
        const value_part = name[pos + 1..];  // "TRIANGLELIST"
        
        // Convert type_part to all lowercase
        var result = try allocator.alloc(u8, name.len - 1); // -1 for removed underscore
        errdefer allocator.free(result);
        
        var result_idx: usize = 0;
        
        // Type part: all lowercase
        for (type_part) |c| {
            result[result_idx] = std.ascii.toLower(c);
            result_idx += 1;
        }
        
        // Value part: first letter uppercase, rest lowercase
        for (value_part, 0..) |c, i| {
            if (i == 0) {
                result[result_idx] = std.ascii.toUpper(c);
            } else {
                result[result_idx] = std.ascii.toLower(c);
            }
            result_idx += 1;
        }
        
        return result;
    } else {
        // No underscore found - just convert to lowercase
        // This handles single-word enum values
        return try screaminToLowerCamel(name, allocator);
    }
}

Update flagNameToZig(): Same logic as enums

pub fn flagNameToZig(c_name: []const u8, prefix: []const u8, allocator: Allocator) ![]const u8 {
    // Flags use same naming convention as enums
    return enumValueToZig(c_name, prefix, allocator);
}

Fix 3: Update Tests

File: naming.zig Update test at line 146-154:

test "enum value to Zig" {
    // Test basic enum value
    const result1 = try enumValueToZig(
        "SDL_GPU_PRIMITIVETYPE_TRIANGLELIST",
        "SDL_GPU_",
        std.testing.allocator,
    );
    defer std.testing.allocator.free(result1);
    try std.testing.expectEqualStrings("primitivetypeTrianglelist", result1);
    
    // Test numeric value
    const result2 = try enumValueToZig(
        "SDL_GPU_SAMPLECOUNT_1",
        "SDL_GPU_",
        std.testing.allocator,
    );
    defer std.testing.allocator.free(result2);
    try std.testing.expectEqualStrings("samplecount1", result2);
    
    // Test with numbers in middle
    const result3 = try enumValueToZig(
        "SDL_GPU_TEXTURETYPE_2D_ARRAY",
        "SDL_GPU_",
        std.testing.allocator,
    );
    defer std.testing.allocator.free(result3);
    try std.testing.expectEqualStrings("texturetype2dArray", result3);
    
    // Test flag name
    const result4 = try enumValueToZig(
        "SDL_GPU_TEXTUREUSAGE_SAMPLER",
        "SDL_GPU_",
        std.testing.allocator,
    );
    defer std.testing.allocator.free(result4);
    try std.testing.expectEqualStrings("textureusageSampler", result4);
}

test "detect common prefix" {
    const names = [_][]const u8{
        "SDL_GPU_PRIMITIVETYPE_TRIANGLELIST",
        "SDL_GPU_PRIMITIVETYPE_TRIANGLESTRIP",
        "SDL_GPU_PRIMITIVETYPE_LINELIST",
    };

    const prefix = try detectCommonPrefix(&names, std.testing.allocator);
    defer std.testing.allocator.free(prefix);
    // Should only strip SDL_GPU_, not the type name
    try std.testing.expectEqualStrings("SDL_GPU_", prefix);
}

Implementation Plan

Phase 1: Fix Critical Flag Scanning Bug (30 min)

  1. Add skipWhitespace() helper to patterns.zig
  2. Call it in scanFlagTypedef() before the #define loop
  3. Test: zig build run -- ../SDL/include/SDL3/SDL_gpu.h | grep -A 10 "GPUTextureUsageFlags"
  4. Verify flags are populated

Phase 2: Fix Naming Conventions (45 min)

  1. Rewrite detectCommonPrefix() in naming.zig to only strip SDL_GPU_/SDL_
  2. Rewrite enumValueToZig() to implement last-underscore rule
  3. Update unit tests to match new behavior
  4. Test: zig build test should pass
  5. Test: Generate gpu.zig and check naming matches

Phase 3: Validation (30 min)

  1. Run parser on SDL_gpu.h: zig build run -- ../SDL/include/SDL3/SDL_gpu.h > /tmp/new_gpu.zig
  2. Try compiling the output: zig ast-check /tmp/new_gpu.zig
  3. Compare with existing: diff /home/sear/Backlog/lib/sdl3/src/gpu.zig /tmp/new_gpu.zig
  4. Verify:
    • No syntax errors
    • All flag fields present
    • All enum values valid (no numeric prefixes)
    • Naming conventions match existing file

Phase 4: Documentation (15 min)

  1. Update naming.zig documentation
  2. Add comments explaining the "last underscore" rule
  3. Document the whitespace skipping fix

Expected Outcomes

Before Fix

// Empty flags
pub const GPUTextureUsageFlags = packed struct(u32) {
    pad0: u31 = 0,
    rsvd: bool = false,
};

// Invalid identifiers
pub const GPUTextureType = enum(c_int) {
    2d,      // COMPILE ERROR
    2dArray,
    3d,
};

// Wrong naming
pub const GPUPrimitiveType = enum(c_int) {
    trianglelist,
    trianglestrip,
};

After Fix

// Properly populated flags
pub const GPUTextureUsageFlags = packed struct(u32) {
    textureusageSampler: bool = false,
    textureusageColorTarget: bool = false,
    textureusageDepthStencilTarget: bool = false,
    textureusageGraphicsStorageRead: bool = false,
    textureusageComputeStorageRead: bool = false,
    textureusageComputeStorageWrite: bool = false,
    textureusageComputeStorageSimultaneousReadWrite: bool = false,
    pad0: u24 = 0,
    rsvd: bool = false,
};

// Valid identifiers
pub const GPUTextureType = enum(c_int) {
    texturetype2d,       // Valid!
    texturetype2dArray,
    texturetype3d,
    texturetypeCube,
    texturetypeCubeArray,
};

// Correct naming convention
pub const GPUPrimitiveType = enum(c_int) {
    primitivetypeTrianglelist,
    primitivetypeTrianglestrip,
    primitivetypeLinelist,
    primitivetypeLinestrip,
    primitivetypePointlist,
};

Risk Assessment

Risk Likelihood Impact Mitigation
Breaking existing tests High Medium Update tests incrementally
Edge cases in naming Medium Low Add comprehensive test cases
Performance impact Low Low Changes are O(n) string operations
Regression in other headers Low Medium Test with multiple SDL3 headers

Success Criteria

  • Parser generates valid Zig code (compiles without errors)
  • All flags have proper fields (not empty)
  • No enum values start with numbers
  • Naming matches existing gpu.zig conventions
  • All unit tests pass
  • Integration test: parser output matches existing file structure
  • Memory leaks remain fixed (verified with GPA)

Estimated Time: 2 hours total