[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Haven't reviewed the code in detail, but the idea seems sound. Is there any prior art in other compilers you can draw data/experiences from? & having some data from some reasonably sized codebases on false positive (how often the correct thing is to suppress the warning, versus fixing the warning) would probably be useful. I would guess it doesn't meet the on-by-default bar we would prefer for Clang, but might still be OK.



================
Comment at: clang/docs/ReleaseNotes.rst:64-74
+  - ``-Wimplicit-widening-of-pointer-offset``:
+
+    .. code-block:: c++
+
+      char* ptr_add(char *base, int a, int b) {
+        return base + a * b; // expected-warning {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+      }
----------------
Does this only trigger when the `sizeof(char*) > sizeof(int)`? (judging by the test coverage I think that's the case)

(ultimately, might be worth committing the two diagnostics separately - usual sort of reasons, separation of concerns, etc)


================
Comment at: clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24
+void t1(char *base, int a, int b) {
+  // FIXME: test `[a * b]base` pattern?
+}
----------------
Question is unclear - is there uncertainty about whether this should be tested? Or is this a false negative case? In which case I'd probably include the test showing no diagnostic and mention it could be fixed/improved?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822