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

2023-02-07 Thread Mike Rice via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd27fb5efc5f2: [Serialization] Add support for (de)serializing #pragma pack (authored by DHowett-MSFT, committed by mikerice). Changed prior to commi

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

2023-02-07 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT added a comment. Thanks for the review! I’ll need somebody to help land this, as I don’t have write access to the project. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143410/new/ https://reviews.llvm.org/D143410 ___

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

2023-02-07 Thread Mike Rice via Phabricator via cfe-commits
mikerice accepted this revision. mikerice added a comment. This revision is now accepted and ready to land. LGTM. Thanks for taking care of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143410/new/ https://reviews.llvm.org/D143410 __

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT added a comment. Test failures seem related to recent driver changes. There's a commit 6a8a423c1864ced060a7041bf6ada7574f35ad4d that purports to fix them. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT updated this revision to Diff 495314. DHowett-MSFT added a comment. - Use .copy() like the other nearby code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143410/new/ https://reviews.llvm.org/D143410 Files: clang/include/clang/Sema/

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT marked 2 inline comments as done. DHowett-MSFT added a comment. The new test checks how tightly things were packed across a stack of packs, pushed and popped. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143410/new/ https://review

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT updated this revision to Diff 495285. DHowett-MSFT added a comment. @mikerice Alright, it turns out that PragmaPackAlignStorage isn't suitable for this (actually, I'm not sure what it _is_ suitable for) because it's deleted by the time the template gets parsed again. The `push, slot

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

2023-02-06 Thread Mike Rice via Phabricator via cfe-commits
mikerice added a comment. In 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 qu

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT added a comment. 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 void foo() { #pragma pack(push, 1) #pragma pack(

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

2023-02-06 Thread Mike Rice via Phabricator via cfe-commits
mikerice added inline comments. Comment at: clang/test/PCH/delayed-template-with-pragma-pack.cpp:14 +int func() { +#pragma pack(push, 1) + struct s { short a; T b; }; Your test should include testing of the slot string too, so we are sure all fields are seriali

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

2023-02-06 Thread Dustin L. Howett via Phabricator via cfe-commits
DHowett-MSFT created this revision. Herald added a project: All. DHowett-MSFT updated this revision to Diff 495195. DHowett-MSFT added a comment. DHowett-MSFT published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Add a test DHowett-MSFT adde