Subspace clang-tidy module

.github/workflows/ci.yml

@@ -138,6 +138,11 @@ jobs:
             set(HAS_LLVM OFF)
           endif()
 
+          set(CLANG_TIDY OFF)
+          if (NOT "${{ runner.os }}" STREQUAL "Windows")
+            set(CLANG_TIDY ON)
+          endif()
+
           if ("${{ runner.os }}" STREQUAL "Windows" AND NOT "x${{ matrix.config.environment_script }}" STREQUAL "x")
             execute_process(
               COMMAND "${{ matrix.config.environment_script }}" && set
@@ -163,6 +168,7 @@ jobs:
               -D CMAKE_MAKE_PROGRAM=${ninja_program}
               -D SUBSPACE_BUILD_CIR=${HAS_LLVM}
               -D SUBSPACE_BUILD_SUBDOC=${HAS_LLVM}
+              -D SUBSPACE_BUILD_TIDY=${CLANG_TIDY}
             RESULT_VARIABLE result
           )
           if (NOT result EQUAL 0)

.github/workflows/try.yml

@@ -138,6 +138,11 @@ jobs:
             set(HAS_LLVM OFF)
           endif()
 
+          set(CLANG_TIDY OFF)
+          if (NOT "${{ runner.os }}" STREQUAL "Windows")
+            set(CLANG_TIDY ON)
+          endif()
+
           if ("${{ runner.os }}" STREQUAL "Windows" AND NOT "x${{ matrix.config.environment_script }}" STREQUAL "x")
             execute_process(
               COMMAND "${{ matrix.config.environment_script }}" && set
@@ -162,6 +167,7 @@ jobs:
               -D CMAKE_MAKE_PROGRAM=${ninja_program}
               -D SUBSPACE_BUILD_CIR=${HAS_LLVM}
               -D SUBSPACE_BUILD_SUBDOC=${HAS_LLVM}
+              -D SUBSPACE_BUILD_TIDY=${CLANG_TIDY}
             RESULT_VARIABLE result
           )
           if (NOT result EQUAL 0)

CMakeLists.txt

@@ -35,9 +35,11 @@ include(OptionIf)
 
 option_if_not_defined(SUBSPACE_BUILD_CIR "Build CIR (requires LLVM)" ON)
 option_if_not_defined(SUBSPACE_BUILD_SUBDOC "Build subdoc (requires LLVM)" ON)
+option_if_not_defined(SUBSPACE_BUILD_TIDY "Build clang-tidy plugin (requires LLVM)" NOT WIN32)
 
 message(STATUS "Build CIR: ${SUBSPACE_BUILD_CIR}")
 message(STATUS "Build subdoc: ${SUBSPACE_BUILD_SUBDOC}")
+message(STATUS "Build clang-tidy plugin: ${SUBSPACE_BUILD_TIDY}")
 
 function(subspace_default_compile_options TARGET)
     if(MSVC)
@@ -81,3 +83,14 @@ endif()
 if (${SUBSPACE_BUILD_SUBDOC})
     add_subdirectory(subdoc)
 endif()
+
+if(${SUBSPACE_BUILD_TIDY})
+    add_subdirectory(tidy)
+
+    #set(CMAKE_CXX_CLANG_TIDY 
+    #    clang-tidy;
+    #    -header-filter=.;
+    #    -checks=-*,subspace-*;
+    #   -warnings-as-errors=*;
+    #)
+endif()

tidy/CMakeLists.txt

@@ -0,0 +1,81 @@
+# Copyright 2023 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+add_library(subspace_clang_tidy_module MODULE "")
+add_library(subspace::clang_tidy ALIAS subspace_clang_tidy_module)
+target_sources(subspace_clang_tidy_module PUBLIC
+    "llvm.h"
+    "module.cc"
+    "smoke_check.cc"
+    "smoke_check.h"
+)
+
+target_link_libraries(subspace_clang_tidy_module
+    subspace::lib
+
+    clangTidy
+
+    clangAnalysis
+    clangAnalysisFlowSensitive
+    clangAnalysisFlowSensitiveModels
+    clangAPINotes
+    clangARCMigrate
+    clangAST
+    clangASTMatchers
+    clangBasic
+    clangCodeGen
+    clangCrossTU
+    clangDependencyScanning
+    clangDirectoryWatcher
+    clangDriver
+    clangDynamicASTMatchers
+    clangEdit
+    clangExtractAPI
+    clangFormat
+    clangFrontend
+    clangFrontendTool
+    clangHandleCXX
+    clangHandleLLVM
+    clangIndex
+    clangIndexSerialization
+    clangInterpreter
+    clangLex
+    clangParse
+    clangRewrite
+    clangRewriteFrontend
+    clangSema
+    clangSerialization
+    clangStaticAnalyzerCheckers
+    clangStaticAnalyzerCore
+    clangStaticAnalyzerFrontend
+    clangSupport
+    clangTooling
+    clangToolingASTDiff
+    clangToolingCore
+    clangToolingInclusions
+    clangToolingInclusionsStdlib
+    clangToolingRefactoring
+    clangToolingSyntax
+    clangTransformer
+)
+
+find_package(Clang REQUIRED)
+llvm_config(subdoc_lib)
+target_include_directories(subspace_clang_tidy_module PUBLIC ${LLVM_INCLUDE_DIRS})
+target_link_directories(subspace_clang_tidy_module PUBLIC ${LLVM_LIBRARY_DIRS})
+
+# Subspace clang-tidy module.
+subspace_default_compile_options(subspace_clang_tidy_module)
+
+add_subdirectory(tests)

tidy/README.md

@@ -0,0 +1,29 @@
+# Things that we want a clang-tidy for
+
+## Closure concept usage
+* Templates restricted as Fn are received as const&-ref
+* Templates restricted as Fn are invoked as lvalue `std::invoke(f, ...)`
+* Templates restricted as FnMut are received by value or as &-ref
+* Templates restricted as FnMut are invoked as lvalue `std::invoke(f, ...)`
+* Templates restricted as FnOnce are received as &&-ref or by value
+* Templates restricted as FnOnce are invoked as rvalue `std::invoke(sus::move(f), ...)`
+
+## References to closures
+* FnOnceRef, FnMutRef and FnRef types only appear as lvalues as ParamVarDecl,
+  and are not returned.
+
+## Long-lived closures
+* Think about how to rewrite FnBox to allow lambdas with capture if we use a
+  clang-tidy to ensure arguments are not stored as references carelessly.
+
+## Mutable references
+* Passing an lvalue to a function receiving an lvalue-reference should require
+  the argument to be wrapped in `sus::mref()`. Probably exceptions for generic
+  code.
+
+## IteratorLoop and GeneratorLoop
+* It's not safe to use IteratorLoop outside of a ranged-for loop, and we should
+  check that it's not happening. UB can result from getting to the end() and
+  calling operator*().
+* The GeneratorLoop can not outlive the Generator it is looping over, so should
+  not be used outside of a ranged-for loop.

tidy/llvm.h

@@ -0,0 +1,47 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+// All LLVM and Clang includes go here, because they are full of compiler
+// warnings that we have to disable.
+
+#pragma warning(push)
+#pragma warning(disable : 4100)
+#pragma warning(disable : 4127)
+#pragma warning(disable : 4146)
+#pragma warning(disable : 4244)
+#pragma warning(disable : 4245)
+#pragma warning(disable : 4267)
+#pragma warning(disable : 4291)
+#pragma warning(disable : 4324)
+#pragma warning(disable : 4389)
+#pragma warning(disable : 4456)
+#pragma warning(disable : 4458)
+#pragma warning(disable : 4459)
+#pragma warning(disable : 4616)
+#pragma warning(disable : 4624)
+#pragma warning(disable : 4702)
+
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+#pragma warning(pop)

tidy/module.cc

@@ -0,0 +1,30 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tidy/llvm.h"
+#include "tidy/smoke_check.h"
+
+namespace clang::tidy::subspace {
+
+class SubspaceClangTidyModule : public ClangTidyModule {
+  void addCheckFactories(ClangTidyCheckFactories& factories) override {
+    factories.registerCheck<SmokeCheck>("subspace-smoke");
+  }
+};
+
+static ClangTidyModuleRegistry::Add<SubspaceClangTidyModule> X(
+    "subspace-clang-tidy-module",
+    "Adds lint checks to be used with the Subspace library.");
+
+}  // namespace clang::tidy::subspace

tidy/smoke_check.cc

@@ -0,0 +1,41 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tidy/smoke_check.h"
+
+#include "subspace/prelude.h"
+#include "tidy/llvm.h"
+
+namespace clang::tidy::subspace {
+
+SmokeCheck::SmokeCheck(llvm::StringRef name, ClangTidyContext* context)
+    : ClangTidyCheck(sus::move(name), context) {}
+
+void SmokeCheck::registerMatchers(ast_matchers::MatchFinder* finder) {
+  using namespace ast_matchers;
+
+  finder->addMatcher(functionDecl().bind("x"), this);
+}
+
+void SmokeCheck::check(const ast_matchers::MatchFinder::MatchResult& match) {
+  const auto* MatchedDecl = match.Nodes.getNodeAs<FunctionDecl>("x");
+  if (!MatchedDecl->getIdentifier() ||
+      MatchedDecl->getName().startswith("awesome_"))
+    return;
+  diag(MatchedDecl->getLocation(), "function %0 is insufficiently awesome")
+      << MatchedDecl
+      << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
+}
+
+}  // namespace clang::tidy::subspace

tidy/smoke_check.h

@@ -0,0 +1,29 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include "subspace/macros/compiler.h"
+#include "tidy/llvm.h"
+
+namespace clang::tidy::subspace {
+
+class SmokeCheck : public ClangTidyCheck {
+ public:
+  SmokeCheck(llvm::StringRef name, ClangTidyContext* context);
+  void registerMatchers(ast_matchers::MatchFinder* finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult& match) override;
+};
+
+}  // namespace clang::tidy::subspace

tidy/tests/CMakeLists.txt

@@ -0,0 +1,27 @@
+# Copyright 2023 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+enable_testing()
+
+function(tidy_test check_name)
+  add_test(NAME "RunClangTidy.${check_name}" COMMAND ${CMAKE_COMMAND}
+    "-DCLANG_TIDY_COMMAND=$<TARGET_FILE:clang-tidy>"
+    "-DCLANG_TIDY_MODULE=$<TARGET_FILE:subspace_clang_tidy_module>"
+    "-DCHECK_NAME=${check_name}"
+    "-DRunClangTidy_BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR}"
+    -P "${CMAKE_CURRENT_SOURCE_DIR}/run_clang_tidy.cmake"
+    )
+endfunction()
+
+tidy_test(subspace-smoke)