[PATCH] MS ABI: Implement #pragma vtordisp() and clang-cl /vdN

David Majnemer david.majnemer at gmail.com
Wed Feb 12 12:54:37 PST 2014



================
Comment at: include/clang/Driver/CLCompatOptions.td:121
@@ -120,1 +120,3 @@
+def _SLASH_vd : CLJoined<"vd">, HelpText<"Control vtordisp placement">,
+  Alias<vtordisp_mode_EQ>;
 def _SLASH_Z7 : CLFlag<"Z7">, Alias<gline_tables_only>;
----------------
Reid Kleckner wrote:
> David Majnemer wrote:
> > Can we have a test in ##test/Driver/cl-options.c## for checking that `-vd` works?
> > 
> > Also, my cl doesn't allow stuff like `cl -vd1 -vd2 q.cpp`. It errors with:
> > > '/vd1' and '/vd2' command-line options are incompatible
> > 
> > 
> We already have a test to make sure we accept /vdN without error.  I don't think it's worth a separate process invocation in every test suite run just to make sure that tablegen aliases work.
We still need a test for when `-vd1` is specified with `-vd2`.

================
Comment at: lib/Parse/ParsePragma.cpp:192
@@ -191,1 +191,3 @@
 
+void Parser::HandlePragmaMSVtorDisp() {
+  assert(Tok.is(tok::annot_pragma_ms_vtordisp));
----------------
Reid Kleckner wrote:
> David Majnemer wrote:
> > This seems a bit unusual compared to the other `#pragma` implementations.  Looking at the very similar `Parser::HandlePragmaPack`, it consumes a single token which contains a pointer to useful data. A similar approach should make the implementation of `Parser::HandlePragmaMSVtorDisp` simpler.
> HandlePragmaPack is somewhat simple, but the other side has to use the PP allocator and placement new to push the magic token, which is gross.  Why not just push the integer or identifier token in question and let the parser consume tokens like it normally would?  Also, pragma pack doesn't do any error handling if the integer value is bogus, which is half of the complexity here.
> 
> Long term, I think the pragma handlers in parse should receive a reference to Sema or Parser so they don't need to inject tokens.  Then we can handle things like:
> 
>   struct A {
>     virtual void f(); 
>   #pragma vtordisp(0)
>      struct B : virtual A { virtual void f(); };
>   };
> 
> Today we error our because the annotations don't appear at the right places in our grammar.
I'm not worried about using the allocator for things like this, `#pragma vtordisp` is very rare.

I don't see a technical argument for duplicating the code to examine "on" and "off".


http://llvm-reviews.chandlerc.com/D2746



More information about the cfe-commits mailing list