Merge pull request #4011 from hzeller/feature-20241109-redundant-casts

Remove redundant casts, use make_uique, use 'using'

.clang-tidy

@@ -12,8 +12,8 @@
 Checks: >
   bugprone-*,
   -bugprone-branch-clone,
-  -bugprone-narrowing-conversions,
   -bugprone-easily-swappable-parameters,
+  -bugprone-narrowing-conversions,
   clang-diagnostic-*,
   clang-analyzer-*,
   -clang-analyzer-core.CallAndMessage,
@@ -32,10 +32,13 @@ Checks: >
   -misc-unused-parameters,
   -misc-use-anonymous-namespace,
   modernize-*,
-  -modernize-use-trailing-return-type,
+  -modernize-type-traits,
   -modernize-use-auto,
+  -modernize-use-nodiscard,
+  -modernize-use-trailing-return-type,
   performance-*,
   -performance-avoid-endl,
+  -performance-enum-size,
   readability-*,
   -readability-braces-around-statements,
   -readability-convert-member-functions-to-static,

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

@@ -15,7 +15,7 @@ B=${0%%.cc}; [ "$B" -nt "$0" ] || c++ -std=c++17 -o"$B" "$0" && exec "$B" "$@";
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Location: https://github.com/hzeller/dev-tools (2024-10-18)
+// Location: https://github.com/hzeller/dev-tools (2024-11-09)
 
 // Script to run clang-tidy on files in a bazel project while caching the
 // results as clang-tidy can be pretty slow. The clang-tidy output messages
@@ -28,8 +28,9 @@ B=${0%%.cc}; [ "$B" -nt "$0" ] || c++ -std=c++17 -o"$B" "$0" && exec "$B" "$@";
 //   run-clang-tidy-cached.cc --checks="-*,modernize-use-override" --fix
 //
 // Note: useful environment variables to configure are
-//  CLANG_TIDY = binary to run; default would just be clang-tidy.
-//  CACHE_DIR  = where to put the cached content; default ~/.cache
+//  CLANG_TIDY         = binary to run; default would just be clang-tidy.
+//  CLANG_TIDY_CONFIG  = override configuration file in kConfig.clang_tidy_file
+//  CACHE_DIR          = where to put the cached content; default ~/.cache
 
 // This file shall be c++17 self-contained; not using any re2 or absl niceties.
 #include <unistd.h>
@@ -109,15 +110,16 @@ 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";
 };
 
 // --------------[ Project-specific configuration ]--------------
 static constexpr ConfigValues kConfig = {
     .start_dir = ".",
     .file_include_re = "^(src|include)",
-    .file_exclude_re = "\\.template\\.",
+    .file_exclude_re = "\\.template\\.|hellouhdm.cpp",
     // Not clean yet, don't revisit yet.
     .revisit_brokenfiles_if_build_config_newer = false,
 };
@@ -176,11 +178,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 +329,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 +358,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 +533,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,32 +13,24 @@
 # 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,
     bugprone-macro-parentheses,
     bugprone-redundant-branch-condition,
     bugprone-string-integer-assignment,
+    bugprone-suspicious-include,
     bugprone-suspicious-missing-comma,
     bugprone-suspicious-string-compare,
     bugprone-too-small-loop-variable,
@@ -49,19 +41,29 @@ Checks: >
     clang-diagnostic-unused-private-field,
     google-build-using-namespace,
     google-explicit-constructor,
+    modernize-concat-nested-namespaces,
     modernize-loop-convert,
+    modernize-make-unique,
     modernize-raw-string-literal,
+    modernize-redundant-void-arg,
+    modernize-use-bool-literals,
+    modernize-use-equals-default,
+    modernize-use-equals-delete,
+    modernize-use-nullptr,
     modernize-use-override,
+    modernize-use-using,
     performance-faster-string-find,
     performance-for-range-copy,
     performance-inefficient-string-concatenation,
     performance-inefficient-vector-operation,
     performance-unnecessary-copy-initialization,
     performance-unnecessary-value-param,
+    readability-avoid-const-params-in-decls,
     readability-const-return-type,
     readability-container-size-empty,
     readability-delete-null-pointer,
     readability-non-const-parameter,
+    readability-redundant-casting,
     readability-redundant-member-init,
     readability-redundant-smartptr-get,
     readability-redundant-string-cstr,
@@ -79,31 +81,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

@@ -1075,7 +1075,7 @@ jobs:
     - name: Install Dependencies
       run: |
         sudo apt-get update -qq
-        sudo apt -qq -y install clang-tidy-18 \
+        sudo apt -qq -y install clang-tidy-18 ccache \
                                 g++-9 default-jre cmake \
                                 uuid-dev build-essential
 
@@ -1096,25 +1096,34 @@ jobs:
         submodules: recursive
         fetch-depth: 0
 
-    - name: Use ccache
-      uses: hendrikmuhs/[email protected]
+    - name: Create Cache Timestamp
+      id: cache_timestamp
+      uses: nanzm/[email protected]
+      with:
+        format: 'YYYY-MM-DD-HH-mm-ss'
+
+    - name: Retrieve cached results
+      uses: actions/cache@v3
       with:
-        key: clang-tidy-codegen
+        path: |
+          /home/runner/.cache/clang-tidy
+          /home/runner/.cache/ccache
+        key: clang-tidy-${{ steps.cache_timestamp.outputs.time }}
+        restore-keys: clang-tidy-
 
     - name: Configure shell
       run: |
         echo 'PATH=/usr/lib/ccache:'"$PATH" >> $GITHUB_ENV
 
     - name: Prepare source
       run: |
-        make run-cmake-release
-        make -j2 -C build GenerateParser
-        make -j2 -C build GenerateParserListeners
-        make -j2 -C build GenerateCacheSerializers
+        make run-cmake-release-with-python
+        make -j2 -C build surelog  # creates all relevant artifacts
         ln -sf build/compile_commands.json .
 
     - name: Run clang tidy
       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 Surelog_clang-tidy.out ; exit 1)

include/Surelog/API/PythonAPI.h

@@ -30,7 +30,7 @@
 #include <vector>
 
 struct _ts;
-typedef struct _ts PyThreadState;
+using PyThreadState = struct _ts;
 
 namespace SURELOG {
 
@@ -43,6 +43,8 @@ struct parser_rule_context;
 class PythonAPI {
  public:
   PythonAPI() = default;
+  PythonAPI(const PythonAPI& orig) = delete;
+
   virtual ~PythonAPI() = default;
 
   /* Main interpreter (in main thread) */
@@ -59,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; }
@@ -74,8 +77,6 @@ class PythonAPI {
   static void setStrictMode(bool mode) { m_strictMode = mode; }
 
  private:
-  PythonAPI(const PythonAPI& orig) = delete;
-
   static void initInterp_();
   static void loadScriptsInInterp_();
   static bool loadScript_(const std::filesystem::path& name,

include/Surelog/API/SLAPI.h

@@ -49,11 +49,11 @@ class ModuleInstance;
 class DesignComponent;
 class ErrorContainer;
 
-typedef std::vector<uint32_t> UIntVector;
+using UIntVector = std::vector<uint32_t>;
 
 /* Error DB API  */
-void SLsetWaiver(const char* messageId, const char* fileName = 0,
-                 uint32_t line = 0, const char* objectName = 0);
+void SLsetWaiver(const char* messageId, const char* fileName = nullptr,
+                 uint32_t line = 0, const char* objectName = nullptr);
 
 void SLoverrideSeverity(const char* messageId, const char* severity);
 
@@ -67,13 +67,13 @@ void SLaddError(ErrorContainer* container, const char* messageId,
 void SLaddErrorContext(SV3_1aPythonListener* prog,
                        antlr4::ParserRuleContext* context,
                        const char* messageId, const char* objectName,
-                       bool printColumn = 0);
+                       bool printColumn = false);
 
 void SLaddMLErrorContext(SV3_1aPythonListener* prog,
                          antlr4::ParserRuleContext* context1,
                          antlr4::ParserRuleContext* context2,
                          const char* messageId, const char* objectName1,
-                         const char* objectName2, bool printColumn = 0);
+                         const char* objectName2, bool printColumn = false);
 
 void SLaddMLError(ErrorContainer* container, const char* messageId,
                   const char* fileName1, uint32_t line1, uint32_t col1,

include/Surelog/Cache/Cache.h

@@ -48,6 +48,7 @@ class VObject;
 
 class Cache {
  public:
+  Cache(const Cache& orig) = delete;
   static constexpr uint64_t Capacity = 0x000000000FFFFFFF;
 
  protected:
@@ -94,9 +95,6 @@ class Cache {
   void restoreSymbols(
       SymbolTable& targetSymbols,
       const ::capnp::List<::capnp::Text>::Reader& sourceSymbols);
-
- private:
-  Cache(const Cache& orig) = delete;
 };
 
 }  // namespace SURELOG

include/Surelog/Cache/PPCache.h

@@ -35,14 +35,13 @@ class PreprocessFile;
 class PPCache : Cache {
  public:
   explicit PPCache(PreprocessFile* pp);
+  PPCache(const PPCache& orig) = delete;
 
   bool restore(bool errorsOnly);
   bool save();
   bool isValid();
 
  private:
-  PPCache(const PPCache& orig) = delete;
-
   PathId getCacheFileId(PathId sourceFileId) const;
 
   bool checkCacheIsValid(PathId cacheFileId,

include/Surelog/Cache/ParseCache.h

@@ -35,14 +35,13 @@ class ParseFile;
 class ParseCache final : Cache {
  public:
   explicit ParseCache(ParseFile* pp);
+  ParseCache(const ParseCache& orig) = delete;
 
   bool restore();
   bool save();
   bool isValid();
 
  private:
-  ParseCache(const ParseCache& orig) = delete;
-
   PathId getCacheFileId(PathId ppFileId) const;
 
   bool checkCacheIsValid(PathId cacheFileId,

include/Surelog/Cache/PythonAPICache.h

@@ -35,14 +35,13 @@ class PythonListen;
 class PythonAPICache final : Cache {
  public:
   explicit PythonAPICache(PythonListen* listener);
+  PythonAPICache(const PythonAPICache& orig) = delete;
 
   bool restore();
   bool save();
   bool isValid() const;
 
  private:
-  PythonAPICache(const PythonAPICache& orig) = delete;
-
   PathId getCacheFileId(PathId sourceFileId) const;
 
   bool checkCacheIsValid(PathId cacheFileId,