Skip to content

contrib/pzstd/Makefile: drop -std=c++11 to fix build of the test#3499

Closed
ldv-alt wants to merge 1 commit intofacebook:devfrom
ldv-alt:dev
Closed

contrib/pzstd/Makefile: drop -std=c++11 to fix build of the test#3499
ldv-alt wants to merge 1 commit intofacebook:devfrom
ldv-alt:dev

Conversation

@ldv-alt
Copy link
Contributor

@ldv-alt ldv-alt commented Feb 11, 2023

It's not just strange to require an older version of the language standard that the compiler supports by default, it's also blocks building the test which requires a newer version of the language standard nowadays:

In file included from /usr/include/gtest/gtest-message.h:57,
from /usr/include/gtest/gtest-assertion-result.h:46,
from /usr/include/gtest/gtest.h:64,
from utils/test/ScopeGuardTest.cpp:11:
/usr/include/gtest/internal/gtest-port.h:270:2: error: #error C++ versions less than C++14 are not supported.

It's not just strange to require an older version of the language
standard that the compiler supports by default, it's also blocks
building the test which requires a newer version of the language
standard nowadays:

  In file included from /usr/include/gtest/gtest-message.h:57,
                   from /usr/include/gtest/gtest-assertion-result.h:46,
                   from /usr/include/gtest/gtest.h:64,
                   from utils/test/ScopeGuardTest.cpp:11:
  /usr/include/gtest/internal/gtest-port.h:270:2: error: #error C++ versions less than C++14 are not supported.
@Cyan4973
Copy link
Contributor

cc @terrelln

@terrelln terrelln assigned terrelln and felixhandte and unassigned terrelln Mar 6, 2023
felixhandte added a commit to felixhandte/zstd that referenced this pull request Mar 27, 2023
Rather than remove the flag entirely, as proposed in facebook#3499, this commit uses
the newest C++ standard the compiler supports. This retains the selection of
using only standardized features (excluding GNU extensions) and keeps the
recency requirements of the codebase explicit.

Tested with various versions of `g++` and `clang++`.
@felixhandte
Copy link
Contributor

We've landed a different fix to this issue, so I'm going to close this PR. Thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants