[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 109883.
faisalv marked 7 inline comments as done.
faisalv added a comment.

Hi Richard,

  This patch attempts to incorporate all the changes you requested.  

You asked for some consideration on the rationale for sticking with my approach (as opposed to the alternative you posed), so here are my reasons (FWTW) for not modeling the VA_OPT as a macro argument:

- the standard does not specify it as behaving exactly as a macro argument (specifically no rescanning, after placemarker token removal) - and so i would have to special case the argument expansion logic - which currently composes well by just launching a tokenlexer on the argument.
- it would be the only argument that would need to have *other* arguments substituted into it
- unlike every other argument it would not be represented by just 1 token
- in regards to the source-location mapping, my sense is that the ExpansionInfo that represents the various expansions might need to add a representation for this 'fourth' kind of expansion (since expanded args are represented by a valid LocStart but an invalid LocEnd, and since this is more than one token, things could get hairy)
- it might require complicating 'Parsing' the macro definition (and the MacroArgs, MacroInfo objects) and the general tokenLexer, ExpandFunctionArguments() logic more than any gained conceptual elegance (in my opinion)

Thanks Richard - Look forward to your review and thoughts!


Repository:
  rL LLVM

https://reviews.llvm.org/D35782

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/TokenLexer.h
  include/clang/Lex/VariadicMacroSupport.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_vaopt_check.cpp
  test/Preprocessor/macro_vaopt_expand.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35782.109883.patch
Type: text/x-patch
Size: 37628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170805/51472529/attachment-0001.bin>