[PATCH] D1623: Support __builtin_ms_va_list.

[Charles Davis via cfe-commits]

cdavis5x added inline comments.

================
Comment at: include/clang/AST/Expr.h:3709
@@ -3708,3 +3708,3 @@
   VAArgExpr(SourceLocation BLoc, Expr* e, TypeSourceInfo *TInfo,
-            SourceLocation RPLoc, QualType t)
+            SourceLocation RPLoc, QualType t, bool IsMS = false)
     : Expr(VAArgExprClass, t, VK_RValue, OK_Ordinary,
----------------
rsmith wrote:
> Is there a reason to add a default argument for this? It looks like all calls provide an argument.
Nope. The default arg is gone now.

================
Comment at: include/clang/AST/Expr.h:3720-3722
@@ -3719,4 +3719,5 @@
 
-  /// \brief Create an empty __builtin_va_arg expression.
-  explicit VAArgExpr(EmptyShell Empty) : Expr(VAArgExprClass, Empty) { }
+  /// Create an empty __builtin_va_arg expression.
+  explicit VAArgExpr(EmptyShell Empty, bool IsMS = false)
+    : Expr(VAArgExprClass, Empty), Val(0), TInfo(0, IsMS) { }
 
----------------
rsmith wrote:
> The `IsMS` flag you added here is never used.
Removed. (I don't know what I was thinking with that...)

================
Comment at: lib/Sema/SemaChecking.cpp:1038-1040
@@ -1037,1 +1037,5 @@
+  case X86::BI__builtin_ms_va_start:
+    if (Context.getTargetInfo().getTriple().getArch() != llvm::Triple::x86_64)
+      return false;
+    return SemaBuiltinVAStart(TheCall);
   case X86::BI_mm_prefetch: i = 1; l = 0; u = 3; break;
----------------
rsmith wrote:
> Can we do better than this? I think it'd be better to check that `__builtin_ms_va_start` only occurs in a function using the MS ABI's variadic calling convention, and `__builtin_va_start` only occurs in a function using the normal variadic C calling convention.
Done.


http://reviews.llvm.org/D1623