[PATCH] D143410: [Serialization] Add support for (de)serializing #pragma pack

[Mike Rice via Phabricator via cfe-commits]

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