shafik added a comment.
In D83008#2136303 <https://reviews.llvm.org/D83008#2136303>, @teemperor wrote:
> I think we are talking about different things. My question was why is this a
> 'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and
> not a test in the `API` directory (using Python). An 'API' test could use the
> proper expression testing tools. And it could actually run when doing
> on-device testing (which is to my knowledge not supported for Shell tests)
> which seems important for a test concerning a bug that only triggers when
> doing on-device testing (beside that one ubuntu ARM bot).
I did not realize the shell tests were not tested on device.
> Also when looking over the ARM-specific test, I think there might be two bugs
> that were involved in triggering it. One is the bug fixed here which triggers
> that Clang will produce its own layout for those classes. Now I also wonder
> why the layout the expression parser Clang generates doesn't match the one
> from the test (which appears to be a second bug). The ARM-specific test
> doesn't have any information in its AST that isn't also available in the
> expression AST, so why would they produce different layouts? Not sure what
> exactly is behind the "differences in how it deals with tail padding"
> description but I assume this is related to tail padding reuse? If yes, then
> maybe the second bug is that the records in our AST are (not) PODs in the
> expression AST (see the inline comment for where the tail padding is
> enabled/disabling based on whether the RD is POD).
So we are hitting this code for non-arm64 case:
case TargetCXXABI::UseTailPaddingUnlessPOD03:
//...
return RD->isPOD();
and we return `false` which is correct for the original test case since it has
a base class and base therefore not an aggregate (until C++17) see it here
<https://github.com/llvm/llvm-project/blob/825e3bb58082eafa8db87a9034379b88f892ce9d/clang/lib/AST/DeclCXX.cpp#L206>
and struct that had the difference I was looking at. I did not check for the
test cases in the PR though.
In the arm64 case from the original problem we hit the following:
case TargetCXXABI::UseTailPaddingUnlessPOD11:
// ...
return RD->isTrivial() && RD->isCXX11StandardLayout();
which returns `true` for the original case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83008/new/
https://reviews.llvm.org/D83008
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
