This patch causes http://llvm.org/PR20660. Can you take a look?
On Tue, Aug 12, 2014 at 1:30 AM, Richard Smith <[email protected]> wrote: > Author: rsmith > Date: Mon Aug 11 18:30:23 2014 > New Revision: 215408 > > URL: http://llvm.org/viewvc/llvm-project?rev=215408&view=rev > Log: > Reject varargs '...' in function prototype if there are more parameters > after > it. Diagnose with recovery if it appears after a function parameter that > was > obviously supposed to be a parameter pack. Otherwise, warn if it > immediately > follows a function parameter pack, because the user most likely didn't > intend > to write a parameter pack followed by a C-style varargs ellipsis. > > This warning can be syntactically disabled by using ", ..." instead of > "...". > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/Parse/ParseDecl.cpp > cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp > cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p14.cpp > cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/metafunctions.cpp > cfe/trunk/test/Parser/cxx-variadic-func.cpp > cfe/trunk/test/Parser/cxx11-templates.cpp > cfe/trunk/test/SemaCXX/issue547.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Aug 11 > 18:30:23 2014 > @@ -503,6 +503,17 @@ def note_bracket_depth : Note< > def err_misplaced_ellipsis_in_declaration : Error< > "'...' must %select{immediately precede declared identifier|" > "be innermost component of anonymous pack declaration}0">; > +def warn_misplaced_ellipsis_vararg : Warning< > + "'...' in this location creates a C-style varargs function" > + "%select{, not a function parameter pack|}0">, > + InGroup<DiagGroup<"ambiguous-ellipsis">>; > +def note_misplaced_ellipsis_vararg_existing_ellipsis : Note< > + "preceding '...' declares a function parameter pack">; > +def note_misplaced_ellipsis_vararg_add_ellipsis : Note< > + "place '...' %select{immediately before declared identifier|here}0 " > + "to declare a function parameter pack">; > +def note_misplaced_ellipsis_vararg_add_comma : Note< > + "insert ',' before '...' to silence this warning">; > def ext_abstract_pack_declarator_parens : ExtWarn< > "ISO C++11 requires a parenthesized pack declaration to have a name">, > InGroup<DiagGroup<"anonymous-pack-parens">>; > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Mon Aug 11 18:30:23 2014 > @@ -5585,6 +5585,10 @@ public: > // C++ Variadic Templates (C++0x [temp.variadic]) > > > //===--------------------------------------------------------------------===// > > + /// Determine whether an unexpanded parameter pack might be permitted > in this > + /// location. Useful for error recovery. > + bool isUnexpandedParameterPackPermitted(); > + > /// \brief The context in which an unexpanded parameter pack is > /// being diagnosed. > /// > > Modified: cfe/trunk/lib/Parse/ParseDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/lib/Parse/ParseDecl.cpp (original) > +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Aug 11 18:30:23 2014 > @@ -5426,6 +5426,15 @@ void Parser::ParseParameterDeclarationCl > // Otherwise, we have something. Add it and let semantic analysis > try > // to grok it and add the result to the ParamInfo we are building. > > + // Last chance to recover from a misplaced ellipsis in an attempted > + // parameter pack declaration. > + if (Tok.is(tok::ellipsis) && > + (NextToken().isNot(tok::r_paren) || > + (!ParmDeclarator.getEllipsisLoc().isValid() && > + !Actions.isUnexpandedParameterPackPermitted())) && > + Actions.containsUnexpandedParameterPacks(ParmDeclarator)) > + DiagnoseMisplacedEllipsisInDeclarator(ConsumeToken(), > ParmDeclarator); > + > // Inform the actions module about the parameter declarator, so it > gets > // added to the current scope. > Decl *Param = Actions.ActOnParamDeclarator(getCurScope(), > @@ -5492,12 +5501,34 @@ void Parser::ParseParameterDeclarationCl > Param, DefArgToks)); > } > > - if (TryConsumeToken(tok::ellipsis, EllipsisLoc) && > - !getLangOpts().CPlusPlus) { > - // We have ellipsis without a preceding ',', which is ill-formed > - // in C. Complain and provide the fix. > - Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis) > + if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) { > + if (!getLangOpts().CPlusPlus) { > + // We have ellipsis without a preceding ',', which is ill-formed > + // in C. Complain and provide the fix. > + Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis) > + << FixItHint::CreateInsertion(EllipsisLoc, ", "); > + } else if (ParmDeclarator.getEllipsisLoc().isValid() || > + > Actions.containsUnexpandedParameterPacks(ParmDeclarator)) { > + // It looks like this was supposed to be a parameter pack. Warn > and > + // point out where the ellipsis should have gone. > + SourceLocation ParmEllipsis = ParmDeclarator.getEllipsisLoc(); > + Diag(EllipsisLoc, diag::warn_misplaced_ellipsis_vararg) > + << ParmEllipsis.isValid() << ParmEllipsis; > + if (ParmEllipsis.isValid()) { > + Diag(ParmEllipsis, > + diag::note_misplaced_ellipsis_vararg_existing_ellipsis); > + } else { > + Diag(ParmDeclarator.getIdentifierLoc(), > + diag::note_misplaced_ellipsis_vararg_add_ellipsis) > + << > FixItHint::CreateInsertion(ParmDeclarator.getIdentifierLoc(), > + "...") > + << !ParmDeclarator.hasName(); > + } > + Diag(EllipsisLoc, diag::note_misplaced_ellipsis_vararg_add_comma) > << FixItHint::CreateInsertion(EllipsisLoc, ", "); > + } > + > + // We can't have any more parameters after an ellipsis. > break; > } > > > Modified: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp Mon Aug 11 18:30:23 2014 > @@ -197,6 +197,20 @@ namespace { > }; > } > > +/// \brief Determine whether it's possible for an unexpanded parameter > pack to > +/// be valid in this location. This only happens when we're in a > declaration > +/// that is nested within an expression that could be expanded, such as a > +/// lambda-expression within a function call. > +/// > +/// This is conservatively correct, but may claim that some unexpanded > packs are > +/// permitted when they are not. > +bool Sema::isUnexpandedParameterPackPermitted() { > + for (auto *SI : FunctionScopes) > + if (isa<sema::LambdaScopeInfo>(SI)) > + return true; > + return false; > +} > + > /// \brief Diagnose all of the unexpanded parameter packs in the given > /// vector. > bool > > Modified: cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p14.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p14.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p14.cpp (original) > +++ cfe/trunk/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p14.cpp Mon Aug 11 > 18:30:23 2014 > @@ -1,5 +1,4 @@ > // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s > -// expected-no-diagnostics > > template<typename T> struct identity; > template<typename ...Types> struct tuple; > @@ -22,7 +21,7 @@ template<typename T> struct is_same<T, T > template<typename T, typename ...Types> > struct X0 { > typedef identity<T(Types...)> function_pack_1; > - typedef identity<T(Types......)> variadic_function_pack_1; > + typedef identity<T(Types......)> variadic_function_pack_1; // > expected-warning {{varargs}} expected-note {{pack}} expected-note {{insert > ','}} > typedef identity<T(T...)> variadic_1; > typedef tuple<T(Types, ...)...> template_arg_expansion_1; > }; > > Modified: > cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/metafunctions.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/metafunctions.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/metafunctions.cpp > (original) > +++ cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/metafunctions.cpp Mon > Aug 11 18:30:23 2014 > @@ -243,7 +243,7 @@ namespace FunctionTypes { > }; > > template<typename R, typename ...Types> > - struct Arity<R(Types......)> { > + struct Arity<R(Types......)> { // expected-warning {{varargs}} > expected-note {{pack}} expected-note {{insert ','}} > static const unsigned value = sizeof...(Types); > }; > > > Modified: cfe/trunk/test/Parser/cxx-variadic-func.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-variadic-func.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/test/Parser/cxx-variadic-func.cpp (original) > +++ cfe/trunk/test/Parser/cxx-variadic-func.cpp Mon Aug 11 18:30:23 2014 > @@ -1,5 +1,8 @@ > -// RUN: %clang_cc1 -fsyntax-only %s > +// RUN: %clang_cc1 -fsyntax-only -verify %s > > void f(...) { > - int g(int(...)); > + // FIXME: There's no disambiguation here; this is unambiguous. > + int g(int(...)); // expected-warning {{disambiguated}} expected-note > {{paren}} > } > + > +void h(int n..., int m); // expected-error {{expected ')'}} expected-note > {{to match}} > > Modified: cfe/trunk/test/Parser/cxx11-templates.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx11-templates.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/test/Parser/cxx11-templates.cpp (original) > +++ cfe/trunk/test/Parser/cxx11-templates.cpp Mon Aug 11 18:30:23 2014 > @@ -7,3 +7,40 @@ struct S { > > template <typename Ty = char> > static_assert(sizeof(Ty) != 1, "Not a char"); // expected-error {{a > static_assert declaration cannot be a template}} > + > +namespace Ellipsis { > + template<typename ...T> void f(T t..., int n); // expected-error {{must > immediately precede declared identifier}} > + template<typename ...T> void f(int n, T t...); // expected-error {{must > immediately precede declared identifier}} > + template<typename ...T> void f(int n, T t, ...); // expected-error > {{unexpanded parameter pack}} > + template<typename ...T> void f() { > + f([]{ > + void g(T > + t // expected-note {{place '...' immediately before declared > identifier to declare a function parameter pack}} > + ... // expected-warning {{'...' in this location creates a > C-style varargs function, not a function parameter pack}} > + // expected-note@-1 {{insert ',' before '...' to silence > this warning}} > + ); > + void h(T (& > + ) // expected-note {{place '...' here to declare a function > parameter pack}} > + ... // expected-warning {{'...' in this location creates a > C-style varargs function, not a function parameter pack}} > + // expected-note@-1 {{insert ',' before '...' to silence > this warning}} > + ); > + void i(T (&), ...); > + }...); > + } > + template<typename ...T> struct S { > + void f(T t...); // expected-error {{must immediately precede declared > identifier}} > + void f(T ... // expected-note {{preceding '...' declares a function > parameter pack}} > + t...); // expected-warning-re {{'...' in this location creates > a C-style varargs function{{$}}}} > + // expected-note@-1 {{insert ',' before '...' to silence this > warning}} > + }; > + > + // FIXME: We should just issue a single error in this case pointing out > where > + // the '...' goes. It's tricky to recover correctly in this case, > though, > + // because the parameter is in scope in the default argument, so must be > + // passed to Sema before we reach the ellipsis. > + template<typename...T> void f(T n = 1 ...); > + // expected-warning@-1 {{creates a C-style varargs}} > + // expected-note@-2 {{place '...' immediately before declared > identifier}} > + // expected-note@-3 {{insert ','}} > + // expected-error@-4 {{unexpanded parameter pack}} > +} > > Modified: cfe/trunk/test/SemaCXX/issue547.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/issue547.cpp?rev=215408&r1=215407&r2=215408&view=diff > > ============================================================================== > --- cfe/trunk/test/SemaCXX/issue547.cpp (original) > +++ cfe/trunk/test/SemaCXX/issue547.cpp Mon Aug 11 18:30:23 2014 > @@ -27,32 +27,32 @@ struct classify_function<R(Args...) cons > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......)> { > +struct classify_function<R(Args..., ...)> { > static const unsigned value = 5; > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......) const> { > +struct classify_function<R(Args..., ...) const> { > static const unsigned value = 6; > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......) volatile> { > +struct classify_function<R(Args..., ...) volatile> { > static const unsigned value = 7; > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......) const volatile> { > +struct classify_function<R(Args..., ...) const volatile> { > static const unsigned value = 8; > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......) &&> { > +struct classify_function<R(Args..., ...) &&> { > static const unsigned value = 9; > }; > > template<typename R, typename ...Args> > -struct classify_function<R(Args......) const &> { > +struct classify_function<R(Args..., ...) const &> { > static const unsigned value = 10; > }; > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
