[PATCH] D133108: [clang] Rework IsTailPaddedMemberArray into isFlexibleArrayMemberExpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Found a few more things while trying to convince myself this really is NFC, and I don't think it is. If you agree, then I think we should have some additional test coverage to show the behavioral changes.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:15977
 
+  const unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays;
+
----------------
Missed this before, but drop the top-level `const` or remove the local and just use the call directly in the one place the value is needed.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:15989
+      BaseType == nullptr ||
+      isFlexibleArrayMemberExpr(*this, BaseExpr, ND, StrictFlexArraysLevel);
   if (EffectiveType->isDependentType() ||
----------------
This seems like a functional change....


================
Comment at: clang/lib/Sema/SemaChecking.cpp:15990-15991
+      isFlexibleArrayMemberExpr(*this, BaseExpr, ND, StrictFlexArraysLevel);
   if (EffectiveType->isDependentType() ||
-      (!IsUnboundedArray && BaseType->isDependentType()))
+      !IsUnboundedArray && BaseType->isDependentType())
     return;
----------------
I think the parens helped to add clarity here (also, there's probably a warning generated about mixed || and && from this too, isn't there?)


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16004
 
-  const NamedDecl *ND = nullptr;
-  if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
-    ND = DRE->getDecl();
-  if (const MemberExpr *ME = dyn_cast<MemberExpr>(BaseExpr))
-    ND = ME->getMemberDecl();
-
   if (IsUnboundedArray) {
     if (EffectiveType->isFunctionType())
----------------
...because now we can get into this block...


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16048
       // dependent CharUnits)
       DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr,
                           PDiag(DiagID)
----------------
... which means we can now hit this diagnostic.


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

https://reviews.llvm.org/D133108