jyknight added inline comments.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:892
+ return false;
if (CAT->getSize().ugt(1))
return false;
----------------
Similar to SemaChecking below, could use a comment like:
FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing arrays
to be treated as flexible-array-members, we still emit ubsan checks as if they
are not. Pending further discussion...
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:906
// member, only a T[0] or T[] member gets that treatment.
+ // Under StrictFlexArraysLevel, obey c99+ that disallows FAM in union,
see
+ // C11 6.7.2.1 ยง18
----------------
I believe this bit is incorrect -- it should just go back to 'return true;'.
The StrictFlexArraysLevel check above already eliminates the cases we want to
eliminate (size==1 in strictness-level 2.)
================
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
----------------
serge-sans-paille wrote:
> 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
The FIXME comment isn't correct, since only nonzero sizes ever reach this
function. The code can be simplified too. Also -- there is another FIXME that
should be here, regarding the behavior of size>1 FAMs.
I suggest (with rewrapped comment of course):
```
if (!ND) return false;
// FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
arrays to be treated as flexible-array-members, we still emit diagnostics as if
they are not. Pending further discussion...
if (StrictFlexArraysLevel >= 2 || Size != 1) return false;`
```
================
Comment at: clang/lib/Sema/SemaChecking.cpp:15969
// access which precedes the array bounds.
if (BaseType->isIncompleteType())
return;
----------------
serge-sans-paille wrote:
> And here
Looks like actually the `int x[]` case is handled with the large
"IsUnboundedArray" condition above not here...
And, actually, all of that code to generate warnings for larger-than-addrspace
offsets OUGHT to be getting used for `int x[0]` and `int x[1]` flexible arrays,
too. Needs another FIXME for that...
================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:792
+ if (getContext().getLangOpts().StrictFlexArrays >= 2)
+ return false;
----------------
Yuk, another place that is weird and doesn't handle StrictFlexArrays as
expected...
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
