r234363 - [Sema] Correctly recurse when looking for [*] in function definitions

[Richard Smith]

On Tue, Apr 7, 2015 at 3:08 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> Author: majnemer
> Date: Tue Apr  7 17:08:51 2015
> New Revision: 234363
>
> URL: http://llvm.org/viewvc/llvm-project?rev=234363&view=rev
> Log:
> [Sema] Correctly recurse when looking for [*] in function definitions
>
> A [*] is only allowed in a declaration for a function, not in its
> definition.  We didn't correctly recurse while looking for it, causing
> us to crash in CodeGen instead of rejecting it.
>
> This fixes PR23151.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaChecking.cpp
>     cfe/trunk/test/Sema/vla.c
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=234363&r1=234362&r2=234363&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Apr  7 17:08:51 2015
> @@ -7705,6 +7705,31 @@ void Sema::CheckBitFieldInitialization(S
>    (void) AnalyzeBitFieldAssignment(*this, BitField, Init, InitLoc);
>  }
>
> +static void diagnoseArrayStarInParamType(Sema &S, QualType PType,
> +                                         SourceLocation Loc) {
> +  if (!PType->isVariablyModifiedType())
> +    return;
> +  if (const auto *PointerTy = dyn_cast<PointerType>(PType)) {
> +    diagnoseArrayStarInParamType(S, PointerTy->getPointeeType(), Loc);
> +    return;
> +  }
> +  if (const auto *ParenTy = dyn_cast<ParenType>(PType)) {
> +    diagnoseArrayStarInParamType(S, ParenTy->getInnerType(), Loc);
> +    return;
> +  }
> +
> +  const ArrayType *AT = S.Context.getAsArrayType(PType);
> +  if (!AT)
> +    return;
> +
> +  if (AT->getSizeModifier() != ArrayType::Star) {
> +    diagnoseArrayStarInParamType(S, AT->getElementType(), Loc);
> +    return;
> +  }
> +
> +  S.Diag(Loc, diag::err_array_star_in_function_definition);
>

Looks like this won't handle DecayedType (for a decayed function
parameter), ReferenceType, BlockPointerType, and a few others... but
RecursiveASTVisitor seems like overkill. Perhaps we could track the
locations of [*] bounds on the Scope for the function declaration instead
of digging them out here?


> +}
> +
>  /// CheckParmsForFunctionDef - Check that the parameters of the given
>  /// function are appropriate for the definition of a function. This
>  /// takes care of any checks that cannot be performed on the
> @@ -7743,15 +7768,9 @@ bool Sema::CheckParmsForFunctionDef(Parm
>      //   notation in their sequences of declarator specifiers to specify
>      //   variable length array types.
>      QualType PType = Param->getOriginalType();
> -    while (const ArrayType *AT = Context.getAsArrayType(PType)) {
> -      if (AT->getSizeModifier() == ArrayType::Star) {
> -        // FIXME: This diagnostic should point the '[*]' if
> source-location
> -        // information is added for it.
> -        Diag(Param->getLocation(),
> diag::err_array_star_in_function_definition);
> -        break;
> -      }
> -      PType= AT->getElementType();
> -    }
> +    // FIXME: This diagnostic should point the '[*]' if source-location
> +    // information is added for it.
> +    diagnoseArrayStarInParamType(*this, PType, Param->getLocation());
>
>      // MSVC destroys objects passed by value in the callee.  Therefore a
>      // function definition which takes such a parameter must be able to
> call the
>
> Modified: cfe/trunk/test/Sema/vla.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vla.c?rev=234363&r1=234362&r2=234363&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/vla.c (original)
> +++ cfe/trunk/test/Sema/vla.c Tue Apr  7 17:08:51 2015
> @@ -61,6 +61,9 @@ void pr5185(int a[*]) // expected-error
>  {
>  }
>
> +void pr23151(int (*p1)[*]) // expected-error {{variable length array must
> be bound in function definition}}
> +{}
> +
>  // Make sure this isn't treated as an error
>  int TransformBug(int a) {
>   return sizeof(*(int(*)[({ goto v; v: a;})]) 0); // expected-warning
> {{use of GNU statement expression extension}}
>
>
> _______________________________________________
> 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/20150407/3ced3127/attachment.html>