Edges/Shifts can be float or string

[stanminlee]

Edges/Shifts were only valid for floats, changed so that they are valid for tuples of strings now. Updated test to test these changes.

---

Important

LibertyReader.cc now supports edges and shifts as floats or strings, with tests updated accordingly.

• Behavior:
  • visitShifts() and visitEdges() in LibertyReader.cc now accept both float and string values, converting strings to floats.
  • Error if shifts or edges are not complex attributes or do not have exactly 3 values.
• Tests:
  • Update generated_clock.lib to include string values for edges and shifts.
  • Update generated_clock.ok to reflect changes, including warning for unsupported shifts.

^{This description was created by for e232c3e. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to e232c3e in 22 seconds. Click for details.

• Reviewed 104 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 0 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_mXjTyxrRydBDgO73

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[greptile-apps]

Greptile Summary

adds support for parsing edges and shifts attributes as strings in addition to floats for Liberty library generated clock definitions. the PR converts string values to floats using strtof, but has critical implementation issues:

• Critical bug in visitEdges: removed the float-to-int conversion that existed in original code. edges is stored as IntSeq * (integer sequence) but now receives float values directly
• No string conversion validation: uses strtof with nullptr which silently converts invalid strings like "abc" to 0.0 without error detection
• Memory leaks: when invalid value types are encountered, allocated memory isn't freed before calling libError
• Minor improvement: error messages changed from "is not" to "must be" for better clarity

the test file validates the functionality with mixed string/numeric values, but the implementation needs fixes before merging

Confidence Score: 1/5

• This PR has critical bugs that will cause runtime errors and memory leaks
• Score reflects critical logic error in visitEdges (missing int conversion causing type mismatch), lack of string validation leading to silent failures, and memory leaks in error paths. These issues will cause incorrect behavior and potential crashes
• liberty/LibertyReader.cc requires immediate attention to fix the missing int conversion, add string validation, and plug memory leaks

Important Files Changed

Filename	Overview
liberty/LibertyReader.cc	adds string-to-float conversion for edges/shifts attributes, but missing critical int conversion in visitEdges and error validation for string parsing, plus memory leaks
test/generated_clock.lib	test case updated to use mixed string/numeric values for edges and shifts attributes
test/generated_clock.ok	updates expected line number in warning message due to test file changes

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Parse edges/shifts attribute] --> B{Is Complex?}
    B -->|No| C[Error: Must be complex]
    B -->|Yes| D[Create sequence]
    D --> E[Iterate values]
    E --> F{Value type?}
    F -->|Float| G[Get float value]
    F -->|String| H[Convert string to float]
    F -->|Other| I[Error + Memory leak]
    G --> J{edges or shifts?}
    H --> J
    J -->|edges| K[Cast to int - MISSING]
    J -->|shifts| L[Push float]
    K --> M[Push to sequence]
    L --> M
    M --> N{Size == 3?}
    N -->|No| O[Error: Must have 3 values]
    N -->|Yes| P[Set on generated_clock]

Loading

_{Last reviewed commit: e232c3e}

[greptile-apps]

_{3 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

strtof with nullptr as the second parameter doesn't validate conversion errors. invalid strings like "abc" will silently convert to 0.0. see line 5282 in this file for proper error handling pattern with char *end