[PATCH] D19361: [MS] Improved implementation of MS stack pragmas (vtordisp, *_seg)

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla created this revision.
d.zobnin.bugzilla added a reviewer: rnk.
d.zobnin.bugzilla added a subscriber: cfe-commits.

This patch attempts to improve implementation of several MS pragmas that use stack (vtordisp, { bss | code | const | data }_seg) and fix some compatibility issues.
Please take a look at the patch and let me know whether it's something worth working on and whether I'm on the right way of doing this.

This patch:
1. Changes implementation of #pragma vtordisp to use existing PragmaStack<> template class that *_seg pragmas use;
2. Fixes "#pragma vtordisp()" reset behavior -- it shouldn't affect the stack;
3. Supports "save-and-restore" of pragma state stacks on entering / exiting a C++ method body, as MSVC does.

If this work is accepted, I propose several to-do's:
1. Change #pragma pack implementation to use the same approach as other "stack" pragmas use;
2. Introduce diagnostics on popping named stack slots, as MSVC does

Currently I tried to make Clang behave exactly as MSVC behaves, though, I can think of possible better solutions, please see "Implementation notes" below.


Motivation:

Currently Clang implements "stack" pragmas in several ways: "vtordisp" and "pack" have their own stacks in Sema, pragma actions are performed differently.

MSVC seems to have a common implementation of all these pragmas. One common feature is "save-restore" on entering / exiting a C++ method body:
```
#pragma pack(4)
#pragma pack(show)

struct S {
  void f() {
    #pragma pack(push, 8)
    #pragma pack(show)
  }
};

#pragma pack(show)
```
MSVC's output: 4, 8, 4
Clang's output:  4, 8, 8

MSVC seems to introduce artificial sentinel slots on entering a C++ method body and pops them on exit. Consider
```
#pragma pack(push, 4)
struct S {
  void f() {
    #pragma pack(pop)
  }
};
```
MSVC emits:
```
warning C4159: #pragma vtordisp(pop,...): has popped previously pushed identifier '<InternalPragmaState>'
```
The same approach is used for "vtordisp" pragma, although it doesn't support #pragma vtordisp (pop, id) syntax.
Please note that in the above example the stack gets corrupted and might lead to unexpected behavior later.

I implemented "save-and-restore" feature for #pragma vtordisp in http://reviews.llvm.org/D14467 by copying a skack to a RAII object and back, no matter how many times "pop" is used. Pragmas other than "vtordisp" in Clang don't support this feature yet.


Implementation notes:

MSVC seems to use the same name "<InternalPragmaState>" even for methods of nested classes. It is legal in correct cases, because "#pragma <name>(pop, id)" will look for the closest slot with mathing name, but if the nested sentinel slot is mistakenly popped, it will cause popping the stack up to surrounding sentinel.
Maybe, if we choose this approach, we could use unique sentinel names for better recovery? Or even mark the sentinels to prevent a programmer from popping them by accident?

http://reviews.llvm.org/D19361

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParsePragma.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaAttr.cpp
  test/CodeGenCXX/sections.cpp
  test/SemaCXX/pragma-vtordisp.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D19361.54484.patch
Type: text/x-patch
Size: 15827 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160421/f9bb739f/attachment-0001.bin>