Merge pull request #3138 from veryl-lang/fix/struct-member-ifdef-semicolon

Fix struct/union member separator emitted outside its ifdef guard

Author: dalance

crates/emitter/src/emitter.rs

@@ -1149,6 +1149,40 @@ impl Emitter {
         }
     }
 
+    /// Pop any trailing `[Hardline, `endif]` pairs (one per group `attribute_end`)
+    /// so a separator can be emitted before the endif chain; re-emit with `.rev()`.
+    fn pop_trailing_endif_chain(&mut self) -> Vec<Doc> {
+        if matches!(self.mode, Mode::Align) {
+            return Vec::new();
+        }
+        let buf = self.doc_buffer.last_mut().unwrap();
+        let mut popped: Vec<Doc> = Vec::new();
+        loop {
+            let len = buf.len();
+            if len < 2 {
+                break;
+            }
+            let is_endif = match &buf[len - 1] {
+                Doc::Text(s) => s.as_ref() == "`endif",
+                _ => false,
+            };
+            if !is_endif {
+                break;
+            }
+            let is_hard = matches!(&buf[len - 2], Doc::Hardline);
+            if !is_hard {
+                break;
+            }
+            let endif = buf.pop().unwrap();
+            let hardline = buf.pop().unwrap();
+            // Push in reverse pair order so the final iter().rev() yields
+            // the original [Hardline, Text] sequence.
+            popped.push(endif);
+            popped.push(hardline);
+        }
+        popped
+    }
+
     fn attribute_end(&mut self) {
         match self.attribute.pop() {
             Some(AttributeType::Ifdef) => {
@@ -2931,39 +2965,7 @@ impl VerylWalker for Emitter {
 
     /// Semantic action for non-terminal 'Comma'
     fn comma(&mut self, arg: &Comma) {
-        // If the trailing Doc nodes are a sequence of
-        // `[Hardline, Doc::Text("`endif")]` pairs (one per `attribute_end`
-        // emitted for this group), pop them, emit the comma, then re-emit
-        // them so the comma sits on the line before the endif chain.
-        if matches!(self.mode, Mode::Align) {
-            self.veryl_token(&arg.comma_token);
-            return;
-        }
-        let buf = self.doc_buffer.last_mut().unwrap();
-        let mut popped: Vec<Doc> = Vec::new();
-        loop {
-            let len = buf.len();
-            if len < 2 {
-                break;
-            }
-            let is_endif = match &buf[len - 1] {
-                Doc::Text(s) => s.as_ref() == "`endif",
-                _ => false,
-            };
-            if !is_endif {
-                break;
-            }
-            let is_hard = matches!(&buf[len - 2], Doc::Hardline);
-            if !is_hard {
-                break;
-            }
-            let endif = buf.pop().unwrap();
-            let hardline = buf.pop().unwrap();
-            // Push in reverse pair order so the final iter().rev() yields
-            // the original [Hardline, Text] sequence.
-            popped.push(endif);
-            popped.push(hardline);
-        }
+        let popped = self.pop_trailing_endif_chain();
         self.veryl_token(&arg.comma_token);
         for d in popped.into_iter().rev() {
             self.emit_doc(d);
@@ -5314,17 +5316,28 @@ impl VerylWalker for Emitter {
 
     /// Semantic action for non-terminal 'StructUnionList'
     fn struct_union_list(&mut self, arg: &StructUnionList) {
+        // The ';' separators must land inside a member's `ifdef guard, like
+        // comma() does — after `endif the preprocessor would keep a stray
+        // ';' when the define is off.
         self.struct_union_group(&arg.struct_union_group);
         for x in &arg.struct_union_list_list {
+            let popped = self.pop_trailing_endif_chain();
             self.token(&x.comma.comma_token.replace(";"));
+            for d in popped.into_iter().rev() {
+                self.emit_doc(d);
+            }
             self.newline();
             self.struct_union_group(&x.struct_union_group);
         }
+        let popped = self.pop_trailing_endif_chain();
         if let Some(ref x) = arg.struct_union_list_opt {
             self.token(&x.comma.comma_token.replace(";"));
         } else {
             self.str(";");
         }
+        for d in popped.into_iter().rev() {
+            self.emit_doc(d);
+        }
     }
 
     /// Semantic action for non-terminal 'StructUnionGroup'

crates/emitter/src/tests.rs

@@ -4697,3 +4697,39 @@ fn hoisted_declaration_keeps_ifdef_guard() {
         "else-arm declaration must sit inside the guard:\n{ret}"
     );
 }
+
+#[test]
+fn struct_member_ifdef_keeps_semicolon_inside_guard() {
+    // The member-separating ';' used to be emitted after `endif, so with
+    // the define off the preprocessor kept a stray ';' inside the struct
+    // body — invalid SV.
+    let metadata = Metadata::create_default("prj").unwrap();
+
+    let code = r#"module M {
+    struct S {
+        a: logic<8>,
+        #[ifdef(DEF_A)]
+        b: logic<8>,
+        c: logic<8>,
+    }
+    var s: S;
+    var o: logic<8>;
+    always_comb {
+        s.a = 1;
+        s.c = 2;
+        #[ifdef(DEF_A)]
+        s.b = 3;
+    }
+    assign o = s.a + s.c;
+}
+"#;
+    let ret = emit(&metadata, code);
+    assert!(
+        ret.contains("logic [8-1:0] b;\n        `endif"),
+        "member ';' must land inside the ifdef guard:\n{ret}"
+    );
+    assert!(
+        !ret.contains("`endif;"),
+        "no stray ';' after `endif:\n{ret}"
+    );
+}