[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.


================
Comment at: clang/docs/ClangCommandLineReference.rst:2644
+
+Control which arrays are considered as flexible arrays members. <arg>
+can be 1 (array of size 0, 1 and undefined are considered), 2 (array of size 0
----------------
Docs should also mention what the default -fno-strict-flex-arrays means -- that ALL sizes of trailing arrays are considered flexible array members. (I'm amazed that's the rule, and I never knew it. I always thought the special casing for FAMs was restricted to sizes 0 and 1!)

Also, since apparently different parts of the compiler have been (and will now continue to) use different default behaviors, may want to document that as well. I'm sure I don't know what the rules actually are intended to be here. E.g. that a macro-expansion of the size arg disables the special-behavior for [1] is extremely surprising!


================
Comment at: clang/lib/Sema/SemaChecking.cpp:15804
+
+  // FIXME: we should also allow Size = 0 here per the definition of
+  // StrictFlexArraysLevel, but that's backward incompatible with previous clang
----------------
Presumably the size-zero/unsized cases are already being taken care of elsewhere in the code? I find it hard to believe we are currently emitting diagnostics for size-0 FAM which we don't emit for size-1 FAM?


================
Comment at: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp:9
+  } s2;
+  s2.a[2] = 0; // no-warning
+}
----------------
Except we actually _do_ know the bounds of the full-object and ought to be able to warn on this code anyhow...

Better to have the test function accept a pointer, so that's not a conflating issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864