From fcb676e0863a057fc323875301b5c59df08eca32 Mon Sep 17 00:00:00 2001
From: daquexian <daquexian566@gmail.com>
Date: Thu, 15 Jul 2021 12:06:44 +0800
Subject: [PATCH] new static check based on clang-tidy (#5476)
* new static check based on clang-tidy
Signed-off-by: daquexian <daquexian566@gmail.com>
* set compiler as clang to generate right compile_commands.json, skip build
Signed-off-by: daquexian <daquexian566@gmail.com>
* include clang headers
Signed-off-by: daquexian <daquexian566@gmail.com>
* build third party
Signed-off-by: daquexian <daquexian566@gmail.com>
* install deps
Signed-off-by: daquexian <daquexian566@gmail.com>
* build third party by gcc, generate compile_commands.json by clang
Signed-off-by: daquexian <daquexian566@gmail.com>
* rm CMakeCache.txt
Signed-off-by: daquexian <daquexian566@gmail.com>
* run of_git_version
Signed-off-by: daquexian <daquexian566@gmail.com>
* download clang-tidy from oneflow-inc/llvm-project repo
Signed-off-by: daquexian <daquexian566@gmail.com>
* rename check name
Signed-off-by: daquexian <daquexian566@gmail.com>
* remove old clang plugin
Signed-off-by: daquexian <daquexian566@gmail.com>
Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com>
---
.clang-tidy | 4 +
.github/workflows/test.yml | 52 +++----
tools/clang-plugin/.clang-format | 1 -
tools/clang-plugin/.gitignore | 4 -
tools/clang-plugin/CMakeLists.txt | 66 ---------
.../CheckUnusedMaybe/CMakeLists.txt | 18 ---
.../CheckUnusedMaybe/CheckUnusedMaybe.cpp | 134 ------------------
.../CheckUnusedMaybe/unused_maybe_example.cpp | 8 --
8 files changed, 24 insertions(+), 263 deletions(-)
create mode 100644 .clang-tidy
delete mode 100644 tools/clang-plugin/.clang-format
delete mode 100644 tools/clang-plugin/.gitignore
delete mode 100644 tools/clang-plugin/CMakeLists.txt
delete mode 100644 tools/clang-plugin/CheckUnusedMaybe/CMakeLists.txt
delete mode 100644 tools/clang-plugin/CheckUnusedMaybe/CheckUnusedMaybe.cpp
delete mode 100644 tools/clang-plugin/CheckUnusedMaybe/unused_maybe_example.cpp
diff --git a/.clang-tidy b/.clang-tidy
new file mode 100644
index 000000000..d5935e488
--- /dev/null
+++ b/.clang-tidy
@@ -0,0 +1,4 @@
+# maybe-* checks are only available on OneFlow custom clang-tidy and clangd
+Checks: '-*, maybe-*'
+# TODO: treat all maybe warnings as errors when existing warnings are all fixed
+WarningsAsErrors: 'maybe-unused'
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index a45142400..78f718010 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -92,8 +92,8 @@ jobs:
run: |
exit 1
- static_analysis_with_clang_plug_in:
- name: Static analysis with Clang plug-in
+ static_analysis_with_clang:
+ name: Static analysis with clang
runs-on: ubuntu-20.04
if: github.event.pull_request.draft == false && contains(github.event.pull_request.requested_reviewers.*.login, 'oneflow-ci-bot')
steps:
@@ -104,48 +104,36 @@ jobs:
repository: ${{github.event.pull_request.head.repo.full_name}}
- name: Install dependencies
run: |
- wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
- sudo apt-add-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-12 main"
sudo apt-get update
- sudo apt-get install -y llvm-12 llvm-12-dev llvm-12-tools clang-12 libclang-common-12-dev libclang-12-dev \
- libopenblas-dev nasm python3-pip ninja-build
- - name: Set environment variables
- run: |
- set -x
- echo "CC=clang-12" >> $GITHUB_ENV
- echo "CXX=clang++-12" >> $GITHUB_ENV
- - name: Build Clang plug-in
+ sudo apt-get install -y libopenblas-dev nasm python3-pip ninja-build
+ - name: Download OneFlow custom clang-tidy
run: |
- cd tools/clang-plugin
- mkdir build
- cd build
- cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCP_LLVM_INSTALL_DIR=/usr/lib/llvm-12
- make -j$(nproc)
- clang_plugin_path="${PWD}/lib/libCheckUnusedMaybe.so"
- ldd ${clang_plugin_path}
- echo "clang_plugin_path=${clang_plugin_path}" >> $GITHUB_ENV
- - name: Build (third party)
+ wget https://github.com/oneflow-inc/llvm-project/releases/download/latest/clang-tidy
+ wget https://raw.githubusercontent.com/oneflow-inc/llvm-project/maybe/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+ chmod +x clang-tidy run-clang-tidy.py
+ - name: Build third party libs and generate files
run: |
mkdir build
cd build
- # clang-12 produces many warnings on oneflow, -w to suppress them
+ # clang-tidy has builtin includes, but doesn't work here
+ # https://clang.llvm.org/docs/LibTooling.html#builtin-includes
cmake .. -C ../cmake/caches/international/cpu.cmake \
-DCMAKE_BUILD_TYPE=Release \
- -DBUILD_TESTING=ON \
- -DTHIRD_PARTY=ON \
- -DONEFLOW=OFF \
- -DCMAKE_CXX_FLAGS="-w"
- cmake --build . -j$(nproc)
- - name: Check unused Maybe (by building OneFlow with Clang plug-in)
+ -DBUILD_TESTING=ON
+ cmake --build . -j$(nproc) --target of_git_version oneflow_deps generate_functional of_cfgobj generate_py_cfg
+ - name: Run Maybe-related checks by clang-tidy
run: |
cd build
+ rm CMakeCache.txt
cmake .. -C ../cmake/caches/international/cpu.cmake \
+ -DCMAKE_C_COMPILER=clang-12 \
+ -DCMAKE_CXX_COMPILER=clang++-12 \
+ -DCMAKE_CXX_FLAGS="-isystem /usr/lib/llvm-12/lib/clang/12.0.1/include/" \
-DCMAKE_BUILD_TYPE=Release \
-DBUILD_TESTING=ON \
- -DTHIRD_PARTY=OFF \
- -DONEFLOW=ON \
- -DCMAKE_CXX_FLAGS="-w -fplugin=${{ env.clang_plugin_path }}"
- cmake --build . -j$(nproc)
+ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+ cd ..
+ ./run-clang-tidy.py -clang-tidy-binary ./clang-tidy -p build -quiet
wait_for_gpu_slot:
name: Wait for GPU slots
diff --git a/tools/clang-plugin/.clang-format b/tools/clang-plugin/.clang-format
deleted file mode 100644
index 9b3aa8b72..000000000
--- a/tools/clang-plugin/.clang-format
+++ /dev/null
@@ -1 +0,0 @@
-BasedOnStyle: LLVM
diff --git a/tools/clang-plugin/.gitignore b/tools/clang-plugin/.gitignore
deleted file mode 100644
index 413e45698..000000000
--- a/tools/clang-plugin/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-build/
-build-*/
-.cache/
-compile_commands.json
diff --git a/tools/clang-plugin/CMakeLists.txt b/tools/clang-plugin/CMakeLists.txt
deleted file mode 100644
index 7e5b98a2c..000000000
--- a/tools/clang-plugin/CMakeLists.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-cmake_minimum_required(VERSION 3.10)
-project(clang-plugins CXX)
-
-set(CP_LLVM_INSTALL_DIR "" CACHE PATH "LLVM installation directory")
-
-# A bit of a sanity checking
-set(CP_LLVM_INCLUDE_DIR "${CP_LLVM_INSTALL_DIR}/include/llvm")
-if(NOT EXISTS "${CP_LLVM_INCLUDE_DIR}")
-message(FATAL_ERROR
- "${CP_LLVM_INCLUDE_DIR} does not exist, CP_LLVM_INSTALL_DIR (${CP_LLVM_INSTALL_DIR}) is invalid")
-endif()
-
-set(CP_LLVM_CMAKE_FILE "${CP_LLVM_INSTALL_DIR}/lib/cmake/clang/ClangConfig.cmake")
-if(NOT EXISTS "${CP_LLVM_CMAKE_FILE}")
-message(FATAL_ERROR
- " CP_LLVM_CMAKE_FILE (${CP_LLVM_CMAKE_FILE}) is invalid.")
-endif()
-
-#===============================================================================
-# 2. LOAD CLANG CONFIGURATION
-# For more: http://llvm.org/docs/CMake.html#embedding-llvm-in-your-project
-#===============================================================================
-list(APPEND CMAKE_PREFIX_PATH "${CP_LLVM_INSTALL_DIR}/lib/cmake/llvm/")
-list(APPEND CMAKE_PREFIX_PATH "${CP_LLVM_INSTALL_DIR}/lib/cmake/clang/")
-
-find_package(Clang REQUIRED CONFIG)
-
-# Sanity check. As Clang does not expose e.g. `CLANG_VERSION_MAJOR` through
-# AddClang.cmake, we have to use LLVM_VERSION_MAJOR instead.
-# TODO: Revisit when next version is released.
-if(NOT "12" VERSION_EQUAL "${LLVM_VERSION_MAJOR}")
- message(FATAL_ERROR "Found LLVM ${LLVM_VERSION_MAJOR}, but need LLVM 12")
-endif()
-
-message(STATUS "Found Clang ${LLVM_PACKAGE_VERSION}")
-message(STATUS "Using ClangConfig.cmake in: ${CP_LLVM_INSTALL_DIR}")
-
-message("CLANG STATUS:
- Includes (clang) ${CLANG_INCLUDE_DIRS}
- Includes (llvm) ${LLVM_INCLUDE_DIRS}"
-)
-
-# Set the LLVM and Clang header and library paths
-include_directories(SYSTEM "${LLVM_INCLUDE_DIRS};${CLANG_INCLUDE_DIRS}")
-
-#===============================================================================
-# 3. BUILD CONFIGURATION
-#===============================================================================
-# Use the same C++ standard as LLVM does
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "")
-
-# -fvisibility-inlines-hidden is set when building LLVM and on Darwin warnings
-# are triggered if llvm-tutor is built without this flag (though otherwise it
-# builds fine). For consistency, add it here too.
-include(CheckCXXCompilerFlag)
-check_cxx_compiler_flag("-fvisibility-inlines-hidden" SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG)
-if (${SUPPORTS_FVISIBILITY_INLINES_HIDDEN_FLAG} EQUAL "1")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")
-endif()
-
-# Set the build directories
-set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/lib")
-
-add_subdirectory(CheckUnusedMaybe)
-
diff --git a/tools/clang-plugin/CheckUnusedMaybe/CMakeLists.txt b/tools/clang-plugin/CheckUnusedMaybe/CMakeLists.txt
deleted file mode 100644
index 15a321fa4..000000000
--- a/tools/clang-plugin/CheckUnusedMaybe/CMakeLists.txt
+++ /dev/null
@@ -1,18 +0,0 @@
-add_library(CheckUnusedMaybe SHARED CheckUnusedMaybe.cpp)
-
-# On Darwin (unlike on Linux), undefined symbols in shared objects are not
-# allowed at the end of the link-edit. The plugins defined here:
-# - _are_ shared objects
-# - reference symbols from LLVM shared libraries, i.e. symbols which are
-# undefined until those shared objects are loaded in memory (and hence
-# _undefined_ during static linking)
-# The build will fail with errors like this:
-# "Undefined symbols for architecture x86_64"
-# with various LLVM symbols being undefined. Since those symbols are later
-# loaded and resolved at runtime, these errors are false positives.
-# This behaviour can be modified via the '-undefined' OS X linker flag as
-# follows.
-target_link_libraries(
- CheckUnusedMaybe
- "$<$<PLATFORM_ID:Darwin>:-undefined dynamic_lookup>"
- )
diff --git a/tools/clang-plugin/CheckUnusedMaybe/CheckUnusedMaybe.cpp b/tools/clang-plugin/CheckUnusedMaybe/CheckUnusedMaybe.cpp
deleted file mode 100644
index 899e185f8..000000000
--- a/tools/clang-plugin/CheckUnusedMaybe/CheckUnusedMaybe.cpp
+++ /dev/null
@@ -1,134 +0,0 @@
-#include "clang/AST/ASTConsumer.h"
-#include "clang/AST/ExprCXX.h"
-#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/AST/Stmt.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendAction.h"
-#include "clang/Frontend/FrontendPluginRegistry.h"
-#include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/CrashRecoveryContext.h"
-
-using namespace clang;
-
-class CheckUnusedMaybeVisitor
- : public RecursiveASTVisitor<CheckUnusedMaybeVisitor> {
-public:
- explicit CheckUnusedMaybeVisitor(ASTContext *Context) : Context(Context) {
- {
- auto skip_filenames_env =
- llvm::StringRef(std::getenv("ONEFLOW_MAYBE_CHECK_SKIP_FN"));
- if (!skip_filenames_env.empty()) {
- skip_filenames_env.split(skip_filenames, ";");
- for (const auto &x : skip_filenames) {
- llvm::outs() << "skip: " << x << "\n";
- }
- }
- }
- {
- auto only_filenames_env =
- llvm::StringRef(std::getenv("ONEFLOW_MAYBE_CHECK_ONLY_FN"));
- if (!only_filenames_env.empty()) {
- only_filenames_env.split(only_filenames, ";");
- for (const auto &x : only_filenames) {
- llvm::outs() << "only: " << x << "\n";
- }
- }
- }
- }
-
- virtual bool VisitCompoundStmt(CompoundStmt *stmt) {
- if (!only_filenames.empty()) {
- bool skip = true;
- for (const auto &x : only_filenames) {
- if (Context->getSourceManager()
- .getFilename(stmt->getBeginLoc())
- .contains(x)) {
- skip = false;
- }
- }
- if (skip) {
- return true;
- }
- }
-
- for (const auto &x : skip_filenames) {
- if (Context->getSourceManager()
- .getFilename(stmt->getBeginLoc())
- .contains(x)) {
- return true;
- }
- }
-
- for (const auto &x : stmt->children()) {
- std::string typeStr;
- if (ExprWithCleanups *expr = dyn_cast<ExprWithCleanups>(x)) {
- typeStr = expr->getType().getAsString();
- }
- if (CallExpr *call = dyn_cast<CallExpr>(x)) {
- llvm::CrashRecoveryContext CRC;
- CRC.RunSafely([&call, &typeStr, this]() {
- QualType returnType;
- if (auto *callee = call->getDirectCallee()) {
- returnType = callee->getReturnType();
- } else {
- returnType = call->getCallReturnType(*this->Context);
- }
- if (!returnType.isNull() && returnType->isClassType()) {
- typeStr = returnType.getAsString();
- }
- });
- }
- if (typeStr.substr(0, 12) == "class Maybe<" ||
- typeStr.substr(0, 6) == "Maybe<") {
- DiagnosticsEngine &DE = Context->getDiagnostics();
- unsigned DiagID =
- DE.getCustomDiagID(DiagnosticsEngine::Error,
- "This function returns Maybe but the return "
- "value is ignored. Wrap it with JUST(..)?");
- auto DB = DE.Report(x->getBeginLoc(), DiagID);
- DB.AddSourceRange(
- clang::CharSourceRange::getCharRange(x->getSourceRange()));
- }
- }
- return true;
- }
-
-private:
- ASTContext *Context;
- llvm::SmallVector<llvm::StringRef, 10> skip_filenames;
- llvm::SmallVector<llvm::StringRef, 10> only_filenames;
-};
-
-class CheckUnusedMaybeConsumer : public clang::ASTConsumer {
-public:
- explicit CheckUnusedMaybeConsumer(ASTContext *Context) : Visitor(Context) {}
-
- void HandleTranslationUnit(clang::ASTContext &Context) override {
- Visitor.TraverseDecl(Context.getTranslationUnitDecl());
- }
-
-private:
- CheckUnusedMaybeVisitor Visitor;
-};
-
-class CheckUnusedMaybeAction : public clang::PluginASTAction {
-public:
- virtual std::unique_ptr<clang::ASTConsumer>
- CreateASTConsumer(clang::CompilerInstance &Compiler,
- llvm::StringRef InFile) override {
- return std::make_unique<CheckUnusedMaybeConsumer>(
- &Compiler.getASTContext());
- }
-
- bool ParseArgs(const CompilerInstance &CI,
- const std::vector<std::string> &args) override {
- return true;
- }
-
- // Automatically run the plugin after the main AST action
- ActionType getActionType() override { return AddAfterMainAction; }
-};
-
-static FrontendPluginRegistry::Add<CheckUnusedMaybeAction>
- X("check-unused-maybe", "Check unused maybe");
diff --git a/tools/clang-plugin/CheckUnusedMaybe/unused_maybe_example.cpp b/tools/clang-plugin/CheckUnusedMaybe/unused_maybe_example.cpp
deleted file mode 100644
index f61664629..000000000
--- a/tools/clang-plugin/CheckUnusedMaybe/unused_maybe_example.cpp
+++ /dev/null
@@ -1,8 +0,0 @@
-template <class T> class Maybe {};
-
-Maybe<void> a() { return Maybe<void>(); }
-
-int main() {
- a();
- return 0;
-}
--
GitLab