Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silence Clang -Wc++20-extensions #737

Merged
merged 1 commit into from
Apr 25, 2020

Conversation

StephanTLavavej
Copy link
Member

In pair, tuple, and optional, we use C++20 conditional explicit (aka explicit(bool)) to simplify constructors and improve compiler throughput, when compiler support is available:

STL/stl/inc/optional

Lines 182 to 193 in 0cbd1b2

#if _HAS_CONDITIONAL_EXPLICIT
template <class _Ty2 = _Ty, enable_if_t<_AllowDirectConversion<_Ty2>::value, int> = 0>
constexpr explicit(!is_convertible_v<_Ty2, _Ty>) optional(_Ty2&& _Right)
: _Mybase(in_place, _STD forward<_Ty2>(_Right)) {}
#else // ^^^ _HAS_CONDITIONAL_EXPLICIT ^^^ / vvv !_HAS_CONDITIONAL_EXPLICIT vvv
template <class _Ty2 = _Ty,
enable_if_t<conjunction_v<_AllowDirectConversion<_Ty2>, is_convertible<_Ty2, _Ty>>, int> = 0>
constexpr optional(_Ty2&& _Right) : _Mybase(in_place, _STD forward<_Ty2>(_Right)) {}
template <class _Ty2 = _Ty,
enable_if_t<conjunction_v<_AllowDirectConversion<_Ty2>, negation<is_convertible<_Ty2, _Ty>>>, int> = 0>
constexpr explicit optional(_Ty2&& _Right) : _Mybase(in_place, _STD forward<_Ty2>(_Right)) {}
#endif // ^^^ !_HAS_CONDITIONAL_EXPLICIT ^^^

Like C++17 if constexpr, MSVC supports C++20 explicit(bool) in older Standard modes, with a suppressible warning. (This is because the feature requires novel syntax, so it can't be accidentally used, and it's extremely useful to library developers.) We suppress both warnings in STL headers:

STL/stl/inc/yvals_core.h

Lines 368 to 380 in 0cbd1b2

// warning C4984: 'if constexpr' is a C++17 language extension
#if !_HAS_CXX17 && _HAS_IF_CONSTEXPR
#define _STL_DISABLED_WARNING_C4984 4984
#else // !_HAS_CXX17 && _HAS_IF_CONSTEXPR
#define _STL_DISABLED_WARNING_C4984
#endif // !_HAS_CXX17 && _HAS_IF_CONSTEXPR
// warning C5053: support for 'explicit(<expr>)' in C++17 and earlier is a vendor extension
#if !_HAS_CXX20 && _HAS_CONDITIONAL_EXPLICIT
#define _STL_DISABLED_WARNING_C5053 5053
#else // !_HAS_CXX20 && _HAS_CONDITIONAL_EXPLICIT
#define _STL_DISABLED_WARNING_C5053
#endif // !_HAS_CXX20 && _HAS_CONDITIONAL_EXPLICIT

Clang 9 supported explicit(bool) in C++20 mode, while Clang 10 added "downlevel" support similar to MSVC. When #708 changed the STL to require Clang 10, it also began using Clang 10's downlevel support for explicit(bool).

@cbezault discovered that we forgot to suppress the warning that Clang emits for explicit(bool) in older Standard modes. We missed this because Clang (unlike MSVC) ordinarily suppresses warnings from "system headers". (-Wsystem-headers activates warnings in system headers, although this is rarely used because users are rarely interested in such warnings, and UCRT/WinSDK headers are still being cleaned up.)

This is a practical issue because anything on the INCLUDE path is considered to be a system header for warning purposes, while /I directories aren't (despite #include <meow> searching both INCLUDE and /I). During STL development, it is very convenient to be able to compile with /I S:\GitHub\STL\stl\inc and immediately consume updated STL headers without rebuilding (this is not possible when separately compiled changes are involved, of course), which will emit Clang warnings.

After this lengthy backstory, the fix is delightfully simple.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Apr 22, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner April 22, 2020 23:23
@BillyONeal
Copy link
Member

:sigh: I really wish clang had a 'there are no system headers' option :(

@CaseyCarter CaseyCarter self-assigned this Apr 23, 2020
@CaseyCarter CaseyCarter changed the title Silence Clang -Wc++20-extensions. Silence Clang -Wc++20-extensions Apr 23, 2020
@CaseyCarter CaseyCarter merged commit b64eeed into microsoft:master Apr 25, 2020
@CaseyCarter
Copy link
Member

Thanks for the cleanup!

@miscco
Copy link
Contributor

miscco commented Apr 26, 2020

If we allow C++17 if constexpr in older code why are there still so many #ifdefs?
Is it solely because of EDG or could one get rid of all that old tag dispatch?

@CaseyCarter
Copy link
Member

If we allow C++17 if constexpr in older code why are there still so many #ifdefs?

Not all of our front ends allow if constexpr in C++14 mode: CUDA 9.2 is now the remaining problem child. Once we're done updating our required CUDA version to 10.1 update 2 (#639) we can remove the #ifdefs (#189).

@StephanTLavavej StephanTLavavej deleted the silence branch April 29, 2020 09:45
@CaseyCarter CaseyCarter removed their assignment Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants