r215408 - Reject varargs '...' in function prototype if there are more parameters after
Richard Smith
richard at metafoo.co.uk
Fri Aug 15 15:56:45 PDT 2014
I think Larisse is already investigating.
On Fri, Aug 15, 2014 at 3:43 PM, Alexander Kornienko <alexfh at google.com>
wrote:
> This patch causes http://llvm.org/PR20660. Can you take a look?
>
>
> On Tue, Aug 12, 2014 at 1:30 AM, Richard Smith <richard-llvm at metafoo.co.uk
> > 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 at -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 at -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 at -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 at -1 {{creates a C-style varargs}}
>> + // expected-note at -2 {{place '...' immediately before declared
>> identifier}}
>> + // expected-note at -3 {{insert ','}}
>> + // expected-error at -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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140815/fa402fa1/attachment.html>
More information about the cfe-commits
mailing list