[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx marked an inline comment as done.
chrish_ericsson_atx added a comment.

I will tinker with the math to simplify as you suggest.  Working with APInt and APSInt seems to promulgate sensitive and brittle code, which makes trying alternative expressions more tedious than I'd like (which is why I bailed on an earlier attempt to simplify this).  However, that same observation about brittle code supports the goal that simpler math would be safer, as there would presumably be fewer opportunities for AP/APSInt to misbehave as they interact.



================
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796