Base run-clang-tidy.sh on run-clang-tidy-cached.cc

This way, only changed files have to be re-processed.

.github/bin/run-clang-tidy-cached.cc

@@ -109,7 +109,8 @@ struct ConfigValues {
   // that is included by a lot of other files results in lots of reprocessing.
   bool revisit_if_any_include_changes = true;
 
-  // Clang tidy configuration: clang tidy files with checks.
+  // Clang tidy configuration: clang tidy files with checks. This can be
+  // overriden with environment variable CLANG_TIDY_CONFIG
   std::string_view clang_tidy_file = ".clang-tidy";
 };
 
@@ -176,11 +177,15 @@ std::string GetContent(const fs::path &f) {
   return GetContent(file_to_read);
 }
 
-const char *EnvWithFallback(const char *varname, const char *fallback) {
-  const char *value = getenv(varname);
+std::string_view EnvWithFallback(const char *var, std::string_view fallback) {
+  const char *value = getenv(var);
   return value ? value : fallback;
 }
 
+std::string_view GetClangTidyConfig() {
+  return EnvWithFallback("CLANG_TIDY_CONFIG", kConfig.clang_tidy_file);
+}
+
 std::string GetCommandOutput(const std::string &prog) {
   return GetContent(popen(prog.c_str(), "r"));  // NOLINT
 }
@@ -323,7 +328,7 @@ class ClangTidyRunner {
   static std::string AssembleArgs(int argc, char **argv) {
     std::string result = " --quiet";
     result.append(" '--config-file=")
-        .append(kConfig.clang_tidy_file)
+      .append(GetClangTidyConfig())
         .append("'");
     for (const std::string_view arg : kExtraArgs) {
       result.append(" --extra-arg='").append(arg).append("'");
@@ -352,7 +357,7 @@ class ClangTidyRunner {
 
     // Make sure directory filename depends on .clang-tidy content.
     hash_t cache_unique_id = hashContent(version + clang_tidy_args_);
-    cache_unique_id ^= hashContent(GetContent(kConfig.clang_tidy_file));
+    cache_unique_id ^= hashContent(GetContent(GetClangTidyConfig()));
     return cache_dir / fs::path(cache_prefix + "v" + major_version + "_" +
                                 ToHex(cache_unique_id, 8));
   }
@@ -527,8 +532,8 @@ class FileGatherer {
 
 int main(int argc, char *argv[]) {
   // Test that key files exist and remember their last change.
-  if (!fs::exists(kConfig.clang_tidy_file)) {
-    std::cerr << "Need a " << kConfig.clang_tidy_file << " config file.\n";
+  if (!fs::exists(GetClangTidyConfig())) {
+    std::cerr << "Need a " << GetClangTidyConfig() << " config file.\n";
     return EXIT_FAILURE;
   }
 

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

@@ -13,26 +13,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# TODO(hzeller): switch to .github/bin/run-clang-tidy-cached.cc
-# entirely once we have all the issues addressed.
-# It caches the results, so is much faster, but doesn't have the
-# concept of the 'limited' clang-tidy.
-
-# Exact binary can be set with environment variable if needed.
-CLANG_TIDY="${CLANG_TIDY:-clang-tidy}"
-
 LOCAL_TMP=${TMPDIR:-/tmp}
 
 TIDY_OUT=${LOCAL_TMP}/clang-tidy-surelog.out
 
-hash ${CLANG_TIDY} || exit 2  # make sure it is installed.
-
 if [ "$1" == "limited" ]; then
+    export CLANG_TIDY_CONFIG="${LOCAL_TMP}/clang-tidy"
     # For now, since there are a lot things to fix, don't enable everything
     # that is mentioned in .clang-tidy, but add rules as we fix them.
     # The more strict .clang-tidy rules will still show up as reminders in the
     # IDE, so are hopefully fixed on the way.
-    cat > ${LOCAL_TMP}/clang-tidy <<EOF
+    cat > "${CLANG_TIDY_CONFIG}" <<EOF
 Checks: >
     -*,
     bugprone-copy-constructor-init,
@@ -88,31 +79,13 @@ CheckOptions:
     - key: performance-unnecessary-copy-initialization.AllowedTypes
       value: ::SURELOG::SymbolId;SURELOG::SymbolId;SymbolId;::SURELOG::NodeId;SURELOG::NodeId;NodeId;::SURELOG::PathId;SURELOG::PathId;PathId
 EOF
-    CLANG_TIDY_OPTS="--config-file=${LOCAL_TMP}/clang-tidy"
 fi
 
-CLANG_TIDY_OPTS="${CLANG_TIDY_OPTS} --quiet"
-
 if [ ! -r compile_commands.json ]; then
     echo "To get compile_commands.json, run in root of project and "
     echo "  make run-cmake-release"
     exit 1
 fi
 
-find src/ include/ -name "*.cpp" -or -name "*.h" \
-    | grep -v Python | grep -v Constraint.h | grep -v "hello.*\.cpp" \
-    | xargs -P$(nproc) -n 5 -- ${CLANG_TIDY} ${CLANG_TIDY_OPTS} 2>/dev/null \
-            > ${TIDY_OUT}
-
-cat ${TIDY_OUT}
-
-sed 's|\(.*\)\(\[[a-zA-Z.-]*\]$\)|\2|p;d' < ${TIDY_OUT} \
-    | sort | uniq -c | sort -rn
-
-if [ -s ${TIDY_OUT} ]; then
-    echo "There were clang-tidy warnings (see ${TIDY_OUT}). Please fix."
-    exit 1
-fi
-
-echo "No clang-tidy complaints.😎"
-exit 0
+# Invoke run-clang-tidy-cached, possibly with the 'limited' clang-tidy config.
+sh $(dirname $0)/run-clang-tidy-cached.cc

.github/workflows/main.yml

@@ -1096,18 +1096,21 @@ jobs:
         submodules: recursive
         fetch-depth: 0
 
-    - name: Use ccache
+    - name: Use cache
+      # ccache and cached clang-tidy output.
       uses: hendrikmuhs/[email protected]
       with:
         key: clang-tidy-codegen
+        path: |
+          /root/.cache
 
     - name: Configure shell
       run: |
         echo 'PATH=/usr/lib/ccache:'"$PATH" >> $GITHUB_ENV
 
     - name: Prepare source
       run: |
-        make run-cmake-release
+        make run-cmake-release-with-python
         make -j2 -C build GenerateParser
         make -j2 -C build GenerateParserListeners
         make -j2 -C build GenerateCacheSerializers
@@ -1117,4 +1120,5 @@ jobs:
       run: |
         export CLANG_TIDY=clang-tidy-18
         "${CLANG_TIDY}" --version
-        ./.github/bin/run-clang-tidy.sh limited
+        ./.github/bin/run-clang-tidy.sh limited \
+           || ( cat Surlog_clang-tidy.out ; exit 1)

include/Surelog/API/PythonAPI.h

@@ -61,7 +61,8 @@ class PythonAPI {
                                 const std::string& function,
                                 const std::vector<std::string>& args,
                                 PyThreadState* interp);
-  static void evalScript(std::string function, SV3_1aPythonListener* listener,
+  static void evalScript(const std::string& function,
+                         SV3_1aPythonListener* listener,
                          parser_rule_context* ctx);
   static std::string getInvalidScriptString() { return m_invalidScriptResult; }
   static bool isListenerLoaded() { return m_listenerLoaded; }

include/Surelog/SourceCompile/CompileSourceFile.h

@@ -35,7 +35,7 @@
 
 #ifdef SURELOG_WITH_PYTHON
 struct _ts;
-typedef struct _ts PyThreadState;
+using PyThreadState = struct _ts;
 #endif
 
 namespace SURELOG {

include/Surelog/SourceCompile/PythonListen.h

@@ -37,6 +37,8 @@ class SV3_1aPythonListener;
 class PythonListen {
  public:
   PythonListen(ParseFile* parse, CompileSourceFile* m_compileSourceFile);
+  PythonListen(const PythonListen& orig) = delete;
+
   bool listen();
 
   virtual ~PythonListen();
@@ -47,8 +49,6 @@ class PythonListen {
   void addError(Error& error);
 
  private:
-  PythonListen(const PythonListen& orig) = delete;
-
   ParseFile* const m_parse;
   CompileSourceFile* const m_compileSourceFile;
   std::vector<SV3_1aPythonListener*> m_pythonListeners;

scripts/generate_python_listener.py

@@ -89,7 +89,7 @@ def _main():
     '  PythonListen*  getPythonListen() { return m_pl; }',
     '  antlr4::CommonTokenStream* getTokenStream() { return m_tokens; }',
     '',
-    '  void logError(ErrorDefinition::ErrorType error, antlr4::ParserRuleContext* ctx, std::string object, bool printColumn = false);',
+    '  void logError(ErrorDefinition::ErrorType error, antlr4::ParserRuleContext* ctx, const std::string& object, bool printColumn = false);',
     '  void logError(ErrorDefinition::ErrorType, Location& loc, bool showDuplicates = false);',
     '  void logError(ErrorDefinition::ErrorType, Location& loc, Location& extraLoc, bool showDuplicates = false);',
     '',

src/API/PythonAPI.cpp

@@ -275,7 +275,8 @@ void PythonAPI::init(int32_t argc, const char** argv) {
 #endif
 }
 
-void PythonAPI::evalScript(std::string function, SV3_1aPythonListener* listener,
+void PythonAPI::evalScript(const std::string& function,
+                           SV3_1aPythonListener* listener,
                            parser_rule_context* ctx1) {
 #ifdef SURELOG_WITH_PYTHON
   antlr4::ParserRuleContext* ctx = (antlr4::ParserRuleContext*)ctx1;

src/API/SV3_1aPythonListener.cpp

@@ -38,13 +38,15 @@ SV3_1aPythonListener::SV3_1aPythonListener(PythonListen* pl,
       m_tokens(tokens),
       m_lineOffset(lineOffset) {}
 
-SV3_1aPythonListener::SV3_1aPythonListener(const SV3_1aPythonListener& orig) {}
+SV3_1aPythonListener::SV3_1aPythonListener(const SV3_1aPythonListener& orig) =
+    default;
 
-SV3_1aPythonListener::~SV3_1aPythonListener() {}
+SV3_1aPythonListener::~SV3_1aPythonListener() = default;
 
 void SV3_1aPythonListener::logError(ErrorDefinition::ErrorType error,
                                     antlr4::ParserRuleContext* ctx,
-                                    std::string object, bool printColumn) {
+                                    const std::string& object,
+                                    bool printColumn) {
   ParseUtils::LineColumn lineCol =
       ParseUtils::getLineColumn(getTokenStream(), ctx);
 

src/SourceCompile/PythonListen.cpp

@@ -37,7 +37,7 @@ PythonListen::PythonListen(ParseFile* parse,
       m_compileSourceFile(compileSourceFile),
       m_usingCachedVersion(false) {}
 
-PythonListen::~PythonListen() {}
+PythonListen::~PythonListen() = default;
 
 void PythonListen::addError(Error& error) {
   getCompileSourceFile()->getErrorContainer()->addError(error);
@@ -51,8 +51,8 @@ bool PythonListen::listen() {
   }
 
   // This is either a parent Parser object of this Parser object has no parent
-  if ((m_parse->m_children.size() != 0) || (m_parse->m_parent == nullptr)) {
-    if ((m_parse->m_parent == nullptr) && (m_parse->m_children.size() == 0)) {
+  if ((!m_parse->m_children.empty()) || (m_parse->m_parent == nullptr)) {
+    if ((m_parse->m_parent == nullptr) && (m_parse->m_children.empty())) {
       SV3_1aPythonListener* pythonListener =
           new SV3_1aPythonListener(this, m_compileSourceFile->getPythonInterp(),
                                    m_parse->m_antlrParserHandler->m_tokens, 0);
@@ -61,20 +61,18 @@ bool PythonListen::listen() {
           pythonListener, m_parse->m_antlrParserHandler->m_tree);
     }
 
-    if (m_parse->m_children.size() != 0) {
-      for (uint32_t i = 0; i < m_parse->m_children.size(); i++) {
-        if (m_parse->m_children[i]->m_antlrParserHandler) {
+    if (!m_parse->m_children.empty()) {
+      for (const ParseFile* child : m_parse->m_children) {
+        if (child->m_antlrParserHandler) {
           // Only visit the chunks that got re-parsed
           // TODO: Incrementally regenerate the FileContent
 
           SV3_1aPythonListener* pythonListener = new SV3_1aPythonListener(
               this, m_compileSourceFile->getPythonInterp(),
-              m_parse->m_children[i]->m_antlrParserHandler->m_tokens,
-              m_parse->m_children[i]->m_offsetLine);
+              child->m_antlrParserHandler->m_tokens, child->m_offsetLine);
           m_pythonListeners.push_back(pythonListener);
           antlr4::tree::ParseTreeWalker::DEFAULT.walk(
-              pythonListener,
-              m_parse->m_children[i]->m_antlrParserHandler->m_tree);
+              pythonListener, child->m_antlrParserHandler->m_tree);
         }
       }
     }