[PATCH] D139436: [C2x] Relaxing requirements for va_start

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:7356
+        // in C before C2x. Complain and provide the fix. We do not support
+        // this as an extension in earlier C language modes.
+        if (getLangOpts().C2x)
----------------
Is this right?  Isnt this also allowing:

    foo(int a ...) 

I don't think we want to do that, right?  GCC diagnoses that as "requires a named argument", not a comma.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7227
+  bool SecondArgIsElided = false;
+  if (Optional<llvm::APSInt> Val =
+          TheCall->getArg(1)->getIntegerConstantExpr(Context);
----------------
This doesn't exactly match the comment, this isn't checking against the literal-0, it is checking against an ICE.

An updated comment would probably be fine here, and perhaps preferential, in order to allow a paren-expr containing 0.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7230
+      Val)
+    SecondArgIsElided = LangOpts.C2x && *Val == 0;
 
----------------
is there value to not jsut doing an early return here?  There is a lot of work happening below based on the last arg that we can just skip instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139436/new/

https://reviews.llvm.org/D139436