Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

indentation of coverpoint is incorrect if coverpoint expression is a concatenation #1321

Open
veripoolbot opened this issue Jun 20, 2018 · 4 comments
Labels

Comments

@veripoolbot
Copy link
Collaborator

@veripoolbot veripoolbot commented Jun 20, 2018


Author Name: Robert Swan
Original Redmine Issue: 1321 from https://www.veripool.org


Coverpoints on curly-brace concatenations indent incorrectly, and accumulate indent level.

module m;
    bit a, b, c;
    covergroup g;
       cp_ab: coverpoint {a,b} {
                                bins one = {1};
                                bins two = {2};
                                }
         cp_bc: coverpoint {b,c} {
                                  bins one = {1};
                                  bins two = {2};
                                  }
           endgroup
endmodule

Desired/correct indenting is seen if an iff clause is added to the coverpoints:

module m;
    bit a, b, c;
    covergroup g;
       cp_ab: coverpoint {a,b} iff (1) {
          bins one = {1};
          bins two = {2};
       }
       cp_bc: coverpoint {b,c} iff (1) {
          bins one = {1};
          bins two = {2};
       }
    endgroup
endmodule

Verilog-mode version is 2018-05-26-be4eda3-vpo.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Jun 20, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-06-20T14:57:39Z


Yes, looks wrong. The indentation code is presently without a major maintainer, so it might be a while before this is fixed unless you can provide a patch.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Jul 6, 2018


Original Redmine Comment
Author Name: Robert Swan
Original Date: 2018-07-06T01:39:18Z


The following is working for me:

verilog-mode version 2018-05-26-be4eda3-vpo:

module m;
    bit[0:0] a, b, c;
    covergroup g;
       cp_ab: coverpoint {a,b} {
                                bins one = {1};
                                bins two = {2};
                                }

         cp_ab_if_c: coverpoint {a,b} iff c {
            bins one = {1};
            bins two = {2};
         }

         cp_ab_if_c_slice: coverpoint {a,b} iff c[0] {
                                                      bins one = {1};
                                                      bins two = {2};
                                                      }

           cp_a_if_bc: coverpoint {a,b} iff {b,c} {
                                                   bins one = {1};
                                                   bins two = {2};
                                                   }

             cp_a_slice : coverpoint a[0] {
                                           bins one = {1};
                                           bins two = {2};
                                           }

               cp_a_slice_if_b : coverpoint a[0] iff b {
                  bins one = {1};
                  bins two = {2};
               }

               cp_a_if_b_slice : coverpoint a iff b[0] {
                                                        bins one = {1};
                                                        bins two = {2};
                                                        }

                 cp_a_slice_if_b_slice : coverpoint a[0] iff b[0] {
                                                                   bins one = {1};
                                                                   bins two = {2};
                                                                   }
                   endgroup
endmodule

Patch:

diff --git a/verilog-mode.el-2018-05-26-be4eda3-vpo b/verilog-mode.el-indent-fix
index f7f757f..a5a3617 100644
--- a/verilog-mode.el-2018-05-26-be4eda3-vpo
+++ b/verilog-mode.el-indent-fix
@@ -6450,6 +6450,34 @@ Return >0 for nested struct."
                                                 "\\(?:\\w+\\s-*:\\s-*\\)?\\(coverpoint\\|cross\\)"
                                                 "\\|with\\)\\>\\|" verilog-in-constraint-re)))
                             (setq pass 1)))))
+
+          ;; step backwards over coverpoint/iff on concatenation/slice
+          (catch 'coverpoint-or-iff
+            (dotimes (test-coverpoint-or-iff 4)
+              (if (looking-at "coverpoint")
+                  (progn
+                    (verilog-beg-of-statement)
+                    (setq pass 1)
+                    (throw 'coverpoint-or-iff nil)))
+              (if (equal (char-after) ?\})
+                  (progn
+                    (forward-char 1)
+                    (backward-list)
+                    (verilog-backward-sexp))
+                (if (equal (char-after) ?\[)
+                    (progn 
+                      (verilog-backward-token)
+                      (verilog-backward-sexp)
+                      )
+                  (if (or (looking-at "iff"))
+                      (progn
+                        (verilog-backward-ws&directives)
+                        (verilog-backward-token))
+                    (if (eq pass 0)
+                        (progn
+                          (verilog-backward-ws&directives)
+                          (verilog-backward-sexp))))))))
+
            (if (eq pass 0)
                (progn (goto-char pt) nil) 1)))
      ;; not


Patched:

module m;
    bit[0:0] a, b, c;
    covergroup g;
       cp_ab: coverpoint {a,b} {
          bins one = {1};
          bins two = {2};
       }

       cp_ab_if_c: coverpoint {a,b} iff c {
          bins one = {1};
          bins two = {2};
       }

       cp_ab_if_c_slice: coverpoint {a,b} iff c[0] {
          bins one = {1};
          bins two = {2};
       }

       cp_a_if_bc: coverpoint {a,b} iff {b,c} {
          bins one = {1};
          bins two = {2};
       }

       cp_a_slice : coverpoint a[0] {
          bins one = {1};
          bins two = {2};
       }

       cp_a_slice_if_b : coverpoint a[0] iff b {
          bins one = {1};
          bins two = {2};
       }

       cp_a_if_b_slice : coverpoint a iff b[0] {
          bins one = {1};
          bins two = {2};
       }

       cp_a_slice_if_b_slice : coverpoint a[0] iff b[0] {
          bins one = {1};
          bins two = {2};
       }
    endgroup
endmodule


@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Jul 12, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-07-12T12:13:43Z


Great work this passes on your new test, but if you could please download the git tree and try the self tests this fails with an error "wrong type argument: consp"

Also a nit, in Lisp rather than this

   (if (a)
       (foo)
     (if (b)
          (bar)
       (if (c)
           (baz))))

do this

   (cond
    ((a)
     (foo))
    ((b)
     (bar))
    ((c)
     (baz)))

If not clear I'll clean it up for you.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Dec 21, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-21T15:16:17Z


Still awaiting a patch+test.

@veripoolbot veripoolbot added the indents label Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.