rjmccall added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13066
+std::tuple<bool, CharUnits, CharUnits>
+static getBaseAlignmentAndOffsetFromLValue(const Expr *E, ASTContext &Ctx) {
+ E = E->IgnoreParens();
----------------
I think an `Optional<std::pair>` would be a more self-documenting type here,
and you'd be able to test the result in an `if`.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13082
+ std::get<2>(P) + EltSize *
IdxRes.getExtValue());
+ }
+ }
----------------
You still need to conservatively adjust the alignment when the index isn't a
constant expression. You can do that by reducing the base result to a simple
alignment, adding the array element size as an offset, and then reducing again.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13091
+ if (VD->getType()->isReferenceType())
+ return getBaseAlignmentAndOffsetFromLValue(VD->getInit(), Ctx);
+ return std::make_tuple(true, Ctx.getDeclAlign(VD), CharUnits::Zero());
----------------
Reference variables don't have to have initializers; e.g. they can be `extern`.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13111
+ }
+ }
+ return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero());
----------------
Derived-to-base conversions.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13160
+ Opcode == BO_Add)
+ HandleBinOp(BO->getRHS(), BO->getLHS(), Opcode, P);
+ return P;
----------------
This all seems like it'd be simpler if you just checked for the pointer/integer
operands and then swapped if necessary.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13164
+ break;
+ }
+ }
----------------
Derived-to-base conversions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
