Skip to content

Fix predefined macro checks in elsif branches - #132

Open
dajiaohuang wants to merge 1 commit into
dalance:masterfrom
dajiaohuang:fix/elsif-predefined-macros-20261015
Open

dajiaohuang wants to merge 1 commit into
dalance:masterfrom
dajiaohuang:fix/elsif-predefined-macros-20261015

Conversation

@dajiaohuang

Copy link
Copy Markdown

Problem

The predefined-macro check in each elsif loop uses the identifier from the opening ifdef/ifndef, rather than the identifier on the current elsif. This changes which valid source reaches the parser:

`ifdef UNDEFINED
module first; endmodule
`elsif __FILE__
module selected; endmodule
`else
module fallback; endmodule
`endif

parse_sv_str currently returns a syntax tree containing fallback, although __FILE__ is predefined and selected should be chosen. Conversely, after ifndef __FILE__, an elsif UNDEFINED is incorrectly treated as true. Both __FILE__ and __LINE__ are affected.

Change

  • Check elsifid for predefined status in both conditional-directive paths.
  • Add public parse_sv_str regression tests asserting the resulting module identifiers across both builtins and both opening directives.
  • Correct the existing FILE/LINE fixture comments and expected output: their elsif UNDEFINED branch must not be emitted. Source line counts are unchanged.

Validation

  • Both new public-API tests fail against the unchanged production base and pass with this fix.
  • cargo test passes on Ubuntu 24.04 / stable Rust 1.98.1: 127 passed, 1 pre-existing ignored.
  • Separate public-API file/string/preprocessed parsing, isolated include/macro expansion, AST traversal and source-origin checks pass; both CLI examples exercised.
  • Initial native Windows workspace tests encountered existing fixture line-ending/OS-error-message differences; full Windows acceptance is not claimed. Linux tests used canonical Git LF fixtures.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant