[Diagnostics] Diagnose -Wsizeof-array-div for array of pointers

[rsmith]

Thanks!

clang/lib/Sema/SemaExpr.cpp

10055–10056

You can simplify this a bit: RHSTy = RHSTy.getNonReferenceType();

I think this should also be done before line 10027 so that sizeof(p) / sizeof(decltype(*p)) produces a warning too. Perhaps remove the reference on line 10023 (since the expression case can never have reference type anyway):

-    RHSTy = RUE->getArgumentType();
+    RHSTy = RUE->getArgumentType().getNonReferenceType();

[xbolva00]

Addressed review comments.
New test cases.

[xbolva00]

clang/lib/Sema/SemaExpr.cpp

10055–10056

Thanks, applied.

[xbolva00]

This comment was removed by xbolva00.

[xbolva00]

Ping @rsmith

[xbolva00]

Ping @rsmith

[xbolva00]

I implemented suggested change so I assume it is fine. I will commit it tomorrow if no futher comments.

clang/lib/Sema/SemaExpr.cpp

if (LUE->getKind() != UETT_SizeOf || LUE->isArgumentType() ||
       RUE->getKind() != UETT_SizeOf)
     return;
 
   const Expr *LHSArg = LUE->getArgumentExpr()->IgnoreParens();
   QualType LHSTy = LHSArg->getType();
   QualType RHSTy;
 
   if (RUE->isArgumentType())
-    RHSTy = RUE->getArgumentType();
+    RHSTy = RUE->getArgumentType().getNonReferenceType();
   else
     RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
 
   if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
     if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), RHSTy))
       return;
 
     S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
     if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
       if (const ValueDecl *LHSArgDecl = DRE->getDecl())
         S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
             << LHSArgDecl;
     }
   } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
     QualType ArrayElemTy = ArrayTy->getElementType();
     if (ArrayElemTy != S.Context.getBaseElementType(ArrayTy) ||
         ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
         RHSTy->isReferenceType() || ArrayElemTy->isCharType() ||
         S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
       return;
     S.Diag(Loc, diag::warn_division_sizeof_array)
         << LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
     if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
       if (const ValueDecl *LHSArgDecl = DRE->getDecl())
         S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here)

clang/test/Sema/div-sizeof-array.cpp

void test(void) {
   int c3 = sizeof(arr5) / sizeof(int);
 
   float m[4][4];
   int d1 = sizeof(m) / sizeof(**m);
 
   int array[10];
   int narray = sizeof(array) / sizeof(int &);
   int narray2 = sizeof(array) / sizeof(decltype(array[0]));
+
+  int *arrptrs[10];                                          // expected-note {{array 'arrptrs' declared here}}
+  int len = sizeof(arrptrs) / sizeof(decltype(*arrptrs[0])); // expected-warning {{expression does not compute the number of elements in this array; element type is 'int *', not 'int'}}
+  // expected-note@-1 {{place parentheses around the 'sizeof(decltype(*arrptrs[0]))' expression to silence this warning}}
 }

clang/test/Sema/div-sizeof-ptr.cpp

 // RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
 
 template <typename Ty, int N>
 int f(Ty (&Array)[N]) {
   return sizeof(Array) / sizeof(Ty); // Should not warn
 }
 
 typedef int int32;
 
-void test(int *p, int **q) {          // expected-note 5 {{pointer 'p' declared here}}
+void test(int *p, int **q) {          // expected-note 6 {{pointer 'p' declared here}}
   const int *r;                       // expected-note {{pointer 'r' declared here}}
   int a1 = sizeof(p) / sizeof(*p);    // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
   int a2 = sizeof p / sizeof *p;      // expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}}
   int a3 = sizeof(p) / sizeof(int);   // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
   int a4 = sizeof(p) / sizeof(p[0]);  // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
   int a5 = sizeof(p) / sizeof(int32); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
   int a6 = sizeof(r) / sizeof(int);   // expected-warning {{'sizeof (r)' will return the size of the pointer, not the array itself}}
 
   int32 *d;                           // expected-note 2 {{pointer 'd' declared here}}
   int a7 = sizeof(d) / sizeof(int32); // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}}
   int a8 = sizeof(d) / sizeof(int);  // expected-warning {{'sizeof (d)' will return the size of the pointer, not the array itself}}
 
   int a9 = sizeof(*q) / sizeof(**q); // expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}}
+  int a10 = sizeof(p) / sizeof(decltype(*p)); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
 
   // Should not warn
   int b1 = sizeof(int *) / sizeof(int);
   int b2 = sizeof(p) / sizeof(p);
   int b3 = sizeof(*q) / sizeof(q);
   int b4 = sizeof(p) / sizeof(char);
 
   int arr[10];
   int c1 = sizeof(arr) / sizeof(*arr);
   int c2 = sizeof(arr) / sizeof(arr[0]);
   int c3 = sizeof(arr) / sizeof(int);
 
   int arr2[10][12];
   int d1 = sizeof(arr2) / sizeof(*arr2);
 }