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