[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D132952#3759398 <https://reviews.llvm.org/D132952#3759398>, @efriedma wrote:

> In D132952#3759226 <https://reviews.llvm.org/D132952#3759226>, @aaron.ballman wrote:
>
>>> We could theoretically mess with the AST representation somehow, but I don't see any compelling reason to.  We probably just want to emit the warn_vla_used diagnostic somewhere else.
>>
>> Yeah, re-reading the C standard, I think I got VM types slightly wrong. Consider:
>>
>>   void func(int n, int array[n]);
>>
>> `array` is a VLA (C2x 6.7.6.2p4) before pointer adjustment (C2x 6.7.6.3p6), which makes `func` a VM type (C2x 6.7.6p3), not `array`.
>
> The type of parameter array after promotion (which is the type that matters for almost any purpose), is `int*`.  That isn't variably modified.  And the function type is just `void(int,int*)`; there's no way we can possibly treat that as variably modified.

On the one hand, yes, that makes sense to me. On the other hand, 6.7.6p3 says:

A full declarator is a declarator that is not part of another declarator. If, in the nested sequence of
declarators in a full declarator, there is a declarator specifying a variable length array type, the type
specified by the full declarator is said to be variably modified. Furthermore, any type derived by
declarator type derivation from a variably modified type is itself variably modified.

`array` is not a full declarator in my example, `func` is, but the standard waves its hands around a lot regarding when the adjustments to function parameters happen. Notionally, I think they only impact the definition of the function body. So I think the function declaration type is `void(int, int[])` and I think the decayed function type on function call is `void (*)(int, int *)`, but I'm not 100% sure. But that's why I was thinking `func` is a VM type.

> If you wrote, say `void func(int n, int array[n][n])`, then array is variably modified.  "func" is still not variably modified, I think?  "array" is not part of the "nested sequence of declarators", despite the fact that the parameter list contains a variably modified type.  At least, that's clang's current interpretation of the rules.

Once upon a time I had convinced myself that Clang got our interpretation wrong here, and I was hoping I'd rediscover exactly why I thought that as I worked my way through the C DRs and C feature statuses by adding actual test coverage. A lot of Clang's C functionality is less rigorously tested, especially the older stuff, so it might help to start pulling together more rigorous testing in this area. That would also help us to identify situations where we're not quite certain what the standard says, and I can go back to WG14 with a list of questions.

At the very least, I don't read the C grammar as supporting that interpretation of the rules. `declarator` reaches `direct-declarator` which reaches `function-declarator` which reaches `parameter-type-list` which reaches `parameter-list` then `parameter-declaration`, which finally gets us back to `declarator` for us to start parsing `array` which ends up being a VLA. So `func` is the full declarator in that case and thus is VM, at least as I read the spec.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132952/new/

https://reviews.llvm.org/D132952