Cut FOLLY_USE_CPP14_CONSTEXPR and FOLLY_CPP14_CONSTEXPR #1029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
@yfeldblum has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
yfeldblum
requested changes
Feb 23, 2019
JoeLoser
force-pushed
the
cut_folly_use_cpp14_constexpr
branch
from
a1c4fae
to
af1544e
Compare
JoeLoser
changed the title
|
This is potentially behavior-changing for ICC since I preserve the value of |
yfeldblum
reviewed
Feb 23, 2019
Summary: - `FOLLY_USE_CPP14_CONSTEXPR` macro is not needed as Folly requires a recent enough version of MSVC where its constexpr support is "good enough". For Clang and GCC, the min compiler versions supported would both evaluate the prior implementation of this macro to true in both cases. - Replace `FOLLY_CPP14_CONSTEXPR` with `constexpr` in all call sites and remove the `FOLLY_CPP14_CONSTEXPR` macro. - Rename `FOLLY_STORAGE_CPP14_CONSTEXPR` to `FOLLY_STORAGE_CONSTEXPR`.
JoeLoser
force-pushed
the
cut_folly_use_cpp14_constexpr
branch
from
af1544e
to
d26d226
Compare
There was a problem hiding this comment.
@yfeldblum has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
sandraiser
pushed a commit
to sandraiser/folly
that referenced
this pull request
Mar 4, 2019
Summary: - `FOLLY_USE_CPP14_CONSTEXPR` macro is not needed as Folly requires a recent enough version of MSVC where its constexpr support is "good enough". For Clang and GCC, the min compiler versions supported would both evaluate the prior implementation of this macro to true in both cases. This is potentially slightly behavior changing since `FOLLY_USE_CPP14_CONSTEXPR` would be `inline` for ICC and I am not sure if its constexpr support is "good enough" for a min version of ICC we claim support for. - Replace `FOLLY_CPP14_CONSTEXPR` with `constexpr` in all call sites and remove the `FOLLY_CPP14_CONSTEXPR` macro. - Simplify how we define `FOLLY_STORAGE_CONSTEXPR` and `FOLLY_STORAGE_CPP14_CONSTEXPR` after cutting `FOLLY_USE_CPP14_CONSTEXPR`. Pull Request resolved: facebook#1029 Reviewed By: Orvid Differential Revision: D14199538 Pulled By: yfeldblum fbshipit-source-id: 99daecf7d7ad0c4bf6735e74247112a78923602a
ahornby
added a commit
to ahornby/folly
that referenced
this pull request
Jul 15, 2024
Summary: These were [failing on github](https://github.com/facebook/folly/actions/runs/9923539772/job/27413784313#step:43:30071) * Fix use of regex in test in FsUtilTest.cpp * Treat MSVC like APPLE in folly::current_exception() to fix ExceptionTest * Fix lookup of executable path on windows, which was breaking SSL test Test Plan: ``` python build/fbcode_builder/getdeps.py build --src-dir=. folly python build/fbcode_builder/getdeps.py test folly ``` before, broken: ``` 1/6 Test facebook#941: ssl_session_test.SSLSessionTest.BasicTest ...................***Failed 0.03 sec Note: Google Test filter = SSLSessionTest.BasicTest [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from SSLSessionTest [ RUN ] SSLSessionTest.BasicTest unknown file: error: C++ exception with description "boost::filesystem::read_symlink: The system cannot find the path specified [system:3]: "/proc/self/exe"" thrown in SetUp(). [ FAILED ] SSLSessionTest.BasicTest (4 ms) [----------] 1 test from SSLSessionTest (4 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (4 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] SSLSessionTest.BasicTest 1 FAILED TEST Start 942: ssl_session_test.SSLSessionTest.NullSessionResumptionTest 2/6 Test facebook#936: HHWheelTimerTest.HHWheelTimerTest.GetTimeRemaining .......... Passed 0.06 sec Start 1029: lang_exception_test.ExceptionTest.current_exception 3/6 Test facebook#942: ssl_session_test.SSLSessionTest.NullSessionResumptionTest ...***Failed 0.04 sec Note: Google Test filter = SSLSessionTest.NullSessionResumptionTest [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from SSLSessionTest [ RUN ] SSLSessionTest.NullSessionResumptionTest unknown file: error: C++ exception with description "boost::filesystem::read_symlink: The system cannot find the path specified [system:3]: "/proc/self/exe"" thrown in SetUp(). [ FAILED ] SSLSessionTest.NullSessionResumptionTest (6 ms) [----------] 1 test from SSLSessionTest (6 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (7 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] SSLSessionTest.NullSessionResumptionTest 1 FAILED TEST 4/6 Test facebook#839: fs_util_test.Simple.UniquePathDefaultModel ..................***Failed 0.08 sec Note: Google Test filter = Simple.UniquePathDefaultModel [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Simple [ RUN ] Simple.UniquePathDefaultModel 5/6 Test facebook#1029: lang_exception_test.ExceptionTest.current_exception .........***Failed 0.02 sec Note: Google Test filter = ExceptionTest.current_exception [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from ExceptionTest [ RUN ] ExceptionTest.current_exception D:\a\folly\folly\folly\lang\test\ExceptionTest.cpp(238): error: Expected equality of these values: std::current_exception() Which is: 16-byte object <20-44 C1-6B 9B-01 00-00 10-44 C1-6B 9B-01 00-00> folly::current_exception() Which is: 16-byte object <30-3B C1-6B 9B-01 00-00 20-3B C1-6B 9B-01 00-00> D:\a\folly\folly\folly\lang\test\ExceptionTest.cpp(247): error: Expected equality of these values: std::current_exception() Which is: 16-byte object <40-3F C1-6B 9B-01 00-00 30-3F C1-6B 9B-01 00-00> folly::current_exception() Which is: 16-byte object <20-44 C1-6B 9B-01 00-00 10-44 C1-6B 9B-01 00-00> [ FAILED ] ExceptionTest.current_exception (0 ms) [----------] 1 test from ExceptionTest (0 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (0 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] ExceptionTest.current_exception 1 FAILED TEST 6/6 Test facebook#838: fs_util_test.Simple.UniquePath ..............................***Failed 0.12 sec Note: Google Test filter = Simple.UniquePath [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Simple [ RUN ] Simple.UniquePath ``` After, works: ``` 1920/1920 Test facebook#813: parallel_test.ParallelTest.PSum ...................................................................... Passed 66.92 sec 100% tests passed, 0 tests failed out of 1919 Total Test time (real) = 67.02 sec The following tests did not run: 1348 - atomic_unordered_map_test.AtomicUnorderedInsertMap.MegaMap (Disabled) ```
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 15, 2024
Summary: These were [failing on github](https://github.com/facebook/folly/actions/runs/9923539772/job/27413784313#step:43:30071) * Fix use of regex in test in FsUtilTest.cpp * MSVC std::current_exception() exception_ptr return values are not comparable, was breaking ExceptionTest * Fix lookup of executable path on windows, which was breaking SSL test Pull Request resolved: #2256 Test Plan: ``` python build/fbcode_builder/getdeps.py build --src-dir=. folly python build/fbcode_builder/getdeps.py test folly ``` before, broken: ``` 1/6 Test #941: ssl_session_test.SSLSessionTest.BasicTest ...................***Failed 0.03 sec Note: Google Test filter = SSLSessionTest.BasicTest [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from SSLSessionTest [ RUN ] SSLSessionTest.BasicTest unknown file: error: C++ exception with description "boost::filesystem::read_symlink: The system cannot find the path specified [system:3]: "/proc/self/exe"" thrown in SetUp(). [ FAILED ] SSLSessionTest.BasicTest (4 ms) [----------] 1 test from SSLSessionTest (4 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (4 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] SSLSessionTest.BasicTest 1 FAILED TEST Start 942: ssl_session_test.SSLSessionTest.NullSessionResumptionTest 2/6 Test #936: HHWheelTimerTest.HHWheelTimerTest.GetTimeRemaining .......... Passed 0.06 sec Start 1029: lang_exception_test.ExceptionTest.current_exception 3/6 Test #942: ssl_session_test.SSLSessionTest.NullSessionResumptionTest ...***Failed 0.04 sec Note: Google Test filter = SSLSessionTest.NullSessionResumptionTest [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from SSLSessionTest [ RUN ] SSLSessionTest.NullSessionResumptionTest unknown file: error: C++ exception with description "boost::filesystem::read_symlink: The system cannot find the path specified [system:3]: "/proc/self/exe"" thrown in SetUp(). [ FAILED ] SSLSessionTest.NullSessionResumptionTest (6 ms) [----------] 1 test from SSLSessionTest (6 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (7 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] SSLSessionTest.NullSessionResumptionTest 1 FAILED TEST 4/6 Test #839: fs_util_test.Simple.UniquePathDefaultModel ..................***Failed 0.08 sec Note: Google Test filter = Simple.UniquePathDefaultModel [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Simple [ RUN ] Simple.UniquePathDefaultModel 5/6 Test #1029: lang_exception_test.ExceptionTest.current_exception .........***Failed 0.02 sec Note: Google Test filter = ExceptionTest.current_exception [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from ExceptionTest [ RUN ] ExceptionTest.current_exception D:\a\folly\folly\folly\lang\test\ExceptionTest.cpp(238): error: Expected equality of these values: std::current_exception() Which is: 16-byte object <20-44 C1-6B 9B-01 00-00 10-44 C1-6B 9B-01 00-00> folly::current_exception() Which is: 16-byte object <30-3B C1-6B 9B-01 00-00 20-3B C1-6B 9B-01 00-00> D:\a\folly\folly\folly\lang\test\ExceptionTest.cpp(247): error: Expected equality of these values: std::current_exception() Which is: 16-byte object <40-3F C1-6B 9B-01 00-00 30-3F C1-6B 9B-01 00-00> folly::current_exception() Which is: 16-byte object <20-44 C1-6B 9B-01 00-00 10-44 C1-6B 9B-01 00-00> [ FAILED ] ExceptionTest.current_exception (0 ms) [----------] 1 test from ExceptionTest (0 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (0 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] ExceptionTest.current_exception 1 FAILED TEST 6/6 Test #838: fs_util_test.Simple.UniquePath ..............................***Failed 0.12 sec Note: Google Test filter = Simple.UniquePath [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Simple [ RUN ] Simple.UniquePath ``` After, works: ``` 1920/1920 Test #813: parallel_test.ParallelTest.PSum ...................................................................... Passed 66.92 sec 100% tests passed, 0 tests failed out of 1919 Total Test time (real) = 67.02 sec The following tests did not run: 1348 - atomic_unordered_map_test.AtomicUnorderedInsertMap.MegaMap (Disabled) ``` Reviewed By: ndmitchell Differential Revision: D59751594 Pulled By: ahornby fbshipit-source-id: 6822abeb3ab73abd4643c2c680855f8f7ba61dbb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
FOLLY_USE_CPP14_CONSTEXPRmacro is not needed as Folly requires arecent enough version of MSVC where its constexpr support is "good
enough". For Clang and GCC, the min compiler versions supported would
both evaluate the prior implementation of this macro to true in both
cases. This is potentially slightly behavior changing since
FOLLY_USE_CPP14_CONSTEXPRwould beinlinefor ICC and I am notsure if its constexpr support is "good enough" for a min version of
ICC we claim support for.
FOLLY_CPP14_CONSTEXPRwithconstexprin all call sites andremove the
FOLLY_CPP14_CONSTEXPRmacro.FOLLY_STORAGE_CONSTEXPRandFOLLY_STORAGE_CPP14_CONSTEXPRafter cuttingFOLLY_USE_CPP14_CONSTEXPR.