chipsalliance · hzeller · Oct 12, 2024 · Oct 12, 2024
diff --git a/.clang-tidy b/.clang-tidy
@@ -34,10 +34,12 @@ Checks: >
   clang-analyzer-*,
   -clang-analyzer-core.NonNullParamChecker,
   -clang-analyzer-cplusplus.InnerPointer,
+  -clang-analyzer-optin.core.EnumCastOutOfRange,
   -clang-analyzer-optin.performance.Padding,
   abseil-*,
   -abseil-no-namespace,
   readability-*,
+  -readability-avoid-nested-conditional-operator,
   -readability-avoid-unconditional-preprocessor-if,
   -readability-braces-around-statements,
   -readability-convert-member-functions-to-static,
@@ -60,10 +62,12 @@ Checks: >
   -google-readability-todo,
   performance-*,
   -performance-avoid-endl,
+  -performance-enum-size,
   bugprone-*,
   -bugprone-branch-clone,
   -bugprone-easily-swappable-parameters,
   -bugprone-exception-escape,
+  -bugprone-inc-dec-in-conditions,
   -bugprone-macro-parentheses,
   -bugprone-move-forwarding-reference,
   -bugprone-narrowing-conversions,
@@ -72,6 +76,7 @@ Checks: >
   -modernize-avoid-bind,
   -modernize-avoid-c-arrays,
   -modernize-concat-nested-namespaces,
+  -modernize-make-shared,
   -modernize-make-unique,
   -modernize-pass-by-value,
   -modernize-raw-string-literal,

diff --git a/common/analysis/matcher/matcher_builders.h b/common/analysis/matcher/matcher_builders.h
@@ -106,22 +106,22 @@ class PathMatchBuilder {
 // Helper functions to create PathMatchers.
 // Length of path determine by number of params. Might need to add more or
 // TODO: implement a arg... template
-inline constexpr PathMatchBuilder<1> MakePathMatcher(const SymbolTag t1) {
+constexpr PathMatchBuilder<1> MakePathMatcher(const SymbolTag t1) {
   return PathMatchBuilder<1>(std::array<SymbolTag, 1>{t1});
 }
-inline constexpr PathMatchBuilder<2> MakePathMatcher(const SymbolTag t1,
-                                                     const SymbolTag t2) {
+constexpr PathMatchBuilder<2> MakePathMatcher(const SymbolTag t1,
+                                              const SymbolTag t2) {
   return PathMatchBuilder<2>(std::array<SymbolTag, 2>{t1, t2});
 }
-inline constexpr PathMatchBuilder<3> MakePathMatcher(const SymbolTag t1,
-                                                     const SymbolTag t2,
-                                                     const SymbolTag t3) {
+constexpr PathMatchBuilder<3> MakePathMatcher(const SymbolTag t1,
+                                              const SymbolTag t2,
+                                              const SymbolTag t3) {
   return PathMatchBuilder<3>(std::array<SymbolTag, 3>{t1, t2, t3});
 }
-inline constexpr PathMatchBuilder<4> MakePathMatcher(const SymbolTag t1,
-                                                     const SymbolTag t2,
-                                                     const SymbolTag t3,
-                                                     const SymbolTag t4) {
+constexpr PathMatchBuilder<4> MakePathMatcher(const SymbolTag t1,
+                                              const SymbolTag t2,
+                                              const SymbolTag t3,
+                                              const SymbolTag t4) {
   return PathMatchBuilder<4>(std::array<SymbolTag, 4>{t1, t2, t3, t4});
 }
 

diff --git a/common/lsp/dummy-ls.cc b/common/lsp/dummy-ls.cc
@@ -89,7 +89,7 @@ int main(int argc, char *argv[]) {
   MessageStreamSplitter stream_splitter;
   stream_splitter.SetMessageProcessor(
       [&dispatcher](absl::string_view /*header*/, absl::string_view body) {
-        return dispatcher.DispatchMessage(body);
+        dispatcher.DispatchMessage(body);
       });
 
   // The buffer collection keeps track of all the buffers opened in the editor.

diff --git a/common/lsp/lsp-protocol-operators.h b/common/lsp/lsp-protocol-operators.h
@@ -23,23 +23,23 @@ namespace verible {
 namespace lsp {
 
 // Less-than ordering of positions
-inline constexpr bool operator<(const Position &a, const Position &b) {
+constexpr bool operator<(const Position &a, const Position &b) {
   if (a.line > b.line) return false;
   if (a.line < b.line) return true;
   return a.character < b.character;
 }
-inline constexpr bool operator>=(const Position &a, const Position &b) {
+constexpr bool operator>=(const Position &a, const Position &b) {
   return !(a < b);
 }
-inline constexpr bool operator==(const Position &a, const Position &b) {
+constexpr bool operator==(const Position &a, const Position &b) {
   return a.line == b.line && a.character == b.character;
 }
 
 // Ranges overlap if some part of one is inside the other range.
 // Also empty ranges are considered overlapping if their start point is within
 // the other range.
 // rangerOverlap() is commutative.
-inline constexpr bool rangeOverlap(const Range &a, const Range &b) {
+constexpr bool rangeOverlap(const Range &a, const Range &b) {
   return !(a.start >= b.end || b.start >= a.end) || (a.start == b.start);
 }
 }  // namespace lsp

diff --git a/common/text/symbol.h b/common/text/symbol.h
@@ -53,7 +53,7 @@ template <typename EnumType>
 constexpr SymbolTag NodeTag(EnumType tag) {
   return {SymbolKind::kNode, static_cast<int>(tag)};
 }
-inline constexpr SymbolTag LeafTag(int tag) { return {SymbolKind::kLeaf, tag}; }
+constexpr SymbolTag LeafTag(int tag) { return {SymbolKind::kLeaf, tag}; }
 
 // forward declare Visitor classes to allow references in Symbol
 

diff --git a/common/util/type_traits.h b/common/util/type_traits.h
@@ -97,12 +97,12 @@ using remove_cvref_t = std::remove_cv_t<std::remove_reference_t<T>>;
 
 // Type used as a placeholder for unavailable feature in type traits.
 struct UnavailableFeatureTraits {
-  static inline constexpr bool available = false;
+  static constexpr bool available = false;
 };
 
 // Type used as a base for available feature in type traits.
 struct FeatureTraits {
-  static inline constexpr bool available = true;
+  static constexpr bool available = true;
 };
 
 }  // namespace verible

diff --git a/verilog/analysis/checkers/dff_name_style_rule.h b/verilog/analysis/checkers/dff_name_style_rule.h
@@ -85,11 +85,10 @@ namespace analysis {
 class DffNameStyleRule : public verible::SyntaxTreeLintRule {
  public:
   using rule_type = verible::SyntaxTreeLintRule;
-  inline static constexpr absl::string_view kDefaultInputSuffixes = "next,n,d";
-  inline static constexpr absl::string_view kDefaultOutputSuffixes =
-      "reg,r,ff,q";
-  inline static constexpr absl::string_view kDefaultWaiveRegex = "(?i)mem.*";
-  inline static constexpr absl::string_view kDefaultWaiveConditions =
+  static constexpr absl::string_view kDefaultInputSuffixes = "next,n,d";
+  static constexpr absl::string_view kDefaultOutputSuffixes = "reg,r,ff,q";
+  static constexpr absl::string_view kDefaultWaiveRegex = "(?i)mem.*";
+  static constexpr absl::string_view kDefaultWaiveConditions =
       "!rst_ni,flush_i,!rst_ni || flush_i,flush_i || !rst_ni";
 
   static const LintRuleDescriptor &GetDescriptor();

diff --git a/verilog/analysis/verilog_linter_configuration.cc b/verilog/analysis/verilog_linter_configuration.cc
@@ -335,8 +335,6 @@ bool LinterConfiguration::operator==(const LinterConfiguration &config) const {
 absl::Status LinterConfiguration::AppendFromFile(
     absl::string_view config_filename) {
   // Read local configuration file
-  std::string content;
-
   absl::StatusOr<std::string> config_or =
       verible::file::GetContentAsString(config_filename);
   if (config_or.ok()) {

diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
@@ -3002,8 +3002,8 @@ void TreeUnwrapper::ReshapeTokenPartitions(
       // adjust indentation and do not merge leaf into previous leaf.
       const verible::UnwrappedLine& unwrapped_line = partition.Value();
       verible::FormatTokenRange tokenrange = unwrapped_line.TokensRange();
-      auto token_tmp = tokenrange.begin();
-      if (token_tmp->TokenEnum() == TK_initial &&
+      if (auto token_tmp = tokenrange.begin();
+          token_tmp->TokenEnum() == TK_initial &&
           (++token_tmp)->TokenEnum() == TK_EOL_COMMENT &&
           (++token_tmp)->TokenEnum() == TK_begin) {
         AdjustIndentationRelative(

diff --git a/verilog/preprocessor/verilog_preprocess_test.cc b/verilog/preprocessor/verilog_preprocess_test.cc
@@ -950,8 +950,6 @@ TEST(VerilogPreprocessTest, IncludingFileWithRelativePath) {
   constexpr absl::string_view included_content(
       "module included_file(); endmodule");
   const absl::string_view included_filename = "included_file.sv";
-  const std::string included_absolute_path =
-      JoinPath(includes_dir, included_filename);
   const ScopedTestFile tf(includes_dir, included_content, included_filename);
 
   const std::string src_content = absl::StrCat("`include \"", included_filename,
@@ -1008,8 +1006,6 @@ TEST(VerilogPreprocessTest,
   constexpr absl::string_view included_content(
       "module included_file(); endmodule\n");
   const absl::string_view included_filename = "included_file.sv";
-  const std::string included_absolute_path =
-      JoinPath(includes_dir, included_filename);
   const ScopedTestFile tf(includes_dir, included_content, included_filename);
   const std::string src_content = absl::StrCat("`include \"", included_filename,
                                                "\"\nmodule src(); endmodule\n");

diff --git a/verilog/tools/ls/verilog-language-server.cc b/verilog/tools/ls/verilog-language-server.cc
@@ -47,7 +47,7 @@ VerilogLanguageServer::VerilogLanguageServer(const WriteFun &write_fun)
   // All bodies the stream splitter extracts are pushed to the json dispatcher
   stream_splitter_.SetMessageProcessor(
       [this](absl::string_view header, absl::string_view body) {
-        return dispatcher_.DispatchMessage(body);
+        dispatcher_.DispatchMessage(body);
       });
 
   // Whenever the text changes in the editor, reparse affected code.

diff --git a/verilog/tools/ls/verilog-language-server.h b/verilog/tools/ls/verilog-language-server.h
@@ -93,5 +93,5 @@ class VerilogLanguageServer {
   bool shutdown_requested_ = false;
 };
 
-};      // namespace verilog
+}  // namespace verilog
 #endif  // VERILOG_TOOLS_LS_LS_WRAPPER_H
diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc
@@ -446,7 +446,8 @@ TEST_F(VerilogLanguageServerTest,
   const std::string mini_module =
       DidOpenRequest("file://mini.sv", "module mini();\nendmodule\n");
   ASSERT_OK(SendRequest(mini_module));
-  std::string discard_diagnostics = GetResponse();
+
+  GetResponse();  // ignore response.
 
   // Close the file from the Language Server perspective
   const absl::string_view closing_request = R"(
@@ -1060,7 +1061,6 @@ endmodule
       root_dir, sample_module_b_with_error, "b.sv");
 
   const std::string module_a_uri = PathToLSPUri(module_a.filename());
-  const std::string module_b_uri = PathToLSPUri(module_b.filename());
   const std::string module_a_open_request =
       DidOpenRequest(module_a_uri, kSampleModuleA);
   ASSERT_OK(SendRequest(module_a_open_request));