dendibakh / perf-ninja

This is an online course where you can learn and master the skill of low-level performance analysis and tuning.
2.61k stars 229 forks source link

Building benchmark on MacOS Sequoia 15.0 #97

Open ragliam opened 2 months ago

ragliam commented 2 months ago

Hi! When I follow steps from https://github.com/dendibakh/perf-ninja/blob/main/QuickstartMacOS.md, I encounter following error:

[ 86%] Building CXX object test/CMakeFiles/benchmark_gtest.dir/benchmark_gtest.cc.o
/Users/***/Downloads/perf-ninja/benchmark/test/benchmark_gtest.cc:15:1: error: use of the 'maybe_unused' attribute is a C++17 extension [-Werror,-Wc++17-attribute-extensions]
   15 | TEST(AddRangeTest, Simple) {
      | ^
/Users/***/Downloads/perf-ninja/benchmark/googletest/googletest/include/gtest/gtest.h:2192:42: note: expanded from macro 'TEST'
 2192 | #define TEST(test_suite_name, test_name) GTEST_TEST(test_suite_name, test_name)
      |                                          ^
/Users/***/Downloads/perf-ninja/benchmark/googletest/googletest/include/gtest/gtest.h:2186:3: note: expanded from macro 'GTEST_TEST'
 2186 |   GTEST_TEST_(test_suite_name, test_name, ::testing::Test, \
      |   ^
/Users/***/Downloads/perf-ninja/benchmark/googletest/googletest/include/gtest/internal/gtest-internal.h:1504:5: note: expanded from macro 'GTEST_TEST_'
 1504 |     GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static ::testing::TestInfo* const    \
      |     ^
/Users/***/Downloads/perf-ninja/benchmark/googletest/googletest/include/gtest/internal/gtest-port.h:784:49: note: expanded from macro 'GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED'
  784 | #define GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED [[maybe_unused]]
      |                                                 ^

My versions are:

% clang --version
Homebrew clang version 17.0.6
Target: arm64-apple-darwin24.0.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm@17/bin
% cmake --version
cmake version 3.30.3
liangzhongkai commented 2 months ago

您的来信我已收到,谢谢

ragliam commented 2 months ago

Tried to derive minimal reproducible example:

#include <iostream>

#if __has_cpp_attribute(maybe_unused)
#define MAYBE_UNUSED [[maybe_unused]]
#define LOG 1
#else 
#define MAYBE_UNUSED
#define LOG 0
#endif

int main() {
    std::cout << LOG << std::endl;

    MAYBE_UNUSED int c = 1;
}

and compile with `clang++ test.cpp -o test -std=c++14 -Wall -Werror' and it worked.

Also checked that CMake uses correct clang.

ragliam commented 2 months ago

Example fails after adding -pedantic flag.

ragliam commented 2 months ago

Fixed compilation error by cancelling the warning:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a86a568..b2572fa 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -220,6 +220,10 @@ else()
     add_cxx_compiler_flag(-fno-exceptions)
   endif()

+  if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
+    add_cxx_compiler_flag(-Wno-c++17-attribute-extensions)
+  endif()
+
   if (HAVE_CXX_FLAG_FSTRICT_ALIASING)
     if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "Intel" AND NOT CMAKE_CXX_COMPILER_ID STREQUAL "IntelLLVM") #ICC17u2: Many false positives for Wstrict-aliasing
       add_cxx_compiler_flag(-Wstrict-aliasing)

But what would be the proper solution?

dendibakh commented 2 months ago

Hi @ragliam , sorry for the late reply. I haven't been building google benchmark for a long time actually, so I haven't seen this issue before. I think you better ask this question in https://github.com/google/benchmark, as there is nothing to fix on the perf-ninja side (I suppose :)).