aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:3220
+
+ // `type[]` is equivalent to `type *` and `type[*]`
+ if (NoSizeInfo(Old) && NoSizeInfo(New))
----------------
================
Comment at: clang/lib/Sema/SemaDecl.cpp:3224
+
+ // Don't try to compare VLA sizes, unless one of them has the star modifier
+ if (Old->isVariableArrayType() && New->isVariableArrayType()) {
----------------
================
Comment at: clang/lib/Sema/SemaDecl.cpp:3234
+
+ // Only compare size, ignore Size modifiers and CVR
+ if (Old->isConstantArrayType() && New->isConstantArrayType())
----------------
================
Comment at: clang/lib/Sema/SemaDecl.cpp:3274-3278
+ S.Diag(NewParam->getLocation(), diag::warn_inconsistent_array_form)
+ << NewParam->getName()
+ << NewParamOT->getCanonicalTypeInternal().getAsString();
+ S.Diag(OldParam->getLocation(), diag::note_previous_declaration_as)
+ << OldParamOT->getCanonicalTypeInternal().getAsString();
----------------
The diagnostics engine knows how to format `NamedDecl` objects as well as
types. Can you do this instead and still get reasonable diagnostics?
================
Comment at: clang/test/Sema/array-parameter.c:2
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Warray-parameter
-verify %s
+
+void f0(int a[]);
----------------
aaron.ballman wrote:
> I'd like to see some additional test cases:
> ```
> void func(int *p);
> void func(int a[2]); // Diagnose this one, I presume?
> void func(int a[]); // But also diagnose this one as well, yes?
> void func(int a[2]) {} // Do we then diagnose this as well, or is this
> matching a previous declaration and thus fine?
>
> void other(int n, int m, int a[n]);
> void other(int n, int m, int a[m]); // Hopefully we diagnose this set!
>
> void another(int n, int array[n]);
> void another(int n, int array[*]); // I *think* this should be warned, but
> I'm still a bit on the fence about it
> ```
>
> Also, if this is expected to work in C++ as well, we should probably have
> cases like:
> ```
> template <int N>
> void func(int i[10]);
>
> template <int N>
> void func(int i[N]); // Should we diagnose this before instantiation or wait
> until we see the instantiation and only diagnose if differs?
>
> static constexpr int Extent = 10;
> void other(int i[10]);
> void other(int i[Extent]); // Should not be diagnosed
> ```
Do you plan to add the C++ tests?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128449/new/
https://reviews.llvm.org/D128449
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
