Author: Aaron Puchert Date: 2025-05-20T23:02:51+02:00 New Revision: 317c932622b8ee65ad0a7df23d8bf4c03aee09bb
URL: https://github.com/llvm/llvm-project/commit/317c932622b8ee65ad0a7df23d8bf4c03aee09bb DIFF: https://github.com/llvm/llvm-project/commit/317c932622b8ee65ad0a7df23d8bf4c03aee09bb.diff LOG: Suppress errors from well-formed-testing type traits in SFINAE contexts (#135390) 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 validity of some expression, `ForValidityCheck` seems to be a good 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 `ForValidityCheck` condition first because it's constant in all uses and this would then allow the compiler to prune the call to `isSFINAEContext` when true. Fixes #132044. Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaTemplate.cpp clang/test/SemaCXX/type-trait-common-type.cpp clang/test/SemaCXX/type-traits.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8e6cf62e11752..ad8409397ff8a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -683,6 +683,19 @@ Bug Fixes to C++ Support - Clang is now better at keeping track of friend function template instance contexts. (#GH55509) - Clang now prints the correct instantiation context for diagnostics suppressed by template argument deduction. +- Errors that occur during evaluation of certain type traits and builtins are + no longer incorrectly emitted when they are used in an SFINAE context. The + type traits are: + + - ``__is_constructible`` and variants, + - ``__is_convertible`` and variants, + - ``__is_assignable`` and variants, + - ``__reference_binds_to_temporary``, + ``__reference_constructs_from_temporary``, + ``__reference_converts_from_temporary``, + - ``__is_trivially_equality_comparable``. + + The builtin is ``__builtin_common_type``. (#GH132044) - Clang is now better at instantiating the function definition after its use inside of a constexpr lambda. (#GH125747) - Fixed a local class member function instantiation bug inside dependent lambdas. (#GH59734), (#GH132208) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9af3387b533c0..a994b845e11fc 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12365,16 +12365,19 @@ class Sema final : public SemaBase { bool PrevLastDiagnosticIgnored; public: - explicit SFINAETrap(Sema &SemaRef, bool AccessCheckingSFINAE = false) + /// \param ForValidityCheck If true, discard all diagnostics (from the + /// immediate context) instead of adding them to the currently active + /// \ref TemplateDeductionInfo (as returned by \ref isSFINAEContext). + explicit SFINAETrap(Sema &SemaRef, bool ForValidityCheck = false) : SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors), PrevInNonInstantiationSFINAEContext( SemaRef.InNonInstantiationSFINAEContext), PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE), PrevLastDiagnosticIgnored( SemaRef.getDiagnostics().isLastDiagnosticIgnored()) { - if (!SemaRef.isSFINAEContext()) + if (ForValidityCheck || !SemaRef.isSFINAEContext()) SemaRef.InNonInstantiationSFINAEContext = true; - SemaRef.AccessCheckingSFINAE = AccessCheckingSFINAE; + SemaRef.AccessCheckingSFINAE = ForValidityCheck; } ~SFINAETrap() { @@ -12404,7 +12407,7 @@ class Sema final : public SemaBase { public: explicit TentativeAnalysisScope(Sema &SemaRef) - : SemaRef(SemaRef), Trap(SemaRef, true), + : SemaRef(SemaRef), Trap(SemaRef, /*ForValidityCheck=*/true), PrevDisableTypoCorrection(SemaRef.DisableTypoCorrection) { SemaRef.DisableTypoCorrection = true; } diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 543bd450c554e..aef78644992b7 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -907,7 +907,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction( if (MLTAL.getNumSubstitutedLevels() == 0) return ConstrExpr; - Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false); + Sema::SFINAETrap SFINAE(S); Sema::InstantiatingTemplate Inst( S, DeclInfo.getLocation(), diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index b071c98051bbe..b53877c40668d 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -5523,7 +5523,7 @@ static bool HasNonDeletedDefaultedEqualityComparison(Sema &S, { EnterExpressionEvaluationContext UnevaluatedContext( S, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(S, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl()); // const ClassT& obj; @@ -6232,7 +6232,7 @@ static ExprResult CheckConvertibilityForTypeTraits( // trap at translation unit scope. EnterExpressionEvaluationContext Unevaluated( Self, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(Self, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); InitializationSequence Init(Self, To, Kind, From); if (Init.Failed()) @@ -6354,7 +6354,7 @@ static bool EvaluateBooleanTypeTrait(Sema &S, TypeTrait Kind, // trap at translation unit scope. EnterExpressionEvaluationContext Unevaluated( S, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(S, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl()); InitializedEntity To( InitializedEntity::InitializeTemporary(S.Context, Args[0])); @@ -6697,7 +6697,7 @@ static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, const TypeSourceI // trap at translation unit scope. EnterExpressionEvaluationContext Unevaluated( Self, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(Self, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl()); ExprResult Result = Self.BuildBinOp(/*S=*/nullptr, KeyLoc, BO_Assign, &Lhs, &Rhs); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 14f9d1d03c5ed..4a99a097a6e71 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3118,7 +3118,7 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate, EnterExpressionEvaluationContext UnevaluatedContext( S, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(S, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl()); QualType BaseTemplateInst = @@ -3164,7 +3164,7 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate, auto CheckConditionalOperands = [&](bool ConstRefQual) -> QualType { EnterExpressionEvaluationContext UnevaluatedContext( S, Sema::ExpressionEvaluationContext::Unevaluated); - Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true); + Sema::SFINAETrap SFINAE(S, /*ForValidityCheck=*/true); Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl()); // false diff --git a/clang/test/SemaCXX/type-trait-common-type.cpp b/clang/test/SemaCXX/type-trait-common-type.cpp index 7190dcad76f1a..8f2b97ba784c3 100644 --- a/clang/test/SemaCXX/type-trait-common-type.cpp +++ b/clang/test/SemaCXX/type-trait-common-type.cpp @@ -34,6 +34,7 @@ void test_vla() { template <class... Args> using common_type_base = __builtin_common_type<common_type_t, type_identity, empty_type, Args...>; +// expected-note@-1 {{in instantiation of default function argument expression for 'InvalidConversion<void>' required here}} template <class... Args> struct common_type : common_type_base<Args...> {}; @@ -208,3 +209,36 @@ struct common_type<PrivateTypeMember, PrivateTypeMember> }; static_assert(__is_same(common_type_base<PrivateTypeMember, PrivateTypeMember, PrivateTypeMember>, empty_type)); + +class PrivateConstructor { +private: + PrivateConstructor(int); +}; + +static_assert(__is_same(common_type_base<int, PrivateConstructor>, empty_type)); + +// expected-note@+1 {{in instantiation of template type alias 'common_type_base' requested here}} +template<typename A, typename B, typename Res = common_type_base<A, B>> +static Res common_type_sfinae(); +// expected-note@-1 {{in instantiation of default argument for 'common_type_sfinae<int, InvalidConversion>' required here}} + +// Make sure we don't emit "calling a private constructor" in SFINAE context ... +static_assert(__is_same(decltype(common_type_sfinae<int, PrivateConstructor>()), empty_type)); + +// ... but we still emit errors outside of the immediate context. +template<typename T> +struct Member { + T t; // expected-error {{field has incomplete type 'void'}} +}; + +// The conversion from int has a non-SFINAE error. +class InvalidConversion { +private: + template<typename T = void> + InvalidConversion(int, Member<T> = {}); + // expected-note@-1 {{in instantiation of template class 'Member<void>' requested here}} + // expected-note@-2 {{passing argument to parameter here}} +}; + +// expected-note@+1 {{while substituting deduced template arguments into function template 'common_type_sfinae'}} +static_assert(__is_same(decltype(common_type_sfinae<int, InvalidConversion>()), empty_type)); diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp index 7768f61ac2d00..9ed3295dc86f6 100644 --- a/clang/test/SemaCXX/type-traits.cpp +++ b/clang/test/SemaCXX/type-traits.cpp @@ -2673,6 +2673,9 @@ struct FloatWrapper } }; +template<typename A, typename B, bool result = __is_convertible(A, B)> +static constexpr bool is_convertible_sfinae() { return result; } + void is_convertible() { static_assert(__is_convertible(IntWrapper, IntWrapper)); @@ -2697,6 +2700,10 @@ void is_convertible() static_assert(__is_convertible(FloatWrapper, const float&)); static_assert(__is_convertible(float, FloatWrapper&&)); static_assert(__is_convertible(float, const FloatWrapper&)); + + static_assert(!__is_convertible(AllPrivate, AllPrivate)); + // Make sure we don't emit "calling a private constructor" in SFINAE context. + static_assert(!is_convertible_sfinae<AllPrivate, AllPrivate>()); } void is_nothrow_convertible() @@ -2822,6 +2829,9 @@ void is_trivial() template<typename T> struct TriviallyConstructibleTemplate {}; +template<typename A, typename B, bool result = __is_assignable(A, B)> +static constexpr bool is_assignable_sfinae() { return result; } + void trivial_checks() { static_assert(__is_trivially_copyable(int)); @@ -2995,6 +3005,10 @@ void trivial_checks() static_assert(!__is_assignable(AnIncompleteType[1], AnIncompleteType[1])); // expected-error {{incomplete type}} static_assert(!__is_assignable(void, void)); static_assert(!__is_assignable(const volatile void, const volatile void)); + + static_assert(!__is_assignable(AllPrivate, AllPrivate)); + // Make sure we don't emit "'operator=' is a private member" in SFINAE context. + static_assert(!is_assignable_sfinae<AllPrivate, AllPrivate>()); } void constructible_checks() { @@ -3191,6 +3205,9 @@ void reference_constructs_from_temporary_checks() { } +template<typename A, typename B, bool result = __reference_converts_from_temporary(A, B)> +static constexpr bool reference_converts_from_temporary_sfinae() { return result; } + void reference_converts_from_temporary_checks() { static_assert(!__reference_converts_from_temporary(int &, int &)); static_assert(!__reference_converts_from_temporary(int &, int &&)); @@ -3241,6 +3258,9 @@ void reference_converts_from_temporary_checks() { static_assert(__reference_converts_from_temporary(const int&, ExplicitConversionRef)); static_assert(__reference_converts_from_temporary(int&&, ExplicitConversionRvalueRef)); + static_assert(!__reference_converts_from_temporary(AllPrivate, AllPrivate)); + // Make sure we don't emit "calling a private constructor" in SFINAE context. + static_assert(!reference_converts_from_temporary_sfinae<AllPrivate, AllPrivate>()); } void array_rank() { @@ -4085,6 +4105,20 @@ struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 { static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2)); +struct NotTriviallyEqualityComparablePrivateComparison { + int i; + +private: + bool operator==(const NotTriviallyEqualityComparablePrivateComparison&) const = default; +}; +static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparablePrivateComparison)); + +template<typename T, bool result = __is_trivially_equality_comparable(T)> +static constexpr bool is_trivially_equality_comparable_sfinae() { return result; } + +// Make sure we don't emit "'operator==' is a private member" in SFINAE context. +static_assert(!is_trivially_equality_comparable_sfinae<NotTriviallyEqualityComparablePrivateComparison>()); + template<bool B> struct MaybeTriviallyEqualityComparable { int i; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
