I think Larisse is already investigating. On Fri, Aug 15, 2014 at 3:43 PM, Alexander Kornienko <[email protected]> wrote:
> 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
