Added new checks
- performance-noexcept-destructor
- performance-noexcept-swap
Also added cppcoreguidlines aliases for the 2 new checks as well as performance-noexcept-move-constructor
This fixes llvm#62154
Differential D148697
[clang-tidy] Add more checks for functions which should be noexcept AMS21 on Apr 19 2023, 3:00 AM. Authored by
Details Added new checks
Also added cppcoreguidlines aliases for the 2 new checks as well as performance-noexcept-move-constructor This fixes llvm#62154
Diff Detail
Event TimelineComment Actions Few issues, first we should have single check for a single cppcoreguidelines rule. This is why better would be to rename this check into performance-noexcept-move-special-functions, and add other checks for swap, hash, default constructor. If you want to keep single check, I'm fine with it, but consider adding option to disable some parts of it (mainly default constructors), and consider including (not necessary in this patch, all compare operators). Give it a thing...
Comment Actions First of all thank you for the contribution! I had just a quick look so here's some very preliminary comments, will have more time to deeply review during the weekend:
Comment Actions Well first of all thanks for the feedback. I agree not renaming a check which is already used and splitting each special case into it's own check seems like the easier and better way to go. Comment Actions Split the changes into 2 new checks and added cppcoreguidelines aliases for all of them. Comment Actions On first glance, looks fine, but I didn't check it too deep. One thing that I see and do not like is duplication, maybe we could extract some "base check" from those 3 (in next step).
Comment Actions Rename cppcoreguidelines-noexcept-move-constructor to cppcoreguidelines-noexcept-move-operations Comment Actions Patch is now open for 2+ weeks. If there are no more reviews, I would kindly ask for someone to push this on my behalf. Comment Actions +-LGTM Only one thing that could be still done here, is to extract some base class from those checks, and move common code there.
Comment Actions Implement the suggested changes
Comment Actions I'm getting a linking error by some of the files changed in this revision. ld.lld: error: undefined symbol: vtable for clang::tidy::performance::NoexceptDestructorCheck >>> referenced by NoexceptDestructorCheck.h:26 (/home/christianu/repos/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/../performance/NoexceptDestructorCheck.h:26) >>> tools/clang/tools/extra/clang-tidy/cppcoreguidelines/CMakeFiles/obj.clangTidyCppCoreGuidelinesModule.dir/CppCoreGuidelinesTidyModule.cpp.o:(std::enable_if<is_invocable_r_v<std::uni que_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::registerCheck<clang::tidy::performance::NoexceptDestructorCheck>(ll vm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>, std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang:: tidy::ClangTidyCheck>>>::type std::__invoke_r<std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::registerCheck <clang::tidy::performance::NoexceptDestructorCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>(void clang::tidy::C langTidyCheckFactories::registerCheck<clang::tidy::performance::NoexceptDestructorCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef&&, clang::tidy: :ClangTidyContext*&&)) >>> the vtable symbol may be undefined because the class is missing its key function (see https://lld.llvm.org/missingkeyfunction) ld.lld: error: undefined symbol: vtable for clang::tidy::performance::NoexceptMoveConstructorCheck >>> referenced by NoexceptMoveConstructorCheck.h:30 (/home/christianu/repos/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/../performance/NoexceptMoveConstructorCheck.h:30) >>> tools/clang/tools/extra/clang-tidy/cppcoreguidelines/CMakeFiles/obj.clangTidyCppCoreGuidelinesModule.dir/CppCoreGuidelinesTidyModule.cpp.o:(std::enable_if<is_invocable_r_v<std::uni que_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::registerCheck<clang::tidy::performance::NoexceptMoveConstructorChec k>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>, std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<cl ang::tidy::ClangTidyCheck>>>::type std::__invoke_r<std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::register Check<clang::tidy::performance::NoexceptMoveConstructorCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>(void clan g::tidy::ClangTidyCheckFactories::registerCheck<clang::tidy::performance::NoexceptMoveConstructorCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef& &, clang::tidy::ClangTidyContext*&&)) >>> the vtable symbol may be undefined because the class is missing its key function (see https://lld.llvm.org/missingkeyfunction) ld.lld: error: undefined symbol: vtable for clang::tidy::performance::NoexceptSwapCheck >>> referenced by NoexceptSwapCheck.h:26 (/home/christianu/repos/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/../performance/NoexceptSwapCheck.h:26) >>> tools/clang/tools/extra/clang-tidy/cppcoreguidelines/CMakeFiles/obj.clangTidyCppCoreGuidelinesModule.dir/CppCoreGuidelinesTidyModule.cpp.o:(std::enable_if<is_invocable_r_v<std::uni que_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::registerCheck<clang::tidy::performance::NoexceptSwapCheck>(llvm::St ringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>, std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy:: ClangTidyCheck>>>::type std::__invoke_r<std::unique_ptr<clang::tidy::ClangTidyCheck, std::default_delete<clang::tidy::ClangTidyCheck>>, void clang::tidy::ClangTidyCheckFactories::registerCheck<clang ::tidy::performance::NoexceptSwapCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef, clang::tidy::ClangTidyContext*>(void clang::tidy::ClangTidyChec kFactories::registerCheck<clang::tidy::performance::NoexceptSwapCheck>(llvm::StringRef)::'lambda'(llvm::StringRef, clang::tidy::ClangTidyContext*)&, llvm::StringRef&&, clang::tidy::ClangTidyContext* &&)) >>> the vtable symbol may be undefined because the class is missing its key function (see https://lld.llvm.org/missingkeyfunction) clang-16: error: linker command failed with exit code 1 (use -v to see invocation) I'm building with shared libraries on. Comment Actions You shouldn't because this library got dependency on clangTidyPerformanceModule. Comment Actions I did a clean build with shared libraries enabled, everything is working. Comment Actions @Dinistro It compiles & links with clang 16 also (shared libs). So I'm unable to reproduce. Comment Actions I tried with a clean build, and I get the exact same issues. Might be caused by the lld and clang-16 combination, not sure. Comment Actions I used the following command to build: cmake -S/home/christianu/repos/llvm-project/llvm -B/home/christianu/repos/llvm-project/build-clang-tidy -GNinja -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_ASSERTIONS=ON -DBUILD_SHARED_LIBS=ON -DLLVM_INCLUDE_UTILS=ON -DLLVM_INSTALL_UTILS=ON -DLLVM_BUILD_EXAMPLES=OFF -DLLVM_CCACHE_BUILD=ON -DLLVM_OPTIMIZED_TABLEGEN=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ENABLE_RUNTIMES=openmp Comment Actions Still no issue on my side. Try cleaning up ccache and directory. Also by looking into code, everything looks fine, dependencies are correctly set. Comment Actions It was also causing PPC bots to break. I fixed it in https://reviews.llvm.org/rGffd7a200fdfbd01ef296101647d2f2da91ddfd41 Comment Actions O, thank you. Thats explain a lot. Because when I look into cmake, clangTidyPerformanceModule were already there because you already added it. Comment Actions Yeah, sorry. I saw that Phabricator automatically added the mention into the Diffusion link but I forgot that it won't add it to the Differential and that I need to add it manually. Comment Actions Sorry for the noise. I didn't pull in between the different runs and was still on the broken version.
|