chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13966
if (index.isUnsigned() || !index.isNegative()) {
- // It is possible that the type of the base expression after
- // IgnoreParenCasts is incomplete, even though the type of the base
- // expression before IgnoreParenCasts is complete (see PR39746 for an
- // example). In this case we have no information about whether the array
- // access exceeds the array bounds. However we can still diagnose an array
- // access which precedes the array bounds.
- if (BaseType->isIncompleteType())
- return;
+ if (isUnboundedArray) {
+ const auto &ASTC = getASTContext();
----------------
ebevhan wrote:
> chrish_ericsson_atx wrote:
> > ebevhan wrote:
> > > It might simplify the patch to move this condition out of the tree and
> > > just early return for the other case. That is:
> > >
> > > ```
> > > if (isUnboundedArray) {
> > > if (!(index.isUnsigned() || !index.isNegative()))
> > > return;
> > >
> > > ...
> > > return;
> > > }
> > >
> > > if (index.isUnsigned() ...
> > > ```
> > There's a bit more code (starting at line 14094 in this patch set) that
> > applies in all cases, so an early return here would prevent the "Array
> > declared here" note from being generated.
> Ah, the note.
>
> I wonder if it wouldn't be cleaner (and avoid indenting the entire block) if
> that was just duplicated.
Hard to say which is cleaner -- it's a tradeoff. Nesting level would be
shallower if that code was duplicated, but then again, the duplication
increases the chance of an incomplete fix should an issue be discovered in that
code. Overall code would be slightly longer, as well (adding about 16 lines of
code, but removing only 4). To me, the current strategy feels more surgical,
but I'll change it if you feel more strongly about it than I do. Please
re-ping if you do.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86796/new/
https://reviews.llvm.org/D86796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
