Indentation within generate construct after always block is wrong if generate/endgenerate omitted

[veripoolbot]

---

Author Name: Dan Hopper
Original Redmine Issue: 1257 from https://www.veripool.org

---

There's an interesting difference in indentation in the below example module. The assign statement after an always block within a generate construct will be correctly indented if it's within a generate region (explicit generate/endgenerate lines), and incorrectly indented if the optional generate region (generate/endgenerate) is omitted.

If you load the below snippet in and reformat, the commented assign statement will be (incorrectly) indented to column 0. If you remove the comments on the generate/endgenerate pairs, it will indent properly. According to SV 1800-2012, the generate/endgenerate pair is optional, so ideally it would treat both versions identically.

I'm not proficient in Lisp so it wasn't immediately obvious to me how to fix the .el file to eliminate.

This occurs with verilog-mode-2017-12-21-39c77b2-vpo.el and also at least as far back as verilog-mode-842.el.

Thanks,
Dan

module t1
 (
 );

genvar pipe;
logic [1:0] v;
logic x;

//generate
for (pipe=0; pipe<2; pipe++) begin : v_bl
     always_comb begin
         assign v[pipe] = '0;
     end
     assign x = '0;  // will be incorrectly indented to column 0
end
//endgenerate

endmodule

[veripoolbot]

---

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-01-03T13:50:18Z

---

Seems so. The indentation code is generally maintained by patches, so if you could contribute a fix that would be great, otherwise it might be a while before it is fixed.

[veripoolbot]

---

Original Redmine Comment
Author Name: David Rogoff
Original Date: 2019-05-10T01:04:00Z

---

I just hit this. What's really weird is if I have the same generate loop twice but have another one in the middle with generate/endgenerate, the first loop will be messed up but the 2nd, identical copy will be indented correctly!

So, still no fix after a year? Wish my elisp skill was anywhere near good enough to help fix this.

David

module indent;

    // CASE 1 indented wrong.  See end of module
    for (genvar aa = 0; aa < FF; aa++) begin : gen_hh
       for (genvar bb = 0; bb < FF; bb++) begin : gen_ii
          if (`asdf [aa]) begin : gen_jj

             always_ff @ (negedge cc [aa][bb]) begin
                if (dd [aa][bb])) begin
                   for (uint_t d_idx = 0; d_idx < 16; d_idx++)
                     ee [aa][bb].push_front (tx_dfe_out [aa][bb]  [15 - d_idx]);
                end
             end

    always_ff @ (posedge hs_clk) begin
       ee_size [aa][bb] = ee [aa][bb].size();
       if (ee_size [aa][bb] > 0)
         gg [aa][bb] <= ee [aa][bb].pop_front;
    end

end : gen_jj
       end : gen_ii
    end : gen_hh



    // this indents correctly without generate/endgenerate
    for (genvar aa = 0; aa < FF; aa++) begin : gen_hh
       for (genvar bb = 0; bb < FF; bb++) begin : gen_ii
          if (`asdf [aa]) begin : gen_jj
             assign a[aa][bb] = aa + bb;
             assign b[aa][bb] = aa + bb;
          end : gen_jj
       end : gen_ii
    end : gen_hh


    // this works now with gen/endgen
    generate
       for (genvar aa = 0; aa < FF; aa++) begin : gen_hh
          for (genvar bb = 0; bb < FF; bb++) begin : gen_ii
             always @ (negedge cc)
               a = 5;
             always @ (negedge dd)
               w = 2;
          end : gen_ii
       end : gen_hh
    endgenerate


    // CASE 1 again. No change but works - apparently since verilog-mode hit a generate statement above this line
    for (genvar aa = 0; aa < FF; aa++) begin : gen_hh
       for (genvar bb = 0; bb < FF; bb++) begin : gen_ii
          if (`asdf [aa]) begin : gen_jj

             always_ff @ (negedge cc [aa][bb]) begin
                if (dd [aa][bb])) begin
                   for (uint_t d_idx = 0; d_idx < 16; d_idx++)
                     ee [aa][bb].push_front (tx_dfe_out [aa][bb]  [15 - d_idx]);
                end
             end

             always_ff @ (posedge hs_clk) begin
                ee_size [aa][bb] = ee [aa][bb].size();
                if (ee_size [aa][bb] > 0)
                  gg [aa][bb] <= ee [aa][bb].pop_front;
             end

          end : gen_jj
       end : gen_ii
    end : gen_hh

endmodule : indent

[veripoolbot]

---

Original Redmine Comment
Author Name: David Rogoff
Original Date: 2019-05-10T01:06:21Z

---

Even stranger - If I just put

    generate
    endgenerate

near the top of my module, everything after is indented correctly.

Good work-around for now and maybe good clue for finding the verilog-mode problem!

David

[punzik]

This bug can be fixed as shown below, but I am not sure if this is correct from an architectural point of view.

diff --git a/verilog-mode.el b/verilog-mode.el
index 8370d4a..248c1f5 100644
--- a/verilog-mode.el
+++ b/verilog-mode.el
@@ -4759,7 +4759,7 @@ More specifically, after a generate and before an endgenerate."
 	(while (and
 		(/= nest 0)
 		(verilog-re-search-backward
-                "\\<\\(module\\)\\|\\(connectmodule\\)\\|\\(generate\\)\\|\\(endgenerate\\)\\>" nil 'move)
+                "\\<\\(module\\)\\|\\(connectmodule\\)\\|\\(generate\\)\\|\\(endgenerate\\)\\|\\(if\\)\\>" nil 'move)
 		(cond
 		 ((match-end 1) ; module - we have crawled out
 		  (throw 'done 1))
@@ -4768,7 +4768,9 @@ More specifically, after a generate and before an endgenerate."
                 ((match-end 3) ; generate
 		  (setq nest (1- nest)))
                 ((match-end 4) ; endgenerate
-		  (setq nest (1+ nest))))))))
+		  (setq nest (1+ nest)))
+                ((match-end 5) ; if
+		  (setq nest (1- nest))))))))
     (= nest 0) )) ; return nest

 (defun verilog-in-fork-region-p ()

[wsnyder]

I thought that might mess up some normal "if" indentation, but it seems OK.

If [generate] "if" is there, then probably [generate] "case" also needs to be?

Could you make a pull request including this and test cases for the if/case?

[punzik]

Could you make a pull request including this and test cases for the if/case?

Yes, sure.

[gmlarumbe]

Seems to be fixed in 9def905