ahatanak added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+ if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
----------------
rjmccall wrote:
> Oh, this — and all the other places that do presumed alignment based on a
> pointee type — needs a special case for C++ records with virtual bases,
> because you need to get its presumed alignment as a base sub-object, not its
> presumed alignment as a complete object, which is what `getTypeAlignInChars`
> will return. The right way to merge that information is to get the normal
> alignment — which may be lower than expected if there's a typedef in play —
> and then `min` that with the base sub-object alignment.
I think the base sub-object alignment in this case is the `NonVirtualAlignment`
of the class (the `CXXRecordDecl` of `Base` in this function), but it's not
clear to me what the normal alignment is. I don't think it's what
`Ctx.getTypeAlignInChars(Base->getType())` returns, is it? I see
`CodeGenModule::getDynamicOffsetAlignment` seems to be doing what you are
suggesting, but I'm not sure whether that's what we should be doing here. It
looks like it's comparing the alignment of the derived class and the
non-virtual alignment of the base class.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:13266
+ if (!RHS->isIntegerConstantExpr(RHSRes, Ctx))
+ return Optional<std::pair<CharUnits, CharUnits>>();
+ CharUnits Offset = LHSRes->second;
----------------
rjmccall wrote:
> This should be handled the same as array subscripting. Maybe you can extract
> a helper for that that's given a pointer expression, an integer expression,
> and a flag indicating whether it's a subtraction?
The offset computation was wrong when the element type wasn't a char. The bug
is fixed in the helper function.
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
