rsmith added inline comments.
================
Comment at: include/clang/AST/TemplateName.h:93-100
@@ +92,10 @@
+
+/// \brief A structure for storing the information associated with an
+/// builtin template name.
+class BuiltinTemplateStorage : public UncommonTemplateNameStorage {
+ friend class ASTContext;
+
+public:
+ BuiltinTemplateStorage() : UncommonTemplateNameStorage(Builtin, 0) {}
+};
+
----------------
Please add an enum for builtin template names (and pass it around when creating
`TemplateName`s and `BuiltinTemplateDecl`s as appropriate), even though it will
only have one value for now.
================
Comment at: lib/Parse/ParseDecl.cpp:2937-2939
@@ +2936,5 @@
+ if (Type.isInvalid()) {
+ // If we failed to parse the template ID but skipped ahead to a >,
we're
+ // not
+ // going to be able to form a token annotation. Eat the '>' if
present.
+ TryConsumeToken(tok::greater);
----------------
Reflow this comment.
================
Comment at: lib/Sema/SemaTemplate.cpp:2060-2102
@@ -2055,1 +2059,45 @@
+ // Instantiations involving __make_integer_seq<S, T, N> are treated like
+ // S<T, 0, ..., N-1>.
+ TemplateArgumentListInfo SyntheticTemplateArgs;
+ TemplateName UnderlyingName = Name;
+ bool UseSyntheticTemplateArgs = false;
+ if (IsBuiltinTemplate) {
+ assert(Converted.size() == 3 && "Unexpected BuiltinTemplate!");
+
+ // Switch our template from __make_integer_seq to S.
+ Name = Converted[0].getAsTemplate();
+ Template = Name.getAsTemplateDecl();
+ TemplateArgument NumArgsArg = Converted[2];
+ if (!NumArgsArg.isDependent()) {
+ // The first template argument will be reused as the template decl that
+ // our synthetic template arguments will be applied to while the last
+ // template argument will be replaced with a pack.
+ TemplateArgument SequenceType = Converted[1];
+ Converted.clear();
+ Converted.push_back(SequenceType);
+ UseSyntheticTemplateArgs = true;
+
+ // Diagnose attempts to create integer sequences with a negative length.
+ llvm::APSInt NumArgs = NumArgsArg.getAsIntegral();
+ if (NumArgs < 0)
+ Diag(UnderlyingTemplateArgs[2].getLocation(),
+ diag::err_integer_sequence_negative_length);
+
+ QualType Ty = NumArgsArg.getIntegralType();
+ SmallVector<TemplateArgument, 2> ArgumentPack;
+ for (llvm::APSInt I(NumArgs.getBitWidth(), NumArgs.isUnsigned());
+ I < NumArgs; ++I) {
+ TemplateArgument TA(Context, I, Ty);
+ Expr *E =
+ BuildExpressionFromIntegralTemplateArgument(TA, SourceLocation())
+ .getAs<Expr>();
+ ArgumentPack.push_back(TA);
+ SyntheticTemplateArgs.addArgument(
+ TemplateArgumentLoc(TemplateArgument(E), E));
+ }
+ Converted.push_back(
+ TemplateArgument::CreatePackCopy(Context, ArgumentPack));
+ }
+ }
+
----------------
Please factor this out into a separate function, `CheckBuiltinTemplateIdType`
or similar.
================
Comment at: lib/Sema/SemaTemplate.cpp:2069-2071
@@ +2068,5 @@
+ // Switch our template from __make_integer_seq to S.
+ Name = Converted[0].getAsTemplate();
+ Template = Name.getAsTemplateDecl();
+ TemplateArgument NumArgsArg = Converted[2];
+ if (!NumArgsArg.isDependent()) {
----------------
I would prefer to see this structured as follows:
In the `if` chain below where we're dispatching on the type of the template, if
the template is a `BuiltinTemplateDecl`, then do some special-case handling to
compute the `CanonType` (which is really poorly / misleadingly named; it's just
the underlying type, not the canonical type). In the `__make_integer_sequence`
case, this would involve recursively calling back into `CheckTemplateIdType` to
build the template-id for the expanded form.
This means you'll retain an intermediate TST between the `__make_integer_seq`
and the ultimate type.
================
Comment at: lib/Sema/SemaTemplate.cpp:2083-2085
@@ +2082,5 @@
+ llvm::APSInt NumArgs = NumArgsArg.getAsIntegral();
+ if (NumArgs < 0)
+ Diag(UnderlyingTemplateArgs[2].getLocation(),
+ diag::err_integer_sequence_negative_length);
+
----------------
Is it worth putting some kind of upper bound on what we'll accept here?
http://reviews.llvm.org/D13786
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
