faisalv created this revision.
faisalv added a reviewer: rsmith.
faisalv added a subscriber: cfe-commits.
Clang currently does no real checking of the template argument list when a
template-id is used to declare a constructor:
template<class T> struct X {
X<T*, T>(); // Clang erroneously accepts this.
};
Both gcc and edg emit an error on the above code (though one could claim the
spec temp.local p1 is perhaps not as explicit as it could be in this regard)
See: https://llvm.org/bugs/show_bug.cgi?id=8170
http://reviews.llvm.org/D15005
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Parse/ParseExprCXX.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaTemplate.cpp
test/CXX/temp/temp.res/temp.local/p1.cpp
Index: test/CXX/temp/temp.res/temp.local/p1.cpp
===================================================================
--- test/CXX/temp/temp.res/temp.local/p1.cpp
+++ test/CXX/temp/temp.res/temp.local/p1.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
+
// C++0x [temp.local]p1:
// Like normal (non-template) classes, class templates have an
@@ -11,12 +11,15 @@
template <typename T> struct X0 {
X0();
~X0();
+ X0<T>(int);
+ X0<T, T>(int*); //expected-error{{expected 'X0' or 'X0<T>'}}
X0 f(const X0&);
};
// Test non-type template parameters.
template <int N1, const int& N2, const int* N3> struct X1 {
X1();
+ template<class U> X1<U,N1,"wtf">(char); //expected-error{{expected 'X1' or 'X1<N1,N2,N3>'}}
~X1();
X1 f(const X1& x1a) { X1 x1b(x1a); return x1b; }
};
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -518,6 +518,65 @@
llvm_unreachable("Unhandled parsed template argument");
}
+std::string
+Sema::stringifyTemplateArguments(const TemplateSpecializationType *TST) {
+ ArrayRef<TemplateArgument> TemplateArgs(TST->getArgs(), TST->getNumArgs());
+
+ llvm::SmallString<256> StringifiedTemplateArgs;
+ std::for_each(TemplateArgs.begin(), TemplateArgs.end(),
+ [&StringifiedTemplateArgs, this](const TemplateArgument &TA) {
+ if (const bool IsFirst = !StringifiedTemplateArgs.size())
+ StringifiedTemplateArgs = "<";
+ else
+ StringifiedTemplateArgs += ",";
+ llvm::raw_svector_ostream OS(StringifiedTemplateArgs);
+ TA.print(this->getPrintingPolicy(), OS);
+ });
+
+ StringifiedTemplateArgs += ">";
+ return StringifiedTemplateArgs.c_str();
+}
+
+void Sema::checkUnqualifiedTemplateIdIsConstructor(
+ TemplateIdAnnotation *TemplateId, const CXXRecordDecl *TemplatedClass) {
+ assert(TemplatedClass->getDescribedClassTemplate());
+ assert(TemplateId->Name == TemplatedClass->getIdentifier() &&
+ "TemplateId must have the same identifier as that of the class!");
+
+ // Get the type that corresponds to this template id, while suppressing all
+ // diagnostics.
+ const bool PrevSuppress = Diags.getSuppressAllDiagnostics();
+ Diags.setSuppressAllDiagnostics(true);
+ TypeResult TemplateIdTy = ActOnTemplateIdType(
+ TemplateId->SS, TemplateId->TemplateKWLoc,
+ TemplateTy::make(
+ TemplateName(TemplatedClass->getDescribedClassTemplate())),
+ TemplateId->TemplateNameLoc, TemplateId->LAngleLoc,
+ ASTTemplateArgsPtr(TemplateId->getTemplateArgs(), TemplateId->NumArgs),
+ TemplateId->RAngleLoc, /*IsCtorOrDtorName*/ true);
+ Diags.setSuppressAllDiagnostics(PrevSuppress);
+
+ // If the template-id type is the same type as the type of the
+ // templated-class, all is ok - else emit a diagnostic.
+ if (TemplateIdTy.get() &&
+ Context.hasSameType(GetTypeFromParser(TemplateIdTy.get()).getTypePtr(),
+ Context.getTypeDeclType(TemplatedClass).getTypePtr()))
+ return;
+
+ // Emit a diagnostic, indicating that the template arguments do not match up
+ // to the injected class name.
+ std::string ExpectedTemplateId = TemplateId->Name->getName();
+ ExpectedTemplateId += stringifyTemplateArguments(
+ cast<InjectedClassNameType>(Context.getTypeDeclType(TemplatedClass))
+ ->getInjectedTST());
+
+ Diag(TemplateId->TemplateNameLoc,
+ diag::err_expected_constructor_template_id)
+ << TemplateId->Name << ExpectedTemplateId.c_str()
+ << FixItHint::CreateRemoval(
+ SourceRange(TemplateId->LAngleLoc, TemplateId->RAngleLoc));
+}
+
/// \brief Translates template arguments as provided by the parser
/// into template arguments used by semantic analysis.
void Sema::translateTemplateArguments(const ASTTemplateArgsPtr &TemplateArgsIn,
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -1270,18 +1270,23 @@
/// nested classes, this will only return true if II is the name of
/// the innermost class.
bool Sema::isCurrentClassName(const IdentifierInfo &II, Scope *,
- const CXXScopeSpec *SS) {
+ const CXXScopeSpec *SS,
+ const CXXRecordDecl **CurClassOut) {
assert(getLangOpts().CPlusPlus && "No class names in C!");
CXXRecordDecl *CurDecl;
+ if (CurClassOut) *CurClassOut = nullptr;
if (SS && SS->isSet() && !SS->isInvalid()) {
DeclContext *DC = computeDeclContext(*SS, true);
CurDecl = dyn_cast_or_null<CXXRecordDecl>(DC);
} else
CurDecl = dyn_cast_or_null<CXXRecordDecl>(CurContext);
if (CurDecl && CurDecl->getIdentifier())
- return &II == CurDecl->getIdentifier();
+ if (&II == CurDecl->getIdentifier()) {
+ if (CurClassOut) *CurClassOut = CurDecl;
+ return true;
+ }
return false;
}
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2384,7 +2384,8 @@
bool AllowConstructorName,
ParsedType ObjectType,
SourceLocation& TemplateKWLoc,
- UnqualifiedId &Result) {
+ UnqualifiedId &Result,
+ const DeclSpec *DS) {
// Handle 'A::template B'. This is for template-ids which have not
// already been annotated by ParseOptionalCXXScopeSpecifier().
@@ -2437,10 +2438,10 @@
// template-id (already parsed and annotated)
if (Tok.is(tok::annot_template_id)) {
TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
-
+ const CXXRecordDecl *CurClass = nullptr;
// If the template-name names the current class, then this is a constructor
if (AllowConstructorName && TemplateId->Name &&
- Actions.isCurrentClassName(*TemplateId->Name, getCurScope(), &SS)) {
+ Actions.isCurrentClassName(*TemplateId->Name, getCurScope(), &SS, &CurClass)) {
if (SS.isSet()) {
// C++ [class.qual]p2 specifies that a qualified template-name
// is taken as the constructor name where a constructor can be
@@ -2464,6 +2465,22 @@
return false;
}
+ // Only perform the template-id check if not declaring a friend here. If a
+ // template-id involves the current class in a friend declaration
+ // referring to a constructor, it must be a qualified id - and is
+ // diagnosed as an error when semantically checking friend function
+ // declarations for such things downstream.
+
+ //
+ // For e.g.
+ // template<class T> struct X {
+ // X<T,T>() { } <-- check template args here
+ // friend X<T,T>() { } <-- ignore template args, bound to fail later
+ // };
+ //
+ if (!DS || !DS->isFriendSpecified())
+ Actions.checkUnqualifiedTemplateIdIsConstructor(TemplateId, CurClass);
+
Result.setConstructorTemplateId(TemplateId);
ConsumeToken();
return false;
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5179,7 +5179,8 @@
AllowConstructorName,
ParsedType(),
TemplateKWLoc,
- D.getName()) ||
+ D.getName(),
+ &D.getDeclSpec()) ||
// Once we're past the identifier, if the scope was bad, mark the
// whole declarator bad.
D.getCXXScopeSpec().isInvalid()) {
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5147,7 +5147,9 @@
// C++ Classes
//
bool isCurrentClassName(const IdentifierInfo &II, Scope *S,
- const CXXScopeSpec *SS = nullptr);
+ const CXXScopeSpec *SS = nullptr,
+ const CXXRecordDecl **CurClassOut = nullptr);
+
bool isCurrentClassNameTypo(IdentifierInfo *&II, const CXXScopeSpec *SS);
bool ActOnAccessSpecifier(AccessSpecifier Access,
@@ -5617,6 +5619,17 @@
SourceLocation TemplateLoc,
TemplateArgumentListInfo &TemplateArgs);
+ // Represent the template arguments of the template specialization as a
+ // string.
+ std::string
+ Sema::stringifyTemplateArguments(const TemplateSpecializationType *TST);
+
+ // Given a template-id and the templated class, check whether the template id
+ // represents the class itself (i.e. the injected class name)
+ void
+ checkUnqualifiedTemplateIdIsConstructor(TemplateIdAnnotation *TemplateId,
+ const CXXRecordDecl *TemplatedClass);
+
TypeResult
ActOnTemplateIdType(CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
TemplateTy Template, SourceLocation TemplateLoc,
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2485,7 +2485,8 @@
bool AllowConstructorName,
ParsedType ObjectType,
SourceLocation& TemplateKWLoc,
- UnqualifiedId &Result);
+ UnqualifiedId &Result,
+ const DeclSpec *DS = nullptr);
private:
//===--------------------------------------------------------------------===//
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3671,6 +3671,11 @@
"explicit specialization has extraneous, inconsistent storage class "
"'%select{none|extern|static|__private_extern__|auto|register}0'">;
+// C++ class template constructor
+def err_expected_constructor_template_id : Error<
+ "constructor template-id has inconsistent (and extraneous) template "
+ "argument list: expected %0 or '%1'">;
+
// C++ class template specializations and out-of-line definitions
def err_template_spec_needs_header : Error<
"template specialization requires 'template<>'">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
