From bcb8a7c52914e2a95caf00d7219bf396dbbb6ac0 Mon Sep 17 00:00:00 2001 From: mcbarton <150042563+mcbarton@users.noreply.github.com> Date: Wed, 15 May 2024 20:32:44 +0100 Subject: [PATCH 1/2] Treat all warnings as errors --- .github/workflows/ci.yml | 6 ++++- CMakeLists.txt | 9 ++++++- lib/Interpreter/Compatibility.h | 1 + lib/Interpreter/CppInterOp.cpp | 14 +++++------ lib/Interpreter/DynamicLibraryManager.cpp | 3 --- .../DynamicLibraryManagerSymbol.cpp | 2 +- unittests/CppInterOp/ScopeReflectionTest.cpp | 18 +++++++------- .../CppInterOp/VariableReflectionTest.cpp | 24 ++++++++++--------- 8 files changed, 44 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72125e309..13f6332d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -819,6 +819,7 @@ jobs: -DBUILD_SHARED_LIBS=ON \ -DCODE_COVERAGE=${{ env.CODE_COVERAGE }} \ -DCMAKE_INSTALL_PREFIX=$CPPINTEROP_DIR \ + -DLLVM_ENABLE_WERROR=On \ ../ else cmake -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ @@ -829,6 +830,7 @@ jobs: -DBUILD_SHARED_LIBS=ON \ -DCODE_COVERAGE=${{ env.CODE_COVERAGE }} \ -DCMAKE_INSTALL_PREFIX=$CPPINTEROP_DIR \ + -DLLVM_ENABLE_WERROR=On \ ../ fi os="${{ matrix.os }}" @@ -844,7 +846,7 @@ jobs: echo "CPPINTEROP_DIR=$CPPINTEROP_DIR" >> $GITHUB_ENV echo "LLVM_BUILD_DIR=$LLVM_BUILD_DIR" >> $GITHUB_ENV echo "CPLUS_INCLUDE_PATH=$CPLUS_INCLUDE_PATH" >> $GITHUB_ENV - + - name: Build and Test/Install CppInterOp on Windows systems continue-on-error: true if: ${{ runner.os == 'windows' }} @@ -903,6 +905,7 @@ jobs: -DUSE_REPL=OFF ` -DCling_DIR="$env:LLVM_BUILD_DIR\tools\cling" ` -DLLVM_DIR="$env:LLVM_BUILD_DIR" ` + -DLLVM_ENABLE_WERROR=On ` -DClang_DIR="$env:LLVM_BUILD_DIR" -DCODE_COVERAGE=${{ env.CODE_COVERAGE }} -DCMAKE_INSTALL_PREFIX="$env:CPPINTEROP_DIR" ..\ } else @@ -911,6 +914,7 @@ jobs: -DUSE_CLING=OFF ` -DUSE_REPL=ON ` -DLLVM_DIR="$env:LLVM_BUILD_DIR\lib\cmake\llvm" ` + -DLLVM_ENABLE_WERROR=On ` -DClang_DIR="$env:LLVM_BUILD_DIR\lib\cmake\clang" -DCODE_COVERAGE=${{ env.CODE_COVERAGE }} -DCMAKE_INSTALL_PREFIX="$env:CPPINTEROP_DIR" ..\ } cmake --build . --config ${{ env.BUILD_TYPE }} --target check-cppinterop --parallel ${{ env.ncpus }} diff --git a/CMakeLists.txt b/CMakeLists.txt index ca7a5bd69..951c02a53 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -211,7 +211,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR ) endif() - ##Replace \ with / in LLVM_DIR (attempt to fix path parsing issue Windows) + #Replace \ with / in LLVM_DIR (attempt to fix path parsing issue Windows) string(REPLACE "\\" "/" LLVM_DIR "${LLVM_DIR}") # When in debug mode the llvm package thinks it is built with -frtti. @@ -302,6 +302,11 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-common -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing -pedantic -Wno-long-long -Wall -W -Wno-unused-parameter -Wwrite-strings") endif () +# Fixes "C++ exception handler used, but unwind semantics are not enabled" warning Windows +if (WIN32) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc") +endif () + if (APPLE) set(CMAKE_MODULE_LINKER_FLAGS "-Wl,-flat_namespace -Wl,-undefined -Wl,suppress") endif () @@ -319,6 +324,8 @@ include_directories(BEFORE SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/include ) +#Removes flag due to issue with Google test download when LLVM_ENABLE_WERROR=On +string(REPLACE "-Wcovered-switch-default" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") file(STRINGS "VERSION" CPPINTEROP_VERSION) string(REGEX MATCH "([0-9]*)\.([0-9]*)\.([0-9]*)" CPPINTEROP_VERSION_ONLY "${CPPINTEROP_VERSION}") diff --git a/lib/Interpreter/Compatibility.h b/lib/Interpreter/Compatibility.h index 9291d67ba..ecc5a3be1 100644 --- a/lib/Interpreter/Compatibility.h +++ b/lib/Interpreter/Compatibility.h @@ -13,6 +13,7 @@ #if LLVM_VERSION_MAJOR < 18 #define starts_with startswith #define ends_with endswith +#define starts_with_insensitive startswith_insensitive #endif #if CLANG_VERSION_MAJOR >= 18 diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 7ae53d6c1..bbeb7ed8b 100644 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -1204,7 +1204,8 @@ namespace Cpp { for (auto &GV : GeneratedPTU->TheModule->globals()) { llvm::GlobalValue::LinkageTypes LT = GV.getLinkage(); if (GV.isDeclaration() || !GV.hasName() || - GV.getName().startswith(".str") || !GV.isDiscardableIfUnused(LT) || + GV.getName().starts_with(".str") || + !GV.isDiscardableIfUnused(LT) || LT != llvm::GlobalValue::InternalLinkage) continue; //nothing to do GV.setLinkage(llvm::GlobalValue::WeakAnyLinkage); @@ -1333,11 +1334,11 @@ namespace Cpp { { bool issigned = false; bool isunsigned = false; - if (typeName.startswith("signed ")) { + if (typeName.starts_with("signed ")) { issigned = true; typeName = StringRef(typeName.data()+7, typeName.size()-7); } - if (!issigned && typeName.startswith("unsigned ")) { + if (!issigned && typeName.starts_with("unsigned ")) { isunsigned = true; typeName = StringRef(typeName.data()+9, typeName.size()-9); } @@ -2813,7 +2814,6 @@ namespace Cpp { } // Let's inject it. - SymbolMap::iterator It; llvm::orc::SymbolMap InjectedSymbols; auto& DL = compat::getExecutionEngine(I)->getDataLayout(); char GlobalPrefix = DL.getGlobalPrefix(); @@ -3078,7 +3078,7 @@ namespace Cpp { std::string GetFunctionArgDefault(TCppFunction_t func, TCppIndex_t param_index) { auto *D = (clang::Decl *)func; - clang::ParmVarDecl* PI; + clang::ParmVarDecl* PI = nullptr; if (auto* FD = llvm::dyn_cast_or_null(D)) PI = FD->getParamDecl(param_index); @@ -3125,7 +3125,7 @@ namespace Cpp { std::string GetFunctionArgName(TCppFunction_t func, TCppIndex_t param_index) { auto *D = (clang::Decl *)func; - clang::ParmVarDecl* PI; + clang::ParmVarDecl* PI = nullptr; if (auto* FD = llvm::dyn_cast_or_null(D)) PI = FD->getParamDecl(param_index); @@ -3154,7 +3154,7 @@ namespace Cpp { auto* const Ctor = GetDefaultConstructor(Class); if (JitCall JC = MakeFunctionCallable(Ctor)) { if (arena) { - JC.Invoke(&arena, {}, (void *)~0U); // Tell Invoke to use placement new. + JC.Invoke(&arena, {}, (void*)0); // Tell Invoke to use placement new. return arena; } diff --git a/lib/Interpreter/DynamicLibraryManager.cpp b/lib/Interpreter/DynamicLibraryManager.cpp index bfa156edf..6748be2be 100644 --- a/lib/Interpreter/DynamicLibraryManager.cpp +++ b/lib/Interpreter/DynamicLibraryManager.cpp @@ -69,9 +69,6 @@ namespace Cpp { for (const std::string& P : SysPaths) addSearchPath(P, /*IsUser*/ false); } -#if LLVM_VERSION_MAJOR < 18 -#define starts_with_insensitive startswith_insensitive -#endif ///\returns substitution of pattern in the front of original with replacement /// Example: substFront("@rpath/abc", "@rpath/", "/tmp") -> "/tmp/abc" static std::string substFront(StringRef original, StringRef pattern, diff --git a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp index e523d10b5..ac1857fa5 100644 --- a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp +++ b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp @@ -93,7 +93,7 @@ struct BloomFilter { // bloomSize = ceil((-1.44 * n * log2f(p)) / bits) const int m_Bits = 8 * sizeof(uint64_t); - const float m_P = 0.02; + const float m_P = 0.02f; bool m_IsInitialized = false; uint32_t m_SymbolsCount = 0; diff --git a/unittests/CppInterOp/ScopeReflectionTest.cpp b/unittests/CppInterOp/ScopeReflectionTest.cpp index f6dc8234a..559a5c3bd 100644 --- a/unittests/CppInterOp/ScopeReflectionTest.cpp +++ b/unittests/CppInterOp/ScopeReflectionTest.cpp @@ -663,15 +663,15 @@ TEST(ScopeReflectionTest, GetBaseClassOffset) { #define Stringify(s) Stringifyx(s) #define Stringifyx(...) #__VA_ARGS__ #define CODE \ - class A { int m_a; }; \ - class B { int m_b; }; \ - class C : virtual A, virtual B { int m_c; }; \ - class D : virtual A, virtual B, public C { int m_d; }; \ - class E : public A, public B { int m_e; }; \ - class F : public A { int m_f; }; \ - class G : public F { int m_g; }; - - CODE; + struct A { int m_a; }; \ + struct B { int m_b; }; \ + struct C : virtual A, virtual B { int m_c; }; \ + struct D : virtual A, virtual B, public C { int m_d; }; \ + struct E : public A, public B { int m_e; }; \ + struct F : public A { int m_f; }; \ + struct G : public F { int m_g; }; + +CODE; GetAllTopLevelDecls(Stringify(CODE), Decls); #undef Stringifyx diff --git a/unittests/CppInterOp/VariableReflectionTest.cpp b/unittests/CppInterOp/VariableReflectionTest.cpp index 700ac7a45..3992f581c 100644 --- a/unittests/CppInterOp/VariableReflectionTest.cpp +++ b/unittests/CppInterOp/VariableReflectionTest.cpp @@ -93,7 +93,7 @@ TEST(VariableReflectionTest, GetVariableType) { #define CODE \ int a; \ - const int N = 5; \ + int N = 5; \ class C { \ public: \ int a; \ @@ -121,12 +121,13 @@ TEST(VariableReflectionTest, GetVariableOffset) { EXPECT_TRUE((bool) Cpp::GetVariableOffset(Decls[1])); // N EXPECT_EQ(Cpp::GetVariableOffset(datamembers[0]), 0); + EXPECT_EQ(Cpp::GetVariableOffset(datamembers[1]), - ((unsigned long) &(c.b)) - ((unsigned long) &(c.a))); + ((intptr_t) &(c.b)) - ((intptr_t) &(c.a))); EXPECT_EQ(Cpp::GetVariableOffset(datamembers[2]), - ((unsigned long) &(c.c)) - ((unsigned long) &(c.a))); + ((intptr_t) &(c.c)) - ((intptr_t) &(c.a))); EXPECT_EQ(Cpp::GetVariableOffset(datamembers[3]), - ((unsigned long) &(c.d)) - ((unsigned long) &(c.a))); + ((intptr_t) &(c.d)) - ((intptr_t) &(c.a))); auto *VD_C_s_a = Cpp::GetNamed("s_a", Decls[2]); // C::s_a EXPECT_TRUE((bool) Cpp::GetVariableOffset(VD_C_s_a)); @@ -240,13 +241,14 @@ TEST(VariableReflectionTest, DISABLED_GetArrayDimensions) { GetAllTopLevelDecls(code, Decls); - auto is_vec_eq = [](const std::vector &arr_dims, - const std::vector &truth_vals) { - if (arr_dims.size() != truth_vals.size()) - return false; - - return std::equal(arr_dims.begin(), arr_dims.end(), truth_vals.begin()); - }; + //FIXME: is_vec_eq is an unused variable until test does something + //auto is_vec_eq = [](const std::vector &arr_dims, + // const std::vector &truth_vals) { + // if (arr_dims.size() != truth_vals.size()) + // return false; + // + // return std::equal(arr_dims.begin(), arr_dims.end(), truth_vals.begin()); + //}; // EXPECT_TRUE(is_vec_eq(Cpp::GetArrayDimensions(Decls[0]), {})); // EXPECT_TRUE(is_vec_eq(Cpp::GetArrayDimensions(Decls[1]), {1})); From dd5f7d9fa1cccaf4b0418c630065bed2dc7d0e9c Mon Sep 17 00:00:00 2001 From: mcbarton <150042563+mcbarton@users.noreply.github.com> Date: Sun, 19 May 2024 08:15:34 +0100 Subject: [PATCH 2/2] Update VariableReflectionTest.cpp --- unittests/CppInterOp/VariableReflectionTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unittests/CppInterOp/VariableReflectionTest.cpp b/unittests/CppInterOp/VariableReflectionTest.cpp index 3992f581c..2c42897dd 100644 --- a/unittests/CppInterOp/VariableReflectionTest.cpp +++ b/unittests/CppInterOp/VariableReflectionTest.cpp @@ -93,7 +93,7 @@ TEST(VariableReflectionTest, GetVariableType) { #define CODE \ int a; \ - int N = 5; \ + const int N = 5; \ class C { \ public: \ int a; \ @@ -102,7 +102,7 @@ TEST(VariableReflectionTest, GetVariableType) { int d; \ static int s_a; \ } c; \ - int C::s_a = 12; + int C::s_a = 7 + N; CODE