[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.


================
Comment at: clang/test/Analysis/initialization.cpp:132
+
+char const glob_arr6[5] = "123";
+void glob_array_index4() {
----------------
steakhal wrote:
> Ah, it's somewhat confusing.
> At first, when I looked at it, I assumed that this array has `6` elements as its name suggests.
> But it has actually 5 elements.
`6` in `glob_arr6` is like a serial number. Means that there are `glob_arr5` and `glob_arr4` and so for above.


================
Comment at: clang/test/Analysis/initialization.cpp:156-157
+void glob_invalid_index7() {
+  int idx = -42;
+  auto x = glob_arr6[idx]; // expected-warning{{garbage or undefined}}
+}
----------------
steakhal wrote:
> You could inline the `-42` without changing any expected behavior.
> It would make the test terser IMO. The same applies to the other case.
`glob_arr6[-42]` this provokes AST parser to emit a warning before reaching to CSA checks. Separate variable allows to avoid AST parser checks to let CSA be engaged.


================
Comment at: clang/test/Analysis/initialization.cpp:160-166
+// TODO: Support multidimensional array.
+void glob_invalid_index8() {
+  const char *ptr = glob_arr6;
+  int idx = 42;
+  // FIXME: Should warn {{garbage or undefined}}.
+  auto x = ptr[idx]; // no-warning
+}
----------------
steakhal wrote:
> I'm not sure if I follow. The `TODO` implies to me that this case is about //multidimensional  array//s, but it's actually not.
> `glob_arr6` is of type `const char[5]`
> Could you clarify this?
> BTW, at first glance, the gist of this case is the same as the `glob_invalid_index7`.
> Why does this behave differently? I'm puzzled.
Correct. Thanks! The consequence of copy-paste. I'll fix.


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

https://reviews.llvm.org/D107339