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

David Majnemer david.majnemer at gmail.com
Tue Feb 11 16:45:52 PST 2014



================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2729
@@ +2728,3 @@
+  // vftables.
+  if (RD->getMSVtorDispMode() == 2) {
+    for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
----------------
Would it be better to use a symbolic constant instead of magic numbers?

================
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>;
----------------
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



================
Comment at: lib/Parse/ParsePragma.cpp:192
@@ -191,1 +191,3 @@
 
+void Parser::HandlePragmaMSVtorDisp() {
+  assert(Tok.is(tok::annot_pragma_ms_vtordisp));
----------------
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.

================
Comment at: lib/Driver/Tools.cpp:4051
@@ -4050,1 +4050,3 @@
 
+  if (Arg *A = Args.getLastArg(options::OPT_vtordisp_mode_EQ))
+    A->render(Args, CmdArgs);
----------------
I didn't think aliases needed special handling.


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



More information about the cfe-commits mailing list