mikerice added a comment.
In D143410#4107862 <https://reviews.llvm.org/D143410#4107862>, @DHowett-MSFT
wrote:
> While this fixes the assertion failure and the immediate issue of whether
> packing _works_ inside delay-parsed templates in a PCH, it does reveal a
> follow-on issue that I can't quite trace out.
>
> c++
> template <typename T>
> void foo() {
> #pragma pack(push, 1)
> #pragma pack(show)
> #pragma pack(pop)
> }
>
> results in...
>
> unterminated '#pragma pack (push, ...)' at end of file
Tried your patch, but am not seeing this. Can you share the exact test case
where you see this?
Also please add a -triple to your test so it is stable across platforms (maybe
-triple x86_64-windows-msvc)
================
Comment at: clang/test/PCH/delayed-template-with-pragma-pack.cpp:14
+int func() {
+#pragma pack(push, 1)
+ struct s { short a; T b; };
----------------
DHowett-MSFT wrote:
> mikerice wrote:
> > Your test should include testing of the slot string too, so we are sure all
> > fields are serialized and restored correctly. Otherwise this looks
> > reasonable to me.
> So, I've identified an additional wrinkle...
>
> Unlike `#pragma omp`, `#pragma pack` and `... align` don't show up in the AST
> dump/reprint and I cannot verify their continued existence; this is true
> _even without delayed template expansion or PCH generation._
>
> alignment/packing seem to not be stored as Attrs, so they don't get printed
> via attr print on the AST nodes or (prettily) via the tablegen-generated
> pretty printers.
>
> Fixing this is somewhat beyond me at the moment. :)
> So, I've identified an additional wrinkle...
>
> Unlike `#pragma omp`, `#pragma pack` and `... align` don't show up in the AST
> dump/reprint and I cannot verify their continued existence; this is true
> _even without delayed template expansion or PCH generation._
>
> alignment/packing seem to not be stored as Attrs, so they don't get printed
> via attr print on the AST nodes or (prettily) via the tablegen-generated
> pretty printers.
True, so you have to verify it with the generated code instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143410/new/
https://reviews.llvm.org/D143410
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
