[Clang] Add __builtin_common_reference #121199
Merged
Conversation
This file contains hidden or 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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
philnik777
force-pushed
the
builtin_common_reference
branch
from
e0db731 to
55f9d1e
Compare
philnik777
force-pushed
the
builtin_common_reference
branch
2 times, most recently
from
3010ba7 to
19a7693
Compare
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesPatch is 40.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121199.diff 9 Files Affected:
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 3b8a9cac6587a..6fc86507ca423 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1677,6 +1677,23 @@ Builtin type aliases
Clang provides a few builtin aliases to improve the throughput of certain metaprogramming facilities.
+__builtin_common_reference
+--------------------------
+
+.. code-block:: c++
+
+ template <template <class, class, template <class> class, template <class> class> class BasicCommonReferenceT,
+ template <class... Args> CommonTypeT,
+ template <class> HasTypeMember,
+ class HasNoTypeMember,
+ class... Ts>
+ using __builtin_common_reference = ...;
+
+This alias is used for implementing ``std::common_refernce``. If ``std::common_reference`` should contain a ``type``
+member, it is an alias to ``HasTypeMember<TheCommonReference>``. Otherwse it is an alias to ``HasNoTypeMember``. The
+``CommonTypeT`` is usually ``std::common_type_t``. ``BasicCommonReferenceT`` is usually an alias template to
+``basic_common_reference<T, U, TX, UX>::type``.
+
__builtin_common_type
---------------------
diff --git a/clang/include/clang/Basic/BuiltinTemplates.td b/clang/include/clang/Basic/BuiltinTemplates.td
index d46ce063d2f7e..5c79e89800829 100644
--- a/clang/include/clang/Basic/BuiltinTemplates.td
+++ b/clang/include/clang/Basic/BuiltinTemplates.td
@@ -10,11 +10,11 @@ class TemplateArg<string name> {
string Name = name;
}
-class Template<list<TemplateArg> args, string name> : TemplateArg<name> {
+class Template<list<TemplateArg> args, string name = ""> : TemplateArg<name> {
list<TemplateArg> Args = args;
}
-class Class<string name, bit is_variadic = 0> : TemplateArg<name> {
+class Class<string name = "", bit is_variadic = 0> : TemplateArg<name> {
bit IsVariadic = is_variadic;
}
@@ -50,3 +50,29 @@ def __builtin_common_type : BuiltinTemplate<
Template<[Class<"TypeMember">], "HasTypeMember">,
Class<"HasNoTypeMember">,
Class<"Ts", /*is_variadic=*/1>]>;
+
+// template <template <class,"
+// class,"
+// template <class> class,"
+// template <class> class> class BasicCommonReferenceT,"
+// template <class... Args> class CommonTypeT,"
+// template <class> class HasTypeMember,"
+// class HasNoTypeMember,"
+// class... Ts>"
+def __builtin_common_reference : BuiltinTemplate<
+ [Template<[Class<>,
+ Class<>,
+ Template<[Class<>]>,
+ Template<[Class<>]>], "BasicCommonReferenceT">,
+ Template<[Class<"Args", /*is_variadic=*/1>], "CommonTypeT">,
+ Template<[Class<>], "HasTypeMember">,
+ Class<"HasNoTypeMember">,
+ Class<"Ts", /*is_variadic=*/1>]>;
+
+foreach Ref = ["", "lvalue", "rvalue"] in {
+ foreach Const = ["", "const"] in {
+ foreach Volatile = ["", "volatile"] in {
+ def __clang_internal_xref_#Ref#Const#Volatile : BuiltinTemplate<[Class<>]>;
+ }
+ }
+}
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 066bce61c74c1..762cb851ee04f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -15076,15 +15076,34 @@ class Sema final : public SemaBase {
QualType BuiltinDecay(QualType BaseType, SourceLocation Loc);
QualType BuiltinAddReference(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
+
+ QualType BuiltinAddRValueReference(QualType BaseType, SourceLocation Loc) {
+ return BuiltinAddReference(BaseType, UnaryTransformType::AddRvalueReference,
+ Loc);
+ }
+
+ QualType BuiltinAddLValueReference(QualType BaseType, SourceLocation Loc) {
+ return BuiltinAddReference(BaseType, UnaryTransformType::AddLvalueReference,
+ Loc);
+ }
+
QualType BuiltinRemoveExtent(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
QualType BuiltinRemoveReference(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
+
+ QualType BuiltinRemoveCVRef(QualType BaseType, SourceLocation Loc) {
+ return BuiltinRemoveReference(BaseType, UTTKind::RemoveCVRef, Loc);
+ }
+
QualType BuiltinChangeCVRQualifiers(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
QualType BuiltinChangeSignedness(QualType BaseType, UTTKind UKind,
SourceLocation Loc);
+ bool BuiltinIsConvertible(QualType From, QualType To, SourceLocation Loc,
+ bool CheckNothrow = false);
+
/// Ensure that the type T is a literal type.
///
/// This routine checks whether the type @p T is a literal type. If @p T is an
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 46895db4a0756..5ca5a8a57fa0f 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5739,76 +5739,6 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, const TypeSourceInfo *Lhs,
const TypeSourceInfo *Rhs, SourceLocation KeyLoc);
-static ExprResult CheckConvertibilityForTypeTraits(
- Sema &Self, const TypeSourceInfo *Lhs, const TypeSourceInfo *Rhs,
- SourceLocation KeyLoc, llvm::BumpPtrAllocator &OpaqueExprAllocator) {
-
- QualType LhsT = Lhs->getType();
- QualType RhsT = Rhs->getType();
-
- // C++0x [meta.rel]p4:
- // Given the following function prototype:
- //
- // template <class T>
- // typename add_rvalue_reference<T>::type create();
- //
- // the predicate condition for a template specialization
- // is_convertible<From, To> shall be satisfied if and only if
- // the return expression in the following code would be
- // well-formed, including any implicit conversions to the return
- // type of the function:
- //
- // To test() {
- // return create<From>();
- // }
- //
- // Access checking is performed as if in a context unrelated to To and
- // From. Only the validity of the immediate context of the expression
- // of the return-statement (including conversions to the return type)
- // is considered.
- //
- // We model the initialization as a copy-initialization of a temporary
- // of the appropriate type, which for this expression is identical to the
- // return statement (since NRVO doesn't apply).
-
- // Functions aren't allowed to return function or array types.
- if (RhsT->isFunctionType() || RhsT->isArrayType())
- return ExprError();
-
- // A function definition requires a complete, non-abstract return type.
- if (!Self.isCompleteType(Rhs->getTypeLoc().getBeginLoc(), RhsT) ||
- Self.isAbstractType(Rhs->getTypeLoc().getBeginLoc(), RhsT))
- return ExprError();
-
- // Compute the result of add_rvalue_reference.
- if (LhsT->isObjectType() || LhsT->isFunctionType())
- LhsT = Self.Context.getRValueReferenceType(LhsT);
-
- // Build a fake source and destination for initialization.
- InitializedEntity To(InitializedEntity::InitializeTemporary(RhsT));
- Expr *From = new (OpaqueExprAllocator.Allocate<OpaqueValueExpr>())
- OpaqueValueExpr(KeyLoc, LhsT.getNonLValueExprType(Self.Context),
- Expr::getValueKindForType(LhsT));
- InitializationKind Kind =
- InitializationKind::CreateCopy(KeyLoc, SourceLocation());
-
- // Perform the initialization in an unevaluated context within a SFINAE
- // trap at translation unit scope.
- EnterExpressionEvaluationContext Unevaluated(
- Self, Sema::ExpressionEvaluationContext::Unevaluated);
- Sema::SFINAETrap SFINAE(Self, /*AccessCheckingSFINAE=*/true);
- Sema::ContextRAII TUContext(Self, Self.Context.getTranslationUnitDecl());
- InitializationSequence Init(Self, To, Kind, From);
- if (Init.Failed())
- return ExprError();
-
- ExprResult Result = Init.Perform(Self, To, Kind, From);
- if (Result.isInvalid() || SFINAE.hasErrorOccurred())
- return ExprError();
-
- return Result;
-}
-
static APValue EvaluateSizeTTypeTrait(Sema &S, TypeTrait Kind,
SourceLocation KWLoc,
ArrayRef<TypeSourceInfo *> Args,
@@ -5958,9 +5888,8 @@ static bool EvaluateBooleanTypeTrait(Sema &S, TypeTrait Kind,
S.Context.getPointerType(T.getNonReferenceType()));
TypeSourceInfo *UPtr = S.Context.CreateTypeSourceInfo(
S.Context.getPointerType(U.getNonReferenceType()));
- return !CheckConvertibilityForTypeTraits(S, UPtr, TPtr, RParenLoc,
- OpaqueExprAllocator)
- .isInvalid();
+ return S.BuiltinIsConvertible(UPtr->getType(), TPtr->getType(),
+ RParenLoc);
}
if (Kind == clang::TT_IsNothrowConstructible)
@@ -6200,20 +6129,9 @@ static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, const TypeSourceI
}
case BTT_IsConvertible:
case BTT_IsConvertibleTo:
- case BTT_IsNothrowConvertible: {
- if (RhsT->isVoidType())
- return LhsT->isVoidType();
- llvm::BumpPtrAllocator OpaqueExprAllocator;
- ExprResult Result = CheckConvertibilityForTypeTraits(Self, Lhs, Rhs, KeyLoc,
- OpaqueExprAllocator);
- if (Result.isInvalid())
- return false;
-
- if (BTT != BTT_IsNothrowConvertible)
- return true;
-
- return Self.canThrow(Result.get()) == CT_Cannot;
- }
+ case BTT_IsNothrowConvertible:
+ return Self.BuiltinIsConvertible(LhsT, RhsT, KeyLoc,
+ BTT == BTT_IsNothrowConvertible);
case BTT_IsAssignable:
case BTT_IsNothrowAssignable:
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index be81b6a46b2c0..47601c1840b92 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3097,6 +3097,33 @@ void Sema::NoteAllFoundTemplates(TemplateName Name) {
}
}
+static QualType InstantiateTemplate(Sema &S, TemplateName Template,
+ ArrayRef<TemplateArgument> Args,
+ SourceLocation Loc) {
+ TemplateArgumentListInfo ArgList;
+ for (auto Arg : Args) {
+ if (Arg.getKind() == TemplateArgument::Type) {
+ ArgList.addArgument(TemplateArgumentLoc(
+ Arg, S.Context.getTrivialTypeSourceInfo(Arg.getAsType())));
+ } else {
+ ArgList.addArgument(
+ S.getTrivialTemplateArgumentLoc(Arg, QualType(), Loc));
+ }
+ }
+
+ EnterExpressionEvaluationContext UnevaluatedContext(
+ S, Sema::ExpressionEvaluationContext::Unevaluated);
+ Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
+ Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
+
+ QualType Instantiation = S.CheckTemplateIdType(Template, Loc, ArgList);
+
+ if (SFINAE.hasErrorOccurred())
+ return QualType();
+
+ return Instantiation;
+}
+
static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
SourceLocation TemplateLoc,
ArrayRef<TemplateArgument> Ts) {
@@ -3107,24 +3134,7 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
if (T1.getAsType()->isBuiltinType() && T2.getAsType()->isBuiltinType())
return builtinCommonTypeImpl(S, BaseTemplate, TemplateLoc, {T1, T2});
- TemplateArgumentListInfo Args;
- Args.addArgument(TemplateArgumentLoc(
- T1, S.Context.getTrivialTypeSourceInfo(T1.getAsType())));
- Args.addArgument(TemplateArgumentLoc(
- T2, S.Context.getTrivialTypeSourceInfo(T2.getAsType())));
-
- EnterExpressionEvaluationContext UnevaluatedContext(
- S, Sema::ExpressionEvaluationContext::Unevaluated);
- Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
- Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
-
- QualType BaseTemplateInst =
- S.CheckTemplateIdType(BaseTemplate, TemplateLoc, Args);
-
- if (SFINAE.hasErrorOccurred())
- return QualType();
-
- return BaseTemplateInst;
+ return InstantiateTemplate(S, BaseTemplate, {T1, T2}, TemplateLoc);
};
// Note A: For the common_type trait applied to a template parameter pack T of
@@ -3231,6 +3241,230 @@ static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
}
}
+static QualType CopyCV(QualType From, QualType To) {
+ if (From.isConstQualified())
+ To.addConst();
+ if (From.isVolatileQualified())
+ To.addVolatile();
+ return To;
+}
+
+// COND-RES(X, Y) be decltype(false ? declval<X(&)()>()() : declval<Y(&)()>()())
+static QualType CondRes(Sema &S, QualType X, QualType Y, SourceLocation Loc) {
+ EnterExpressionEvaluationContext UnevaluatedContext(
+ S, Sema::ExpressionEvaluationContext::Unevaluated);
+ Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
+ Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
+
+ // false
+ OpaqueValueExpr CondExpr(SourceLocation(), S.Context.BoolTy, VK_PRValue);
+ ExprResult Cond = &CondExpr;
+
+ // declval<X(&)()>()()
+ OpaqueValueExpr LHSExpr(Loc, X.getNonLValueExprType(S.Context),
+ Expr::getValueKindForType(X));
+ ExprResult LHS = &LHSExpr;
+
+ // declval<Y(&)()>()()
+ OpaqueValueExpr RHSExpr(Loc, Y.getNonLValueExprType(S.Context),
+ Expr::getValueKindForType(Y));
+ ExprResult RHS = &RHSExpr;
+
+ ExprValueKind VK = VK_PRValue;
+ ExprObjectKind OK = OK_Ordinary;
+
+ // decltype(false ? declval<X(&)()>()() : declval<Y(&)()>()())
+ QualType Result = S.CheckConditionalOperands(Cond, LHS, RHS, VK, OK, Loc);
+
+ if (SFINAE.hasErrorOccurred())
+ return QualType();
+ if (VK == VK_LValue)
+ return S.BuiltinAddLValueReference(Result, Loc);
+ if (VK == VK_XValue)
+ return S.BuiltinAddRValueReference(Result, Loc);
+ return Result;
+}
+
+static QualType CommonRef(Sema &S, QualType A, QualType B, SourceLocation Loc) {
+ // Given types A and B, let X be remove_reference_t<A>, let Y be
+ // remove_reference_t<B>, and let COMMON-REF(A, B) be:
+ assert(A->isReferenceType() && B->isReferenceType() &&
+ "A and B have to be ref qualified for a COMMON-REF");
+ auto X = A.getNonReferenceType();
+ auto Y = B.getNonReferenceType();
+
+ // If A and B are both lvalue reference types, COMMON-REF(A, B) is
+ // COND-RES(COPYCV(X, Y) &, COPYCV(Y, X) &) if that type exists and is a
+ // reference type.
+ if (A->isLValueReferenceType() && B->isLValueReferenceType()) {
+ auto CR = CondRes(S, S.BuiltinAddLValueReference(CopyCV(X, Y), Loc),
+ S.BuiltinAddLValueReference(CopyCV(Y, X), Loc), Loc);
+ if (CR.isNull() || !CR->isReferenceType())
+ return QualType();
+ return CR;
+ }
+
+ // Otherwise, let C be remove_reference_t<COMMON-REF(X&, Y&)>&&. If A and B
+ // are both rvalue reference types, C is well-formed, and
+ // is_convertible_v<A, C> && is_convertible_v<B, C> is true, then
+ // COMMON-REF(A, B) is C.
+ if (A->isRValueReferenceType() && B->isRValueReferenceType()) {
+ auto C = CommonRef(S, S.BuiltinAddLValueReference(X, Loc),
+ S.BuiltinAddLValueReference(Y, Loc), Loc);
+ if (C.isNull())
+ return QualType();
+
+ C = C.getNonReferenceType();
+
+ if (S.BuiltinIsConvertible(A, C, Loc) && S.BuiltinIsConvertible(B, C, Loc))
+ return S.BuiltinAddRValueReference(C, Loc);
+ return QualType();
+ }
+
+ // Otherwise, if A is an lvalue reference and B is an rvalue reference, then
+ // COMMON-REF(A, B) is COMMON-REF(B, A).
+ if (A->isLValueReferenceType() && B->isRValueReferenceType())
+ std::swap(A, B);
+
+ // Otherwise, let D be COMMON-REF(const X&, Y&). If A is an rvalue reference
+ // and B is an lvalue reference and D is well-formed and
+ // is_convertible_v<A, D> is true, then COMMON-REF(A, B) is D.
+ if (A->isRValueReferenceType() && B->isLValueReferenceType()) {
+ auto X2 = X;
+ X2.addConst();
+ auto D = CommonRef(S, S.BuiltinAddLValueReference(X2, Loc),
+ S.BuiltinAddLValueReference(Y, Loc), Loc);
+ if (!D.isNull() && S.BuiltinIsConvertible(A, D, Loc))
+ return D;
+ return QualType();
+ }
+
+ // Otherwise, COMMON-REF(A, B) is ill-formed.
+ // This is implemented by returning from the individual branches above.
+
+ llvm_unreachable("The above cases should be exhaustive");
+}
+
+static QualType builtinCommonReferenceImpl(Sema &S,
+ TemplateName CommonReference,
+ TemplateName CommonType,
+ SourceLocation TemplateLoc,
+ ArrayRef<TemplateArgument> Ts) {
+ switch (Ts.size()) {
+ // If sizeof...(T) is zero, there shall be no member type.
+ case 0:
+ return QualType();
+
+ // Otherwise, if sizeof...(T) is one, let T0 denote the sole type in the
+ // pack T. The member typedef type shall denote the same type as T0.
+ case 1:
+ return Ts[0].getAsType();
+
+ // Otherwise, if sizeof...(T) is two, let T1 and T2 denote the two types in
+ // the pack T. Then
+ case 2: {
+ auto T1 = Ts[0].getAsType();
+ auto T2 = Ts[1].getAsType();
+
+ // Let R be COMMON-REF(T1, T2). If T1 and T2 are reference types, R is
+ // well-formed, and is_convertible_v<add_pointer_t<T1>, add_pointer_t<R>> &&
+ // is_convertible_v<add_pointer_t<T2>, add_pointer_t<R>> is true, then the
+ // member typedef type denotes R.
+ if (T1->isReferenceType() && T2->isReferenceType()) {
+ QualType R = CommonRef(S, T1, T2, TemplateLoc);
+ if (!R.isNull()) {
+ if (S.BuiltinIsConvertible(S.BuiltinAddPointer(T1, TemplateLoc),
+ S.BuiltinAddPointer(R, TemplateLoc),
+ TemplateLoc) &&
+ S.BuiltinIsConvertible(S.BuiltinAddPointer(T2, TemplateLoc),
+ S.BuiltinAddPointer(R, TemplateLoc),
+ TemplateLoc)) {
+ return R;
+ }
+ }
+ }
+
+ // Otherwise, if basic_common_reference<remove_cvref_t<T1>,
+ // remove_cvref_t<T2>, XREF(T1), XREF(T2)>::type is well-formed,
+ // then the member typedef type denotes that type.
+ {
+ auto getXRef = [&](QualType T) {
+ static BuiltinTemplateDecl *Quals[12] = {
+ S.Context.get__clang_internal_xref_Decl(),
+ S.Context.get__clang_internal_xref_constDecl(),
+ S.Context.get__clang_internal_xref_volatileDecl(),
+ S.Context.get__clang_internal_xref_constvolatileDecl(),
+ S.Context.get__clang_internal_xref_lvalueDecl(),
+ S.Context.get__clang_internal_xref_lvalueconstDecl(),
+ S.Context.get__clang_internal_xref_lvaluevolatileDecl(),
+ S.Context.get__clang_internal_xref_lvalueconstvolatileDecl(),
+ S.Context.get__clang_internal_xref_rvalueDecl(),
+ S.Context.get__clang_internal_xref_rvalueconstDecl(),
+ S.Context.get__clang_internal_xref_rvaluevolatileDecl(),
+ S.Context.get__clang_internal_xref_rvalueconstvolatileDecl(),
+ };
+ size_t Index = 0;
+ if (T->isLValueReferenceType()) {
+ T = T.getNonReferenceType();
+ Index += 4;
+ } else if (T->isRValueReferenceType()) {
+ T = T.getNonReferenceType();
+ Index += 8;
+ }
+ if (T.isConstQualified())
+ Index += 1;
+
+ if (T.isVolatileQualified())
+ Index += 2;
+
+ return Quals[Index];
+ };
+
+ auto BCR = InstantiateTemplate(S, CommonReference,
+ {S.BuiltinRemoveCVRef(T1, TemplateLoc),
+ S.BuiltinRemoveCVRef(T2, TemplateLoc),
+ TemplateName{getXRef(T1)},
+ TemplateName{getXRef(T2)}},
+ TemplateLoc);
+ if (!BCR.isNull())
+ return BCR;
+ }
+
+ // Otherwise, if COND-RES(T1...
[truncated]
|
huixie90
requested changes
Mar 31, 2025
mordante
reviewed
Apr 1, 2025
philnik777
force-pushed
the
builtin_common_reference
branch
from
19a7693 to
97d6d69
Compare
philnik777
force-pushed
the
builtin_common_reference
branch
from
97d6d69 to
273d0e6
Compare
huixie90
reviewed
May 24, 2025
Member
|
FWIW, I created patch #141408 to implement the libc++ counter part of the missing bit.
|
erichkeane
approved these changes
May 27, 2025
Collaborator
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
This seems reasonable to me? I'm not a huge fan of how much is being put into SemaTemplate.cpp to support all the builtins though. I'm wondering if we need to split that up in a follow-up and have a SemaTemplateBuiltins for all of these to live.
Please let Aaron/others take a poke at this, and make sure there are a few more Libc++ reviewers to make sure you're all in agreement with how this works (I see the convo that is happening).
Also, should we loop in libstdc++ folks here?
Contributor
Author
I can take a look at splitting things up in a follow-up. Shouldn't be too complicated. I do wonder whether it's worth the split, since it's a relatively small amount of code. OTOH it's a very clean split...
IDK whether they have any thought on this, but certainly doesn't hurt. CC @jwakely |
Contributor
Author
|
@huixie90 Are you happy with landing this now? |
Collaborator
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
Precommit CI found relevant build failures:
FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaTemplate.cpp.o
sccache /opt/llvm/bin/clang++ -DCLANG_EXPORTS -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/lib/Sema -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wno-pass-failed -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17 -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaTemplate.cpp.o -MF tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaTemplate.cpp.o.d -o tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaTemplate.cpp.o -c /home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp:3542:18: error: no matching function for call to 'InstantiateTemplate'
3542 | auto BCR = InstantiateTemplate(S, CommonReference,
| ^~~~~~~~~~~~~~~~~~~
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp:3215:17: note: candidate function not viable: requires 5 arguments, but 4 were provided
3215 | static QualType InstantiateTemplate(Sema &S, ElaboratedTypeKeyword Keyword, TemplateName Template,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3216 | ArrayRef<TemplateArgument> Args,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3217 | SourceLocation Loc) {
| ~~~~~~~~~~~~~~~~~~
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp:3559:19: error: no matching function for call to 'InstantiateTemplate'
3559 | if (auto CT = InstantiateTemplate(S, CommonType, {T1, T2}, TemplateLoc);
| ^~~~~~~~~~~~~~~~~~~
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp:3215:17: note: candidate function not viable: requires 5 arguments, but 4 were provided
3215 | static QualType InstantiateTemplate(Sema &S, ElaboratedTypeKeyword Keyword, TemplateName Template,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3216 | ArrayRef<TemplateArgument> Args,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3217 | SourceLocation Loc) {
| ~~~~~~~~~~~~~~~~~~
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Sema/SemaTemplate.cpp:3762:73: error: too few arguments to function call, expected 6, have 3
3762 | return SemaRef.CheckTemplateIdType(HasTypeMember, TemplateLoc, TAs);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/include/clang/Sema/Sema.h:11648:12: note: 'CheckTemplateIdType' declared here
11648 | QualType CheckTemplateIdType(ElaboratedTypeKeyword Keyword,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
11649 | TemplateName Template,
| ~~~~~~~~~~~~~~~~~~~~~~
11650 | SourceLocation TemplateLoc,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
11651 | TemplateArgumentListInfo &TemplateArgs,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
11652 | Scope *Scope, bool ForNestedNameSpecifier);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
Comment on lines
+13
to
+17
| class Template<list<TemplateArg> args, string name = ""> : TemplateArg<name> { | ||
| list<TemplateArg> Args = args; | ||
| } | ||
|
|
||
| class Class<string name, bit is_variadic = 0> : TemplateArg<name> { | ||
| class Class<string name = "", bit is_variadic = 0> : TemplateArg<name> { |
Collaborator
There was a problem hiding this comment.
Do we really want these to have default arguments instead of passing "" in explicitly? I'm worried this makes it easier to add new subclasses that accidentally forget to set the name.
Contributor
Author
There was a problem hiding this comment.
Currently the names are purely cosmetic in nature except when you have NTTPs which rely on a previous type, so I'm not too worried if someone forgets. I wanted to auto-generate docs with this at some point, but I didn't get to it yet.
shafik
reviewed
Nov 19, 2025
clang/lib/Sema/SemaTemplate.cpp
Outdated
| return To; | ||
| } | ||
|
|
||
| // COND-RES(X, Y) be decltype(false ? declval<X(&)()>()() : declval<Y(&)()>()()) |
Collaborator
There was a problem hiding this comment.
This comment is very opaque, please expand on it.
After looking at the body of the function, it makes more sense (only just) but that is not how this should work. The comment on top of the function should be sufficient for the reader to understand what they expect to find in the body. I should not have to look at the body of the function to get the topline comment.
Contributor
Author
There was a problem hiding this comment.
What exactly do you want? I'm quoting the standard here, just like everywhere else.
huixie90
approved these changes
Nov 29, 2025
Member
huixie90
left a comment
huixie90
left a comment
There was a problem hiding this comment.
libc++ changes LGTM. Leave the final approval to the clang folks
Contributor
Author
|
Landing for now, since I don't think there are big concerns anymore and I'd like to avoid merge conflicts. @shafik @AaronBallman if you have any more comments I'm happy to address them post-commit. |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/19672 Here is the relevant piece of the build log for the reference |
Contributor
|
This breaks building the libc++ module in the chromium build for me: The diag is a bit poor in that it leaves out the bit which links the path.h note to path.h does include __iterator/iterator_traits.h which does include __type_traits/nat.h. So this might be a bug in the impl? Please take a look and revert for now if it takes a while to fix. |
Contributor
|
Here's another snippet that builds fine before this patch but not after it: Before, this built fine. After, it yields The snippet is a bit weird! The motivation is that it wants to say This worked before this change, but not after. Was that behavior change intentional? (The repro only works if the Maybe this is also what's causing the libc++-built-as-module diag mentioned above? |
nico
added a commit
that referenced
this pull request
Dec 3, 2025
Contributor
|
I reverted this in e05fffb for now, per Discord. |
augusto2112
pushed a commit
to augusto2112/llvm-project
that referenced
this pull request
Dec 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
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.
No description provided.