diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3edd9f084..86804fc83 100644 --- a/.github/workflows/ci.yml +++ b/.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) diff --git a/.github/workflows/try.yml b/.github/workflows/try.yml index 0489dc975..ebd11c690 100644 --- a/.github/workflows/try.yml +++ b/.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) diff --git a/CMakeLists.txt b/CMakeLists.txt index 08f76a035..f0b11c284 100644 --- a/CMakeLists.txt +++ b/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() diff --git a/tidy/CMakeLists.txt b/tidy/CMakeLists.txt new file mode 100644 index 000000000..35580da25 --- /dev/null +++ b/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) diff --git a/tidy/README.md b/tidy/README.md new file mode 100644 index 000000000..45c65247f --- /dev/null +++ b/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. diff --git a/tidy/llvm.h b/tidy/llvm.h new file mode 100644 index 000000000..a66431993 --- /dev/null +++ b/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) diff --git a/tidy/module.cc b/tidy/module.cc new file mode 100644 index 000000000..7edc0d0a1 --- /dev/null +++ b/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("subspace-smoke"); + } +}; + +static ClangTidyModuleRegistry::Add X( + "subspace-clang-tidy-module", + "Adds lint checks to be used with the Subspace library."); + +} // namespace clang::tidy::subspace diff --git a/tidy/smoke_check.cc b/tidy/smoke_check.cc new file mode 100644 index 000000000..c3c49ecd3 --- /dev/null +++ b/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("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 diff --git a/tidy/smoke_check.h b/tidy/smoke_check.h new file mode 100644 index 000000000..93704bddf --- /dev/null +++ b/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 diff --git a/tidy/tests/CMakeLists.txt b/tidy/tests/CMakeLists.txt new file mode 100644 index 000000000..a623e7e31 --- /dev/null +++ b/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=$" + "-DCLANG_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) diff --git a/tidy/tests/run_clang_tidy.cmake b/tidy/tests/run_clang_tidy.cmake new file mode 100644 index 000000000..293ac1200 --- /dev/null +++ b/tidy/tests/run_clang_tidy.cmake @@ -0,0 +1,107 @@ +# 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. + +set(config_arg) +if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}.clang-tidy") + set(config_arg "--config-file=${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}.clang-tidy") +endif() + +if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}-stdout.txt") + file(READ "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}-stdout.txt" expect_stdout) + string(REGEX REPLACE "\n+$" "" expect_stdout "${expect_stdout}") +else() + set(expect_stdout "") +endif() + +set(source_file "${RunClangTidy_BINARY_DIR}/${CHECK_NAME}.cc") +configure_file("${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}.cc" "${source_file}" COPYONLY) + +file(GLOB header_files RELATIVE "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}" "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}/*") +file(REMOVE_RECURSE "${RunClangTiy_BINARY_DIR}/${CHECK_NAME}") +foreach(header_file IN LISTS header_files) + if(NOT header_file MATCHES "-fixit\\.h\$") + file(MAKE_DIRECTORY "${RunClangTidy_BINARY_DIR}/${CHECK_NAME}") + configure_file("${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}/${header_file}" "${RunClangTidy_BINARY_DIR}/${CHECK_NAME}/${header_file}" COPYONLY) + endif() +endforeach() + +set(command + "${CLANG_TIDY_COMMAND}" + "--load=${CLANG_TIDY_MODULE}" + "--checks=-*,${CHECK_NAME}" + "--fix" + "--format-style=file" + "--header-filter=/${CHECK_NAME}/" + ${config_arg} + "${source_file}" + -- + ) +execute_process( + COMMAND ${command} + RESULT_VARIABLE result + OUTPUT_VARIABLE actual_stdout + ERROR_VARIABLE actual_stderr + ) +string(REPLACE "${RunClangTidy_BINARY_DIR}/" "" actual_stdout "${actual_stdout}") + +set(RunClangTidy_TEST_FAILED) + +if(NOT result EQUAL 0) + string(APPEND RunClangTidy_TEST_FAILED "Expected result: 0, actual result: ${result}\n") +endif() + +string(REGEX REPLACE "\n+$" "" actual_stdout "${actual_stdout}") +if(NOT actual_stdout STREQUAL expect_stdout) + string(REPLACE "\n" "\n " expect_stdout_formatted " ${expect_stdout}") + string(REPLACE "\n" "\n " actual_stdout_formatted " ${actual_stdout}") + string(APPEND RunClangTidy_TEST_FAILED "Expected stdout:\n${expect_stdout_formatted}\nActual stdout:\n${actual_stdout_formatted}\n") +endif() + +function(check_fixit expected fallback_expected actual) + if(EXISTS "${expected}") + set(expect_fixit_file "${expected}") + else() + set(expect_fixit_file "${fallback_expected}") + endif() + file(READ "${expect_fixit_file}" expect_fixit) + file(READ "${actual}" actual_fixit) + if(NOT expect_fixit STREQUAL actual_fixit) + string(REPLACE "\n" "\n " expect_fixit_formatted " ${expect_fixit}") + string(REPLACE "\n" "\n " actual_fixit_formatted " ${actual_fixit}") + string(APPEND RunClangTidy_TEST_FAILED "Expected fixit for ${actual}:\n${expect_fixit_formatted}\nActual fixit:\n${actual_fixit_formatted}\n") + set(RunClangTidy_TEST_FAILED "${RunClangTidy_TEST_FAILED}" PARENT_SCOPE) + endif() +endfunction() + +check_fixit( + "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}-fixit.cc" + "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}.cc" + "${source_file}" + ) + +foreach(header_file IN LISTS header_files) + if(NOT header_file MATCHES "-fixit\\.h\$") + string(REGEX REPLACE "\\.h\$" "-fixit.h" header_fixit "${header_file}") + check_fixit( + "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}/${header_fixit}" + "${CMAKE_CURRENT_LIST_DIR}/${CHECK_NAME}/${header_file}" + "${RunClangTidy_BINARY_DIR}/${CHECK_NAME}/${header_file}" + ) + endif() +endforeach() + +if(RunClangTidy_TEST_FAILED) + string(REPLACE ";" " " command_formatted "${command}") + message(FATAL_ERROR "Command:\n ${command_formatted}\n${RunClangTidy_TEST_FAILED}") +endif() diff --git a/tidy/tests/subspace-smoke.cc b/tidy/tests/subspace-smoke.cc new file mode 100644 index 000000000..56757a701 --- /dev/null +++ b/tidy/tests/subspace-smoke.cc @@ -0,0 +1 @@ +void f() {}