Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice, that's a lot of code cleaned up! LGTM, but probably should wait for someone from Apple to weigh in. I think the new formatting for those error messages is better for them too but that's not my call :)
Agreed on both counts. While I generally tried to be careful to preserve existing behavior, it seemed to me that "x86_64-apple-macosx10.9" (for example) should have the version reported in error messages as "10.9", not "10.9.0". But definitely need to hear from someone at Apple about that.
clang/test/Sema/attr-availability-android.c | ||
---|---|---|
8 | Observation for reviewers: This is the message we wanted to fix that was the motivation for this change. |
FYI I reverted this due to test failures -- one specific assertion failure is mentioned in https://reviews.llvm.org/rG40d5eeac6cd8.
It is also breaking building llvm-project/llvm/unittests/Support/Host.cpp on macOS: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/25781/consoleFull
FAILED: unittests/Support/CMakeFiles/SupportTests.dir/Host.cpp.o /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iunittests/Support -I/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/unittests/Support -Iinclude -I/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/include -I/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include -I/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -mmacosx-version-min=10.9 -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT unittests/Support/CMakeFiles/SupportTests.dir/Host.cpp.o -MF unittests/Support/CMakeFiles/SupportTests.dir/Host.cpp.o.d -o unittests/Support/CMakeFiles/SupportTests.dir/Host.cpp.o -c /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/unittests/Support/Host.cpp /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/unittests/Support/Host.cpp:428:48: error: too many arguments to function call, expected single argument 'Version', have 3 arguments .getMacOSXVersion(SystemMajor, SystemMinor, SystemMicro), ~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2057:48: note: expanded from macro 'ASSERT_EQ' # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2) ^~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2041:63: note: expanded from macro 'GTEST_ASSERT_EQ' ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2) ^~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:168:36: note: expanded from macro 'ASSERT_PRED_FORMAT2' GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_) ^~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:149:39: note: expanded from macro 'GTEST_PRED_FORMAT2_' GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2), \ ^~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:77:52: note: expanded from macro 'GTEST_ASSERT_' if (const ::testing::AssertionResult gtest_ar = (expression)) \ ^~~~~~~~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/include/llvm/ADT/Triple.h:353:3: note: 'getMacOSXVersion' declared here bool getMacOSXVersion(VersionTuple &Version) const; ^ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/unittests/Support/Host.cpp:431:52: error: too many arguments to function call, expected single argument 'Version', have 3 arguments ASSERT_EQ(HostTriple.getMacOSXVersion(HostMajor, HostMinor, HostMicro), true); ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2057:48: note: expanded from macro 'ASSERT_EQ' # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2) ^~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2041:63: note: expanded from macro 'GTEST_ASSERT_EQ' ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2) ^~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:168:36: note: expanded from macro 'ASSERT_PRED_FORMAT2' GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_) ^~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:149:39: note: expanded from macro 'GTEST_PRED_FORMAT2_' GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2), \ ^~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h:77:52: note: expanded from macro 'GTEST_ASSERT_' if (const ::testing::AssertionResult gtest_ar = (expression)) \ ^~~~~~~~~~ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/llvm/include/llvm/ADT/Triple.h:353:3: note: 'getMacOSXVersion' declared here bool getMacOSXVersion(VersionTuple &Version) const; ^ 2 errors generated. `
Observation for reviewers: This is the message we wanted to fix that was the motivation for this change.