Merge branch 'master' into file_inclusion

.clang-tidy

@@ -34,6 +34,7 @@ Checks: >
   -readability-redundant-access-specifiers,
   -readability-use-anyofallof,
   -readability-identifier-length,
+  -readability-convert-member-functions-to-static,  # creates false positive ?
   google-*,
   -google-readability-braces-around-statements,
   -google-readability-todo,

.github/bin/build-and-test.sh

@@ -68,8 +68,9 @@ BAZEL_OPTS="${BAZEL_OPTS} --cxxopt=-Wno-array-bounds"
 # This option is only recognized by gcc
 if [[ ! "${CXX}" == clang* ]]; then
   BAZEL_OPTS="${BAZEL_OPTS} --cxxopt=-Wno-cast-function-type"       # gflags
-  # Once we use gcc-11 this might be needed for protobuf
-  #BAZEL_OPTS="${BAZEL_OPTS} --cxxopt=-Wno-stringop-overflow"
+  # Current protobuf creates a warning on newer compilers, hopefully
+  # fixed with protobuf > 21.5
+  BAZEL_OPTS="${BAZEL_OPTS} --cxxopt=-Wno-stringop-overflow"        # protobuf
 else
   # -- only recognized by clang
   # Don't rely on implicit template type deduction

.github/bin/run-clang-tidy.sh

@@ -23,7 +23,7 @@ readonly CLANG_TIDY_SEEN_CACHE=${TMPDIR}/clang-tidy-hashes.cache
 touch ${CLANG_TIDY_SEEN_CACHE}  # Just in case it is not there yet
 
 readonly FILES_TO_PROCESS=${TMPDIR}/clang-tidy-files.list
-readonly TIDY_OUT=${TMPDIR}/clang-tidy.out
+readonly TIDY_OUT=${TMPDIR}/clang-tidy-verible.out
 
 # Use clang-tidy-11 if available as it still checks for
 # google-runtime-references, non-const references - which is the
@@ -55,11 +55,12 @@ readonly EXEC_ROOT=$(bazel info execution_root)
 # (TODO: could the tidy result be different if an include content changes ?
 #  Then we have to do g++ -E (using compilation database knowing about -I etc.)
 #  or combine hashes of all mentioned #includes that are also in our list)
-# Make need to re-run file dependent on clang-tidy configuration + file content.
+# Make need to re-run file dependent on clang-tidy configuration, WORKSPACE
+# and of course file content.
 for f in $(find . -name "*.cc" -and -not -name "*test*.cc" \
                 -or -name "*.h" -and -not -name "*test*.h" \
              | grep -v "verilog/tools/kythe") ; do
-  (echo $CLANG_TIDY ; cat .clang-tidy $f) | md5sum | sed "s|-|$f|g"
+  (echo $CLANG_TIDY ; cat .clang-tidy WORKSPACE $f) | md5sum | sed "s|-|$f|g"
 done | sort > ${CLANG_TIDY_SEEN_CACHE}.new
 
 join -v2 ${CLANG_TIDY_SEEN_CACHE} ${CLANG_TIDY_SEEN_CACHE}.new \
@@ -78,6 +79,10 @@ if [ -s ${FILES_TO_PROCESS} ]; then
     | sed "s|$EXEC_ROOT/||g" > ${TIDY_OUT}
 
   cat ${TIDY_OUT}
+  echo "::endgroup::"
+
+  echo ::group::Summary
+  sed 's|\(.*\)\(\[[a-zA-Z.-]*\]$\)|\2|p;d' < ${TIDY_OUT} | sort | uniq -c | sort -n
 
   echo "::endgroup::"
 

.github/workflows/verible-ci.yml

@@ -43,6 +43,7 @@ jobs:
       run: ./.github/bin/run-clang-format.sh
 
   ClangTidy:
+    if: false  # currently disabled, see #1434
     runs-on: ubuntu-20.04
 
     steps:

common/analysis/lint_waiver.h

@@ -124,8 +124,8 @@ class LintWaiverBuilder {
         waive_one_line_keyword_(waive_line_command),
         waive_range_start_keyword_(waive_start_command),
         waive_range_stop_keyword_(waive_stop_command),
-        is_token_comment_(is_comment),
-        is_token_whitespace_(is_space) {}
+        is_token_comment_(std::move(is_comment)),
+        is_token_whitespace_(std::move(is_space)) {}
 
   // Takes a single line's worth of tokens and determines updates to the set of
   // waived lines.  Pass a slice of tokens using make_range.

common/text/token_info.h

@@ -68,8 +68,8 @@ class TokenInfo {
     explicit Context(absl::string_view b);
 
     Context(absl::string_view b,
-            const std::function<void(std::ostream&, int)>& translator)
-        : base(b), token_enum_translator(translator) {}
+            std::function<void(std::ostream&, int)> translator)
+        : base(b), token_enum_translator(std::move(translator)) {}
 
     Context(const Context&) = default;
   };

shell.nix

@@ -2,16 +2,7 @@
 # If you have nix installed, you may simply run `nix-shell`
 # in this repo, and have all dependencies ready in the new shell.
 
-{pkgs ? import (builtins.fetchTarball {
-  # Descriptive name to make the store path easier to identify
-  name = "nixos-2021-11";
-  # Commit hash
-  url = "https://github.com/NixOS/nixpkgs/archive/refs/tags/21.11.tar.gz";
-  # Hash obtained using `nix-prefetch-url --unpack <url>`
-  sha256 = "162dywda2dvfj1248afxc45kcrg83appjd0nmdb541hl7rnncf02";
-}) {}}:
-
-
+{ pkgs ? import <nixpkgs> {} }:
 pkgs.mkShell {
   buildInputs = with pkgs;
     [
@@ -23,6 +14,9 @@ pkgs.mkShell {
       jdk11
       lcov
       python3
-    ];
 
+      # Ease development
+      clang-tools_11    # clang-format
+      bazel-buildtools  # buildifier
+    ];
 }

verilog/analysis/BUILD

@@ -81,8 +81,8 @@ cc_library(
     deps = [
         "//common/text:token_stream_view",
         "//verilog/parser:verilog_token_enum",
-        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/strings",
     ],
 )
 
@@ -99,7 +99,6 @@ cc_test(
     ],
 )
 
-
 cc_library(
     name = "lint_rule_registry",
     srcs = ["lint_rule_registry.cc"],

verilog/analysis/flow_tree.cc

@@ -177,33 +177,17 @@ absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) {
 // edges_.
 absl::Status FlowTree::GenerateControlFlowTree() {
   // Adding edges for if blocks.
-  int current_token_enum = 0;
-  ConditionalBlock empty_block;
-  empty_block.if_location = source_sequence_.end();
-  empty_block.else_location = source_sequence_.end();
-  empty_block.endif_location = source_sequence_.end();
+  const TokenSequenceConstIterator non_location = source_sequence_.end();
 
   for (TokenSequenceConstIterator iter = source_sequence_.begin();
        iter != source_sequence_.end(); iter++) {
-    current_token_enum = iter->token_enum();
+    int current_token_enum = iter->token_enum();
 
     if (IsConditional(iter)) {
       switch (current_token_enum) {
-        case PP_ifdef: {
-          if_blocks_.push_back(empty_block);
-          if_blocks_.back().if_location = iter;
-          if_blocks_.back().positive_condition = 1;
-          auto status = AddMacroOfConditional(iter);
-          if (!status.ok()) {
-            return absl::InvalidArgumentError(
-                "ERROR: couldn't give a macro an ID.");
-          }
-          break;
-        }
+        case PP_ifdef:
         case PP_ifndef: {
-          if_blocks_.push_back(empty_block);
-          if_blocks_.back().if_location = iter;
-          if_blocks_.back().positive_condition = 0;
+          if_blocks_.emplace_back(iter, non_location);
           auto status = AddMacroOfConditional(iter);
           if (!status.ok()) {
             return absl::InvalidArgumentError(

verilog/analysis/flow_tree.h

@@ -88,12 +88,14 @@ class FlowTree {
 
  private:
   struct ConditionalBlock {
+    ConditionalBlock(TokenSequenceConstIterator if_location,
+                     TokenSequenceConstIterator non_location)
+        : if_location(if_location),
+          else_location(non_location),
+          endif_location(non_location) {}
+
     // "if_location" points to `ifdef or `ifndef.
     TokenSequenceConstIterator if_location;
-
-    // When "positive_condition" equals 1, then "if_location" points to `ifdef,
-    // Otherwise it points to `ifndef.
-    bool positive_condition;
     std::vector<TokenSequenceConstIterator> elsif_locations;
     TokenSequenceConstIterator else_location;
     TokenSequenceConstIterator endif_location;

verilog/preprocessor/BUILD

@@ -22,9 +22,9 @@ cc_library(
         "//common/util:container_util",
         "//common/util:file_util",
         "//common/util:logging",
+        "//verilog/parser:verilog_lexer",
         "//verilog/parser:verilog_parser",
         "//verilog/parser:verilog_token_enum",
-        "//verilog/parser:verilog_lexer",
         "//verilog/analysis:verilog_filelist",
         "@com_google_absl//absl/memory",
         "@com_google_absl//absl/status",

verilog/tools/preprocessor/BUILD

@@ -13,12 +13,12 @@ cc_binary(
         "//common/util:file_util",
         "//common/util:init_command_line",
         "//common/util:subcommand",
-        "//verilog/transform:strip_comments",
-        "//verilog/preprocessor:verilog_preprocess",
-        "//verilog/parser:verilog_lexer",
+        "//verilog/analysis:flow_tree",
         "//verilog/analysis:verilog_analyzer",
         "//verilog/analysis:verilog_project",
-        "//verilog/analysis:flow_tree",
+        "//verilog/parser:verilog_lexer",
+        "//verilog/preprocessor:verilog_preprocess",
+        "//verilog/transform:strip_comments",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/flags:usage",
         "@com_google_absl//absl/status",