Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5c6ff2e

Browse filesBrowse files
aaronpuchertaaronpuchert
committed
Suppress errors from well-formed-testing type traits in SFINAE contexts
There are several type traits that produce a boolean value or type based on the well-formedness of some expression (more precisely, the immediate context, i.e. for example excluding nested template instantiation): * `__is_constructible` and variants, * `__is_convertible` and variants, * `__is_assignable` and variants, * `__reference_{binds_to,{constructs,converts}_from}_temporary`, * `__is_trivially_equality_comparable`, * `__builtin_common_type`. (It should be noted that the standard doesn't always base this on the immediate context being well-formed: for `std::common_type` it's based on whether some expression "denotes a valid type." But I assume that's an editorial issue and means the same thing.) Errors in the immediate context are suppressed, instead the type traits return another value or produce a different type if the expression is not well-formed. This is achieved using an `SFINAETrap` with `AccessCheckingSFINAE` set to true. If the type trait is used outside of an SFINAE context, errors are discarded because in that case the `SFINAETrap` sets `InNonInstantiationSFINAEContext`, which makes `isSFINAEContext` return an `optional(nullptr)`, which causes the errors to be discarded in `EmitDiagnostic`. However, in an SFINAE context this doesn't happen, and errors are added to `SuppressedDiagnostics` in the `TemplateDeductionInfo` returned by `isSFINAEContext`. Once we're done with deducing template arguments and have decided which template is going to be instantiated, the errors corresponding to the chosen template are then emitted. At this point we get errors from those type traits that we wouldn't have seen if used with the same arguments outside of an SFINAE context. That doesn't seem right. So what we want to do is always set `InNonInstantiationSFINAEContext` when evaluating these well-formed-testing type traits, regardless of whether we're in an SFINAE context or not. This should only affect the immediate context, as nested contexts add a new `CodeSynthesisContext` that resets `InNonInstantiationSFINAEContext` for the time it's active. Going through uses of `SFINAETrap` with `AccessCheckingSFINAE` = `true`, it occurred to me that all of them want this behavior and we can just use this parameter to decide whether to use a non-instantiation context. The uses are precisely the type traits mentioned above plus the `TentativeAnalysisScope`, where I think it is also fine. (Though I think we don't do tentative analysis in SFINAE contexts anyway.) Because the parameter no longer just sets `AccessCheckingSFINAE` in Sema but also `InNonInstantiationSFINAEContext`, I think it should be renamed (along with uses, which also point the reviewer to the affected places). Since we're testing for well-formedness of some expression, I think `WellFormedSFINAE` is a good new name. The added tests should more or less correspond to the users of `SFINAETrap` with `AccessCheckingSFINAE` = `true`. I added a test for errors outside of the immediate context for only one type trait, because it requires some setup and is relatively noisy. We put the `WellFormedSFINAE` condition first because it's constant in all uses and would allow the compiler to prune the call to `isSFINAEContext` when true. Fixes #132044.
1 parent 22c3dac commit 5c6ff2e
Copy full SHA for 5c6ff2e

File tree

Expand file treeCollapse file tree

6 files changed

+79
-11
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+79
-11
lines changed

‎clang/include/clang/Sema/Sema.h

Copy file name to clipboardExpand all lines: clang/include/clang/Sema/Sema.h
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12240,16 +12240,16 @@ class Sema final : public SemaBase {
1224012240
bool PrevLastDiagnosticIgnored;
1224112241

1224212242
public:
12243-
explicit SFINAETrap(Sema &SemaRef, bool AccessCheckingSFINAE = false)
12243+
explicit SFINAETrap(Sema &SemaRef, bool TestWellformedSFINAE = false)
1224412244
: SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors),
1224512245
PrevInNonInstantiationSFINAEContext(
1224612246
SemaRef.InNonInstantiationSFINAEContext),
1224712247
PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE),
1224812248
PrevLastDiagnosticIgnored(
1224912249
SemaRef.getDiagnostics().isLastDiagnosticIgnored()) {
12250-
if (!SemaRef.isSFINAEContext())
12250+
if (TestWellformedSFINAE || !SemaRef.isSFINAEContext())
1225112251
SemaRef.InNonInstantiationSFINAEContext = true;
12252-
SemaRef.AccessCheckingSFINAE = AccessCheckingSFINAE;
12252+
SemaRef.AccessCheckingSFINAE = TestWellformedSFINAE;
1225312253
}
1225412254

1225512255
~SFINAETrap() {
@@ -12279,7 +12279,7 @@ class Sema final : public SemaBase {
1227912279

1228012280
public:
1228112281
explicit TentativeAnalysisScope(Sema &SemaRef)
12282-
: SemaRef(SemaRef), Trap(SemaRef, true),
12282+
: SemaRef(SemaRef), Trap(SemaRef, /*TestWellformedSFINAE=*/true),
1228312283
PrevDisableTypoCorrection(SemaRef.DisableTypoCorrection) {
1228412284
SemaRef.DisableTypoCorrection = true;
1228512285
}

‎clang/lib/Sema/SemaConcept.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaConcept.cpp
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
908908
if (MLTAL.getNumSubstitutedLevels() == 0)
909909
return ConstrExpr;
910910

911-
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
911+
Sema::SFINAETrap SFINAE(S);
912912

913913
Sema::InstantiatingTemplate Inst(
914914
S, DeclInfo.getLocation(),

‎clang/lib/Sema/SemaExprCXX.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaExprCXX.cpp
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5198,7 +5198,7 @@ static bool HasNonDeletedDefaultedEqualityComparison(Sema &S,
51985198
{
51995199
EnterExpressionEvaluationContext UnevaluatedContext(
52005200
S, Sema::ExpressionEvaluationContext::Unevaluated);
5201-
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
5201+
Sema::SFINAETrap SFINAE(S, /*TestWellformedSFINAE=*/true);
52025202
Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
52035203

52045204
// const ClassT& obj;
@@ -5809,7 +5809,7 @@ static ExprResult CheckConvertibilityForTypeTraits(
58095809
// trap at translation unit scope.
58105810
EnterExpressionEvaluationContext Unevaluated(
58115811
Self, Sema::ExpressionEvaluationContext::Unevaluated);
5812-
Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
5812+
Sema::SFINAETrap SFINAE(Self, /*TestWellformedSFINAE=*/true);
58135813
Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
58145814
InitializationSequence Init(Self, To, Kind, From);
58155815
if (Init.Failed())
@@ -5932,7 +5932,7 @@ static bool EvaluateBooleanTypeTrait(Sema &S, TypeTrait Kind,
59325932
// trap at translation unit scope.
59335933
EnterExpressionEvaluationContext Unevaluated(
59345934
S, Sema::ExpressionEvaluationContext::Unevaluated);
5935-
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
5935+
Sema::SFINAETrap SFINAE(S, /*TestWellformedSFINAE=*/true);
59365936
Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
59375937
InitializedEntity To(
59385938
InitializedEntity::InitializeTemporary(S.Context, Args[0]));
@@ -6272,7 +6272,7 @@ static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, const TypeSourceI
62726272
// trap at translation unit scope.
62736273
EnterExpressionEvaluationContext Unevaluated(
62746274
Self, Sema::ExpressionEvaluationContext::Unevaluated);
6275-
Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
6275+
Sema::SFINAETrap SFINAE(Self, /*TestWellformedSFINAE=*/true);
62766276
Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
62776277
ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs,
62786278
&Rhs);

‎clang/lib/Sema/SemaTemplate.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaTemplate.cpp
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,7 +3112,7 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
31123112

31133113
EnterExpressionEvaluationContext UnevaluatedContext(
31143114
S, Sema::ExpressionEvaluationContext::Unevaluated);
3115-
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
3115+
Sema::SFINAETrap SFINAE(S, /*TestWellformedSFINAE=*/true);
31163116
Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
31173117

31183118
QualType BaseTemplateInst =
@@ -3158,7 +3158,7 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
31583158
auto CheckConditionalOperands = [&](bool ConstRefQual) -> QualType {
31593159
EnterExpressionEvaluationContext UnevaluatedContext(
31603160
S, Sema::ExpressionEvaluationContext::Unevaluated);
3161-
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
3161+
Sema::SFINAETrap SFINAE(S, /*TestWellformedSFINAE=*/true);
31623162
Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
31633163

31643164
// false

‎clang/test/SemaCXX/type-trait-common-type.cpp

Copy file name to clipboardExpand all lines: clang/test/SemaCXX/type-trait-common-type.cpp
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ void test_vla() {
3434

3535
template <class... Args>
3636
using common_type_base = __builtin_common_type<common_type_t, type_identity, empty_type, Args...>;
37+
// expected-note@-1 {{in instantiation of default function argument expression for 'InvalidConversion<void>' required here}}
3738

3839
template <class... Args>
3940
struct common_type : common_type_base<Args...> {};
@@ -208,3 +209,36 @@ struct common_type<PrivateTypeMember, PrivateTypeMember>
208209
};
209210

210211
static_assert(__is_same(common_type_base<PrivateTypeMember, PrivateTypeMember, PrivateTypeMember>, empty_type));
212+
213+
class PrivateConstructor {
214+
private:
215+
PrivateConstructor(int);
216+
};
217+
218+
static_assert(__is_same(common_type_base<int, PrivateConstructor>, empty_type));
219+
220+
// expected-note@+1 {{in instantiation of template type alias 'common_type_base' requested here}}
221+
template<typename A, typename B, typename Res = common_type_base<A, B>>
222+
static Res common_type_sfinae();
223+
// expected-note@-1 {{in instantiation of default argument for 'common_type_sfinae<int, InvalidConversion>' required here}}
224+
225+
// Make sure we swallow "calling a private constructor" even in SFINAE context ...
226+
static_assert(__is_same(decltype(common_type_sfinae<int, PrivateConstructor>()), empty_type));
227+
228+
// ... but not when there is an error outside of the immediate context.
229+
template<typename T>
230+
struct Member {
231+
T t; // expected-error {{field has incomplete type 'void'}}
232+
};
233+
234+
// The conversion from int has a non-SFINAE error.
235+
class InvalidConversion {
236+
private:
237+
template<typename T = void>
238+
InvalidConversion(int, Member<T> = {});
239+
// expected-note@-1 {{in instantiation of template class 'Member<void>' requested here}}
240+
// expected-note@-2 {{passing argument to parameter here}}
241+
};
242+
243+
// expected-note@+1 {{while substituting deduced template arguments into function template 'common_type_sfinae'}}
244+
static_assert(__is_same(decltype(common_type_sfinae<int, InvalidConversion>()), empty_type));

‎clang/test/SemaCXX/type-traits.cpp

Copy file name to clipboardExpand all lines: clang/test/SemaCXX/type-traits.cpp
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2673,6 +2673,9 @@ struct FloatWrapper
26732673
}
26742674
};
26752675

2676+
template<typename A, typename B, bool = __is_convertible(A, B)>
2677+
static void is_convertible_sfinae();
2678+
26762679
void is_convertible()
26772680
{
26782681
static_assert(__is_convertible(IntWrapper, IntWrapper));
@@ -2697,6 +2700,10 @@ void is_convertible()
26972700
static_assert(__is_convertible(FloatWrapper, const float&));
26982701
static_assert(__is_convertible(float, FloatWrapper&&));
26992702
static_assert(__is_convertible(float, const FloatWrapper&));
2703+
2704+
static_assert(!__is_convertible(AllPrivate, AllPrivate));
2705+
// Make sure we swallow "calling a private constructor" even in SFINAE context.
2706+
using Result = decltype(is_convertible_sfinae<AllPrivate, AllPrivate>());
27002707
}
27012708

27022709
void is_nothrow_convertible()
@@ -2822,6 +2829,9 @@ void is_trivial()
28222829

28232830
template<typename T> struct TriviallyConstructibleTemplate {};
28242831

2832+
template<typename A, typename B, bool = __is_assignable(A, B)>
2833+
static void is_assignable_sfinae();
2834+
28252835
void trivial_checks()
28262836
{
28272837
static_assert(__is_trivially_copyable(int));
@@ -2995,6 +3005,10 @@ void trivial_checks()
29953005
static_assert(!__is_assignable(AnIncompleteType[1], AnIncompleteType[1])); // expected-error {{incomplete type}}
29963006
static_assert(!__is_assignable(void, void));
29973007
static_assert(!__is_assignable(const volatile void, const volatile void));
3008+
3009+
static_assert(!__is_assignable(AllPrivate, AllPrivate));
3010+
// Make sure we swallow "'operator=' is a private member" even in SFINAE context.
3011+
using Result = decltype(is_assignable_sfinae<AllPrivate, AllPrivate>());
29983012
}
29993013

30003014
void constructible_checks() {
@@ -3191,6 +3205,9 @@ void reference_constructs_from_temporary_checks() {
31913205

31923206
}
31933207

3208+
template<typename A, typename B, bool = __reference_converts_from_temporary(A, B)>
3209+
static void reference_converts_from_temporary_sfinae();
3210+
31943211
void reference_converts_from_temporary_checks() {
31953212
static_assert(!__reference_converts_from_temporary(int &, int &));
31963213
static_assert(!__reference_converts_from_temporary(int &, int &&));
@@ -3241,6 +3258,9 @@ void reference_converts_from_temporary_checks() {
32413258
static_assert(__reference_converts_from_temporary(const int&, ExplicitConversionRef));
32423259
static_assert(__reference_converts_from_temporary(int&&, ExplicitConversionRvalueRef));
32433260

3261+
static_assert(!__reference_converts_from_temporary(AllPrivate, AllPrivate));
3262+
// Make sure we swallow "calling a private constructor" even in SFINAE context.
3263+
using Result = decltype(reference_converts_from_temporary_sfinae<AllPrivate, AllPrivate>());
32443264
}
32453265

32463266
void array_rank() {
@@ -4075,6 +4095,20 @@ struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
40754095

40764096
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
40774097

4098+
struct NotTriviallyEqualityComparablePrivateComparison {
4099+
int i;
4100+
4101+
private:
4102+
bool operator==(const NotTriviallyEqualityComparablePrivateComparison&) const = default;
4103+
};
4104+
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparablePrivateComparison));
4105+
4106+
template<typename T, bool = __is_trivially_equality_comparable(T)>
4107+
static void is_trivially_equality_comparable_sfinae();
4108+
4109+
// Make sure we swallow "'operator==' is a private member" even in SFINAE context.
4110+
using SFINAEResult = decltype(is_trivially_equality_comparable_sfinae<NotTriviallyEqualityComparablePrivateComparison>());
4111+
40784112
template<bool B>
40794113
struct MaybeTriviallyEqualityComparable {
40804114
int i;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.