[Clang] Add __builtin_common_type #99473
Conversation
philnik777
force-pushed
the
builtin_common_type
branch
from
c893eb9 to
f9b9431
Compare
You can test this locally with the following command:git-clang-format --diff 91450f1b57b34034376662dae5452af8c992c103 4e3cd046d6f6e1b5be8e334245940ed8c9e41260 --extensions h,cpp -- clang/test/SemaCXX/type-trait-common-type.cpp clang/include/clang/AST/ASTContext.h clang/include/clang/AST/DeclID.h clang/include/clang/Basic/Builtins.h clang/lib/AST/ASTContext.cpp clang/lib/AST/ASTImporter.cpp clang/lib/AST/DeclTemplate.cpp clang/lib/Lex/PPMacroExpansion.cpp clang/lib/Sema/SemaLookup.cpp clang/lib/Sema/SemaTemplate.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp libcxx/include/__type_traits/common_type.hView the diff from clang-format here.diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index d4aef07e29..1072b73070 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3059,8 +3059,8 @@ void Sema::NoteAllFoundTemplates(TemplateName Name) {
}
static QualType builtinCommonTypeImpl(Sema &S, TemplateName BaseTemplate,
- SourceLocation TemplateLoc,
- ArrayRef<TemplateArgument> Ts) {
+ SourceLocation TemplateLoc,
+ ArrayRef<TemplateArgument> Ts) {
auto lookUpCommonType = [&](TemplateArgument T1,
TemplateArgument T2) -> QualType {
// Don't bother looking for other specializations if both types are
|
philnik777
force-pushed
the
builtin_common_type
branch
3 times, most recently
from
d11417d to
f034248
Compare
philnik777
force-pushed
the
builtin_common_type
branch
from
f034248 to
d6903da
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-clang-modules Author: Nikolas Klauser (philnik777) ChangesThis implements the logic of the Patch is 26.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99473.diff 16 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..d02e742297898 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -399,6 +399,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// The identifier '__type_pack_element'.
mutable IdentifierInfo *TypePackElementName = nullptr;
+ /// The identifier '__common_type'.
+ mutable IdentifierInfo *CommonTypeName = nullptr;
+
QualType ObjCConstantStringType;
mutable RecordDecl *CFConstantStringTagDecl = nullptr;
mutable TypedefDecl *CFConstantStringTypeDecl = nullptr;
@@ -606,6 +609,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
mutable ExternCContextDecl *ExternCContext = nullptr;
mutable BuiltinTemplateDecl *MakeIntegerSeqDecl = nullptr;
mutable BuiltinTemplateDecl *TypePackElementDecl = nullptr;
+ mutable BuiltinTemplateDecl *CommonTypeDecl = nullptr;
/// The associated SourceManager object.
SourceManager &SourceMgr;
@@ -1107,6 +1111,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
ExternCContextDecl *getExternCContextDecl() const;
BuiltinTemplateDecl *getMakeIntegerSeqDecl() const;
BuiltinTemplateDecl *getTypePackElementDecl() const;
+ BuiltinTemplateDecl *getCommonTypeDecl() const;
// Builtin Types.
CanQualType VoidTy;
@@ -1984,6 +1989,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
return TypePackElementName;
}
+ IdentifierInfo *getCommonTypeName() const {
+ if (!CommonTypeName)
+ CommonTypeName = &Idents.get("__common_type");
+ return CommonTypeName;
+ }
+
/// Retrieve the Objective-C "instancetype" type, if already known;
/// otherwise, returns a NULL type;
QualType getObjCInstanceType() {
diff --git a/clang/include/clang/AST/DeclID.h b/clang/include/clang/AST/DeclID.h
index e5e27389fac60..875e9a72b3951 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -84,13 +84,16 @@ enum PredefinedDeclIDs {
/// The internal '__type_pack_element' template.
PREDEF_DECL_TYPE_PACK_ELEMENT_ID = 17,
+
+ /// The internal '__common_type' template.
+ PREDEF_DECL_COMMON_TYPE_ID = 18,
};
/// The number of declaration IDs that are predefined.
///
/// For more information about predefined declarations, see the
/// \c PredefinedDeclIDs type and the PREDEF_DECL_*_ID constants.
-const unsigned int NUM_PREDEF_DECL_IDS = 18;
+const unsigned int NUM_PREDEF_DECL_IDS = 19;
/// GlobalDeclID means DeclID in the current ASTContext and LocalDeclID means
/// DeclID specific to a certain ModuleFile. Specially, in ASTWriter, the
diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h
index e85ec5b2dca14..4353b72f71383 100644
--- a/clang/include/clang/Basic/Builtins.h
+++ b/clang/include/clang/Basic/Builtins.h
@@ -309,7 +309,10 @@ enum BuiltinTemplateKind : int {
BTK__make_integer_seq,
/// This names the __type_pack_element BuiltinTemplateDecl.
- BTK__type_pack_element
+ BTK__type_pack_element,
+
+ /// This names the __common_type BuiltinTemplateDecl.
+ BTK__common_type,
};
} // end namespace clang
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3cb1aa935fe46..5c7945c4c5c58 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2282,6 +2282,10 @@ class Sema final : public SemaBase {
/// Check to see if a given expression could have '.c_str()' called on it.
bool hasCStrMethod(const Expr *E);
+ // Check whether a type member 'Type::Name' exists, and if yes, return the
+ // type. If there is no type, the QualType is null
+ QualType getTypeMember(StringRef Name, QualType Type);
+
/// Diagnose pointers that are always non-null.
/// \param E the expression containing the pointer
/// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..b8b4f426ff96c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1170,6 +1170,13 @@ ASTContext::getTypePackElementDecl() const {
return TypePackElementDecl;
}
+BuiltinTemplateDecl *ASTContext::getCommonTypeDecl() const {
+ if (!CommonTypeDecl)
+ CommonTypeDecl =
+ buildBuiltinTemplateDecl(BTK__common_type, getCommonTypeName());
+ return CommonTypeDecl;
+}
+
RecordDecl *ASTContext::buildImplicitRecord(StringRef Name,
RecordDecl::TagKind TK) const {
SourceLocation Loc;
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 0c27f6f5df2da..e4515e19a49a3 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5408,6 +5408,9 @@ ExpectedDecl ASTNodeImporter::VisitBuiltinTemplateDecl(BuiltinTemplateDecl *D) {
case BuiltinTemplateKind::BTK__type_pack_element:
ToD = Importer.getToContext().getTypePackElementDecl();
break;
+ case BuiltinTemplateKind::BTK__common_type:
+ ToD = Importer.getToContext().getCommonTypeDecl();
+ break;
}
assert(ToD && "BuiltinTemplateDecl of unsupported kind!");
Importer.MapImported(D, ToD);
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 722c7fcf0b0df..d290c91fb8290 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -1599,6 +1599,57 @@ createTypePackElementParameterList(const ASTContext &C, DeclContext *DC) {
nullptr);
}
+static TemplateParameterList *createCommonTypeList(const ASTContext &C,
+ DeclContext *DC) {
+ // class... Args
+ auto *Args = TemplateTypeParmDecl::Create(
+ C, DC, {}, {}, /*Depth=*/1, /*Position=*/0, /*Id=*/nullptr,
+ /*Typename=*/false, /*ParameterPack=*/true);
+ Args->setImplicit();
+
+ // <class... Args>
+ auto *BaseTemplateList =
+ TemplateParameterList::Create(C, {}, {}, Args, {}, nullptr);
+
+ // template <class... Args> class BaseTemplate
+ auto *BaseTemplate = TemplateTemplateParmDecl::Create(
+ C, DC, {}, /*Depth=*/0, /*Position=*/0, /*ParameterPack=*/false, {},
+ /*Typename=*/false, BaseTemplateList);
+
+ // class TypeMember
+ auto *TypeMember = TemplateTypeParmDecl::Create(
+ C, DC, {}, {}, /*Depth=*/1, /*Position=*/0, /*Id=*/nullptr,
+ /*Typename=*/false, /*ParameterPack=*/false);
+
+ // <class TypeMember>
+ auto *HasTypeMemberList =
+ TemplateParameterList::Create(C, {}, {}, TypeMember, {}, nullptr);
+
+ // template <class TypeMember> class HasTypeMember
+ auto *HasTypeMember =
+ TemplateTemplateParmDecl::Create(C, DC, {}, /*Depth=*/0, /*Position=*/1,
+ /*ParameterPack=*/false, {},
+ /*Typename=*/false, HasTypeMemberList);
+
+ // class HasNoTypeMember
+ auto *HasNoTypeMember = TemplateTypeParmDecl::Create(
+ C, DC, {}, {}, /*Depth=*/0, /*Position=*/2, /*Id=*/nullptr,
+ /*Typename=*/false, /*ParameterPack=*/false);
+
+ // class... Ts
+ auto *Ts = TemplateTypeParmDecl::Create(
+ C, DC, {}, {}, /*Depth=*/0, /*Position=*/3,
+ /*Id=*/nullptr, /*Typename=*/false, /*ParameterPack=*/true);
+ Ts->setImplicit();
+
+ // template <template <class... Args> class BaseTemplate,
+ // template <class TypeMember> class HasTypeMember, class HasNoTypeMember,
+ // class... Ts>
+ return TemplateParameterList::Create(
+ C, {}, {}, {BaseTemplate, HasTypeMember, HasNoTypeMember, Ts}, {},
+ nullptr);
+}
+
static TemplateParameterList *createBuiltinTemplateParameterList(
const ASTContext &C, DeclContext *DC, BuiltinTemplateKind BTK) {
switch (BTK) {
@@ -1606,6 +1657,8 @@ static TemplateParameterList *createBuiltinTemplateParameterList(
return createMakeIntegerSeqParameterList(C, DC);
case BTK__type_pack_element:
return createTypePackElementParameterList(C, DC);
+ case BTK__common_type:
+ return createCommonTypeList(C, DC);
}
llvm_unreachable("unhandled BuiltinTemplateKind!");
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 3913ff08c2eb5..4a6dd13229fe0 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1822,6 +1822,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
// Report builtin templates as being builtins.
.Case("__make_integer_seq", getLangOpts().CPlusPlus)
.Case("__type_pack_element", getLangOpts().CPlusPlus)
+ .Case("__common_type", getLangOpts().CPlusPlus)
// Likewise for some builtin preprocessor macros.
// FIXME: This is inconsistent; we usually suggest detecting
// builtin macros via #ifdef. Don't add more cases here.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 45b9bbb23dbf7..10c821fd367e7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6844,6 +6844,14 @@ CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
return Results;
}
+QualType Sema::getTypeMember(StringRef Name, QualType Type) {
+ auto Results = CXXRecordMembersNamed<TypeDecl>(Name, *this, Type);
+ assert(Results.size() <= 1);
+ if (Results.empty())
+ return {};
+ return Context.getTypeDeclType(*Results.begin());
+}
+
/// Check if we could call '.c_str()' on an object.
///
/// FIXME: This returns the wrong results in some cases (if cv-qualifiers don't
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 7a6a64529f52e..96551c5106b1b 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -932,6 +932,10 @@ bool Sema::LookupBuiltin(LookupResult &R) {
R.addDecl(getASTContext().getTypePackElementDecl());
return true;
}
+ if (II == getASTContext().getCommonTypeName()) {
+ R.addDecl(getASTContext().getCommonTypeDecl());
+ return true;
+ }
}
// Check if this is an OpenCL Builtin, and if so, insert its overloads.
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9d96201625389..bf3d35f0c1abb 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3058,6 +3058,141 @@ void Sema::NoteAllFoundTemplates(TemplateName Name) {
}
}
+static std::optional<QualType> commonTypeImpl(Sema &S,
+ TemplateName BaseTemplate,
+ SourceLocation TemplateLoc,
+ ArrayRef<TemplateArgument> Ts) {
+ auto lookUpCommonType = [&](TemplateArgument T1,
+ TemplateArgument T2) -> std::optional<QualType> {
+ // Don't bother looking for other specializations if both types are
+ // builtins - users aren't allowed to specialize for them
+ if (T1.getAsType()->isBuiltinType() && T2.getAsType()->isBuiltinType())
+ return commonTypeImpl(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())));
+ QualType BaseTemplateInst =
+ S.CheckTemplateIdType(BaseTemplate, TemplateLoc, Args);
+ if (S.RequireCompleteType(TemplateLoc, BaseTemplateInst,
+ diag::err_incomplete_type))
+ return std::nullopt;
+ if (QualType Type = S.getTypeMember("type", BaseTemplateInst);
+ !Type.isNull()) {
+ return Type;
+ }
+ return std::nullopt;
+ };
+
+ // Note A: For the common_type trait applied to a template parameter pack T of
+ // types, the member type shall be either defined or not present as follows:
+ switch (Ts.size()) {
+
+ // If sizeof...(T) is zero, there shall be no member type.
+ case 0:
+ return std::nullopt;
+
+ // If sizeof...(T) is one, let T0 denote the sole type constituting the
+ // pack T. The member typedef-name type shall denote the same type, if any, as
+ // common_type_t<T0, T0>; otherwise there shall be no member type.
+ case 1:
+ return lookUpCommonType(Ts[0], Ts[0]);
+
+ // If sizeof...(T) is two, let the first and second types constituting T be
+ // denoted by T1 and T2, respectively, and let D1 and D2 denote the same types
+ // as decay_t<T1> and decay_t<T2>, respectively.
+ case 2: {
+ QualType T1 = Ts[0].getAsType();
+ QualType T2 = Ts[1].getAsType();
+ QualType D1 = S.BuiltinDecay(T1, {});
+ QualType D2 = S.BuiltinDecay(T2, {});
+
+ // If is_same_v<T1, D1> is false or is_same_v<T2, D2> is false, let C denote
+ // the same type, if any, as common_type_t<D1, D2>.
+ if (!S.Context.hasSameType(T1, D1) || !S.Context.hasSameType(T2, D2)) {
+ return lookUpCommonType(D1, D2);
+ }
+
+ // Otherwise, if decay_t<decltype(false ? declval<D1>() : declval<D2>())>
+ // denotes a valid type, let C denote that type.
+ {
+ auto CheckConditionalOperands =
+ [&](bool ConstRefQual) -> std::optional<QualType> {
+ EnterExpressionEvaluationContext UnevaluatedContext(
+ S, Sema::ExpressionEvaluationContext::Unevaluated);
+ Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
+ Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
+
+ // false
+ OpaqueValueExpr CondExpr({}, S.Context.BoolTy,
+ ExprValueKind::VK_PRValue);
+ ExprResult Cond = &CondExpr;
+
+ auto EVK =
+ ConstRefQual ? ExprValueKind::VK_LValue : ExprValueKind::VK_PRValue;
+ if (ConstRefQual) {
+ D1.addConst();
+ D2.addConst();
+ }
+
+ // declval<D1>()
+ OpaqueValueExpr LHSExpr(TemplateLoc, D1, EVK);
+ ExprResult LHS = &LHSExpr;
+
+ // declval<D2>()
+ OpaqueValueExpr RHSExpr(TemplateLoc, D2, EVK);
+ ExprResult RHS = &RHSExpr;
+
+ ExprValueKind VK = VK_PRValue;
+ ExprObjectKind OK = OK_Ordinary;
+
+ // decltype(false ? declval<D1>() : declval<D2>())
+ QualType Result =
+ S.CheckConditionalOperands(Cond, LHS, RHS, VK, OK, TemplateLoc);
+
+ if (Result.isNull() || SFINAE.hasErrorOccurred())
+ return std::nullopt;
+
+ // decay_t<decltype(false ? declval<D1>() : declval<D2>())>
+ return S.BuiltinDecay(Result, TemplateLoc);
+ };
+
+ if (auto Res = CheckConditionalOperands(false))
+ return Res;
+
+ // Let:
+ // CREF(A) be add_lvalue_reference_t<const remove_reference_t<A>>,
+ // COND-RES(X, Y) be
+ // decltype(false ? declval<X(&)()>()() : declval<Y(&)()>()()).
+
+ // C++20 only
+ // Otherwise, if COND-RES(CREF(D1), CREF(D2)) denotes a type, let C denote
+ // the type decay_t<COND-RES(CREF(D1), CREF(D2))>.
+ if (!S.Context.getLangOpts().CPlusPlus20)
+ return std::nullopt;
+ return CheckConditionalOperands(true);
+ }
+ }
+
+ // If sizeof...(T) is greater than two, let T1, T2, and R, respectively,
+ // denote the first, second, and (pack of) remaining types constituting T. Let
+ // C denote the same type, if any, as common_type_t<T1, T2>. If there is such
+ // a type C, the member typedef-name type shall denote the same type, if any,
+ // as common_type_t<C, R...>. Otherwise, there shall be no member type.
+ default: {
+ std::optional<QualType> Result = Ts[Ts.size() - 1].getAsType();
+ for (size_t i = Ts.size() - 1; i != 0; --i) {
+ Result = lookUpCommonType(Ts[i - 1].getAsType(), *Result);
+ if (!Result)
+ return std::nullopt;
+ }
+ return Result;
+ }
+ }
+}
+
static QualType
checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD,
ArrayRef<TemplateArgument> Converted,
@@ -3114,7 +3249,7 @@ checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD,
TemplateLoc, SyntheticTemplateArgs);
}
- case BTK__type_pack_element:
+ case BTK__type_pack_element: {
// Specializations of
// __type_pack_element<Index, T_1, ..., T_N>
// are treated like T_Index.
@@ -3140,6 +3275,29 @@ checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD,
int64_t N = Index.getExtValue();
return Ts.getPackAsArray()[N].getAsType();
}
+
+ case BTK__common_type: {
+ assert(Converted.size() == 4);
+ if (Converted[0].isDependent() || Converted[1].isDependent() ||
+ Converted[2].isDependent() || Converted[3].isDependent())
+ return Context.getCanonicalTemplateSpecializationType(TemplateName(BTD),
+ Converted);
+
+ TemplateName BaseTemplate = Converted[0].getAsTemplate();
+ TemplateName HasTypeMember = Converted[1].getAsTemplate();
+ QualType HasNoTypeMember = Converted[2].getAsType();
+ ArrayRef<TemplateArgument> Ts = Converted[3].getPackAsArray();
+ if (auto CT = commonTypeImpl(SemaRef, BaseTemplate, TemplateLoc, Ts)) {
+ TemplateArgumentListInfo TAs;
+ TAs.addArgument(TemplateArgumentLoc(
+ TemplateArgument(*CT), SemaRef.Context.getTrivialTypeSourceInfo(
+ *CT, TemplateArgs[1].getLocation())));
+
+ return SemaRef.CheckTemplateIdType(HasTypeMember, TemplateLoc, TAs);
+ }
+ return HasNoTypeMember;
+ }
+ }
llvm_unreachable("unexpected BuiltinTemplateDecl!");
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index afdeccaf93a9d..40ebc2c5d020f 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7789,6 +7789,9 @@ static Decl *getPredefinedDecl(ASTContext &Context, PredefinedDeclIDs ID) {
case PREDEF_DECL_TYPE_PACK_ELEMENT_ID:
return Context.getTypePackElementDecl();
+
+ case PREDEF_DECL_COMMON_TYPE_ID:
+ return Context.getCommonTypeDecl();
}
llvm_unreachable("PredefinedDeclIDs unknown enum value");
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index c78d8943d6d92..b17eeed357aac 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5036,6 +5036,8 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) {
PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID);
RegisterPredefDecl(Context.TypePackElementDecl,
PREDEF_DECL_TYPE_PACK_ELEMENT_ID);
+ RegisterPredefDecl(Context.CommonTypeDecl,
+ PREDEF_DECL_COMMON_TYPE_ID);
const TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
diff --git a/clang/test/SemaCXX/type-trait-common-type.cpp b/clang/test/SemaCXX/type-trait-common-type.cpp
new file mode 100644
index 0000000000000..44207f066a333
--- /dev/null
+++ b/clang/test/SemaCXX/type-trait-common-type.cpp
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=c++17 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=c++20 %s
+
+#if !__has_builtin(__common_type)
+# error
+#endif
+
+// expected-note@*:* {{template declaration from hidden source: template <class, template <class> class, class>}}
+
+void test() {
+ __common_type<> a; // expected-error {{too few template arguments for template '__common_type'}}
+}
+
+struct empty_type {};
+
+template <class T>
+struct type_identity {
+ using type = T;
+};
+
+template <class...>
+struct common_type;
+
+template <class... Args>
+using common_type_base = __common_type<common_type, type_identity, empty_type, Args...>; // expected-error {{incomplete type 'common_type<Incomplete, Incomplete>' where a complete type is required}}
+
+template <class... Args>
+struct common_type : common_type_base<Args...> {};
+
+struct Incomplete;
+
+template<>
+struct common_type<Incomplete, Incomplete>; // expected-note {{forward declaration}}
+
+static_assert(__is_same(common_type_base<>, empty_type));
+static_assert(__is_same(common_type_base<Incomplete>, empty_type)); // expected-note {{requested here}}
+static_assert(__is_same(common_type_base<char>, type_identity<char>));
+static_asse...
[truncated]
|
There was a problem hiding this comment.
Overall, the implementation seems fine.
Do we want a release note for this since it’s supposed to be an internal thing?
CC @AaronBallman
Also, some documentation somewhere as to what this builtin does would be nice, even if it’s just in a comment somewhere. It took me a bit to figure out what exactly its semantics are supposed to be.
|
Should we prefix this with |
Yes, if we are doing this, we should be consistent going forward |
I would like that. It's easier directly determine |
|
|
||
| // decltype(false ? declval<D1>() : declval<D2>()) | ||
| QualType Result = | ||
| S.CheckConditionalOperands(Cond, LHS, RHS, VK, OK, TemplateLoc); |
There was a problem hiding this comment.
So, I think we can call CXXCheckConditionalOperands here.
But the question I have is, should we factor out the part of CXXCheckConditionalOperands that deals with types directly and use that?
One one hand, doing surgery to CXXCheckConditionalOperands requires care and might be a tad difficult, on the other hand the whole danse we are doing here is a bit convoluted
There was a problem hiding this comment.
AFAICT we'd have to do that for all the extensions as well, which seems like it'd be a lot of work. We'd also only save the SFINAE context and the opaque values, which seems to me like not that much code. It'd probably be a bit faster, but I'm not sure it's that significant.
|
I spent a lot of time trying to implement this myself, but I always struggled to capture detecting user specialisations of |
The |
| // Don't bother looking for other specializations if both types are | ||
| // builtins - users aren't allowed to specialize for them |
There was a problem hiding this comment.
Can we diagnose the case where users do this, please? Similarly for specialising common_type<>, common_type<T>, and common_type<T1, T2, T3, Ts...>.
There was a problem hiding this comment.
We can do that, but I don't think that's part of this project. AFAICT this requires tinkering with the template specialization code and check whether a specialization is allowed to exist. To me that seems like an entire project on its own.
There was a problem hiding this comment.
We need to handle the first case in this patch because it is currently silently changing the outcome of common_type<builtin, builtin>. Although the standard gives us licence to silently change this, it isn't fair to users who either weren't aware of the consequences of specialising this, or are depending on such code.
There was a problem hiding this comment.
How is this any different from all the other cases where we switched to builtins, which results in potential breaks?
There was a problem hiding this comment.
Good point. There are only a handful of cases that this applies to, but they should also break loudly. In that case, I'd prefer it if getting that diagnostic logic into Clang preceded submitting this PR.
There was a problem hiding this comment.
Given the precedent I don't think it's reasonable to block this patch on diagnosing some edge cases.
There was a problem hiding this comment.
It is only precedent due to it not having been previously recognised as an issue. It has now been identified and should be addressed, instead of increasing the problem's surface area.
We should do things right, not wrong because "they were done that way previously".
There was a problem hiding this comment.
I believe it would make sense for such a diagnostic to work today, even without the builtin. For example, it would be nice for the following code to be diagnosed with a warning (or an error, whatever):
#include <type_traits>
template <>
struct std::common_type<int, int> { using type = long; }; // should warn
struct Foo { };
template <>
struct std::common_type<Foo> { using type = int; }; // should warn
// etc...I don't think this should be tied to the implementation of the builtin, i.e. it should be diagnosed where the user writes the invalid specialization, not where we use std::common_type_t. As such, I think it would make sense to tackle this as a follow-up and I filed an issue for it: #101509
clang/lib/Sema/SemaTemplate.cpp
Outdated
| if (S.RequireCompleteType(TemplateLoc, BaseTemplateInst, | ||
| diag::err_incomplete_type)) | ||
| return QualType(); | ||
| return S.getTypeMember(BaseTemplateInst, "type"); |
There was a problem hiding this comment.
It's a bit incongruous that we hardcode type here but don't hardcode any other names. Could we change the first parameter so that the library passes in std::common_type_t instead of std::common_type? (Or does alias template instantiation add noticeable performance overhead compared to this approach?)
There was a problem hiding this comment.
Nice idea! Looks like it works just fine and doesn't have any impact on the performance. The only downside is that we don't diagnose incomplete common_types anymore, since they SFINAE away now. OTOH http://eel.is/c++draft/type.traits#meta.trans.other-4.2 could be read as requiring that behaviour so who knows.
There was a problem hiding this comment.
How much of a performance difference are you measuring with this change?
That depends heavily on what you're looking at. For common_types with a lot of builtin types, I've seen a 10x improvement. When it's basically just combining specializations I've seen as low as a 1.25x improvement. When having classes that are convertible to another there is about a 2x improvement.
| // Don't bother looking for other specializations if both types are | ||
| // builtins - users aren't allowed to specialize for them |
There was a problem hiding this comment.
Given the precedent I don't think it's reasonable to block this patch on diagnosing some edge cases.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| if (S.RequireCompleteType(TemplateLoc, BaseTemplateInst, | ||
| diag::err_incomplete_type)) | ||
| return QualType(); | ||
| return S.getTypeMember(BaseTemplateInst, "type"); |
There was a problem hiding this comment.
Nice idea! Looks like it works just fine and doesn't have any impact on the performance. The only downside is that we don't diagnose incomplete common_types anymore, since they SFINAE away now. OTOH http://eel.is/c++draft/type.traits#meta.trans.other-4.2 could be read as requiring that behaviour so who knows.
|
|
||
| // decltype(false ? declval<D1>() : declval<D2>()) | ||
| QualType Result = | ||
| S.CheckConditionalOperands(Cond, LHS, RHS, VK, OK, TemplateLoc); |
There was a problem hiding this comment.
AFAICT we'd have to do that for all the extensions as well, which seems like it'd be a lot of work. We'd also only save the SFINAE context and the opaque values, which seems to me like not that much code. It'd probably be a bit faster, but I'm not sure it's that significant.
philnik777
changed the title
|
gentle ping~ |
There was a problem hiding this comment.
Changes LGTM! It would be nice to add an additional test case that does -ast-dump (or I suppose -ast-print would work as well) to verify the builtin common type list has the expected form. However, I don't insist as I expect the libc++ usage to quickly tell us if we change the signature inadvertently.
| using __common_type_t = typename common_type<_Args...>::type; | ||
|
|
||
| template <class... _Args> | ||
| struct common_type : __builtin_common_type<__common_type_t, __type_identity, __empty, _Args...> {}; |
There was a problem hiding this comment.
@philnik777 This causes a problem when modules are enabled, because we'd have to export __builtin_common_type from this module but I don't think there's any way of doing that. That leads to errors of this kind:
# | In module 'std' imported from /llvm/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.destr/dtor.pass.cpp:19:
# | /llvm/build-runtimes/libcxx/test-suite-install/include/c++/v1/__chrono/duration.h:90:101: error: no type named 'type' in 'std::common_type<long double, long double, long>'
# | 90 | typedef typename common_type<typename _ToDuration::rep, typename _FromDuration::rep, intmax_t>::type _Ct;
# | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
# | /llvm/build-runtimes/libcxx/test-suite-install/include/c++/v1/__chrono/duration.h:107:10: note: in instantiation of member function 'std::chrono::__duration_cast<std::chrono::duration<long double>, std::chrono::duration<long double, std::ratio<1, 1000>>>::operator()' requested here
# | 107 | return __duration_cast<duration<_Rep, _Period>, _ToDuration>()(__fd);
# | | ^
# | /llvm/build-runtimes/libcxx/test-suite-install/include/c++/v1/__chrono/duration.h:220:24: note: in instantiation of function template specialization 'std::chrono::duration_cast<std::chrono::duration<long double, std::ratio<1, 1000>>, long double, std::ratio<1>, 0>' requested here
# | 220 | : __rep_(chrono::duration_cast<duration>(__d).count()) {}
# | | ^
# | /llvm/build-runtimes/libcxx/test-suite-install/include/c++/v1/__chrono/duration.h:385:33: note: in instantiation of function template specialization 'std::chrono::duration<long double, std::ratio<1, 1000>>::duration<long double, std::ratio<1>, 0>' requested here
# | 385 | return _Ct(__lhs).count() <=> _Ct(__rhs).count();
# | | ^
# | /llvm/build-runtimes/libcxx/test-suite-install/include/c++/v1/__thread/this_thread.h:45:13: note: in instantiation of function template specialization 'std::chrono::operator<=><long long, std::ratio<1, 1000>, long double, std::ratio<1>>' requested here
# | 45 | if (__d < __max) {
# | | ^
# | /llvm/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.destr/dtor.pass.cpp:63:29: note: in instantiation of function template specialization 'std::this_thread::sleep_for<long long, std::ratio<1, 1000>>' requested here
# | 63 | std::this_thread::sleep_for(std::chrono::milliseconds(250));
# | | ^
# | 1 error generated.
This didn't show up in the CI here because the modules CI doesn't run against Clang trunk.
There was a problem hiding this comment.
(I am trying to fix this in the following PR where I re-land the modulemap changes: #110501)
There was a problem hiding this comment.
@ldionne I think you actually want to export __type_traits/type_identity.h and __utility/empty.h, since the alias resolves to one of these types in the end.
Based on a comment in llvm#99473, it seems like `export *` may be overkill.
Based on a comment in llvm#99473, it seems like `export *` may be overkill.
Based on a comment in #99473, it seems like `export *` may be overkill.
| return SemaRef.CheckTemplateIdType(HasTypeMember, TemplateLoc, TAs); | ||
| } | ||
| return HasNoTypeMember; |
There was a problem hiding this comment.
This is missing wrapping the result in a TemplateSpecializationType, just like the other builtin templates do, because users of CheckTemplateId assume that it returns a TST of whatever TemplateName it is checking.
Otherwise, these users will wrap the resulting type in a TemplateSpecializationTypeLoc which is unrelated to what is returned.
Worse, in the negative case, HasNoTypeMember can be something which is not a TST at all, and this will lead to crashes when we try to build a TypeLoc with the wrong kind.
There was a problem hiding this comment.
I'm not sure I understand. __type_pack_element above also just returns a type in the usual case.
This implements the logic of the
common_typebase template as a builtin alias. If there should be notypemember, an empty class is returned. Otherwise a specialization of atype_identity-like class is returned. The base template (i.e.std::common_type) as well as the empty class andtype_identity-like struct are given as arguments to the builtin.