serge-sans-paille 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
----------------
jyknight wrote:
> 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!
it is worse than that: for some checks, any size is valid for FAM, but not for
alls. For some checks, macro expansion prohibits FAM, but not for all, etc,
etc. I don't want to document that behavior, because it is too specific to each
pass. My plan is
1. land -fstrict-flex-array support
2. launch a thread on the ugly situation we put ourselves in, and extract a
decision for each case
3. support and document an homogeneous behavior across passes.
4. syndicate code across passes
================
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
----------------
jyknight wrote:
> 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?
correct
================
Comment at: clang/lib/Sema/SemaChecking.cpp:15969
// access which precedes the array bounds.
if (BaseType->isIncompleteType())
return;
----------------
And here
================
Comment at: clang/lib/Sema/SemaChecking.cpp:15973
llvm::APInt size = ArrayTy->getSize();
if (!size.isStrictlyPositive())
return;
----------------
Handled here
================
Comment at: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp:9
+ } s2;
+ s2.a[2] = 0; // no-warning
+}
----------------
jyknight wrote:
> 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?
Correct
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126864/new/
https://reviews.llvm.org/D126864
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
