Page MenuHomePhabricator

[libcxx] Enable C++17 for the benchmarks
ClosedPublic

Authored by Mordante on Mar 10 2020, 12:54 PM.

Details

Reviewers
ldionne
EricWF
mclow.lists
Group Reviewers
Restricted Project
Commits
rGd184d0226301: [libcxx] Enable C++17 for the benchmarks.
Summary

The benchmarks are intended to be build with C++17 but the CMAKE_CXX_STANDARD in the LLVM forces the build to use C++14 by default. This fixes the issue by setting the CXX_STANDARD property of the benchmark targets.

The CMake documentation is not clear whether this will use the C++1z fallback for older compilers. So this may break the benchmarks if somebody uses the benchmarks with pre C++17 compilers with the C++1z fallback.

This was discovered while investigating the build issue of D62778.

Diff Detail

Event Timeline

Mordante created this revision.Mar 10 2020, 12:54 PM
Mordante updated this revision to Diff 249485.Mar 10 2020, 1:16 PM

Noticed I didn't stage my last hunk. Don't use BENCHMARK_DIALECT_FLAG which no longer is set.

ldionne accepted this revision.Mar 11 2020, 12:04 PM

LGTM since CXX_STANDARD 17 will decay to an older standard if the compiler doesn't support -std=c++17. It seems to be the behavior we want (although I would argue that setting CXX_STANDARD_REQUIRED would also make sense).

This revision is now accepted and ready to land.Mar 11 2020, 12:04 PM
EricWF accepted this revision.Mar 11 2020, 2:37 PM
This revision was automatically updated to reflect the committed changes.
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2020, 1:36 PM

CXX_STANDARD_REQUIRED is already set by the main LLVM CMake script.

Seems it requires CMake 3.8 to use CXX_STANDARD 17, so reverted the commit for now.