[PATCH] D101192: Add support for #elifdef and #elifndef

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 2 inline comments as done.
aaron.ballman added inline comments.


================
Comment at: clang/lib/Lex/PPDirectives.cpp:3254-3265
+  Token MacroNameTok;
+  ReadMacroName(MacroNameTok);
+
+  // Error reading macro name?  If so, diagnostic already issued.
+  if (MacroNameTok.is(tok::eod)) {
+    // Skip code until we get to #endif.  This helps with recovery by not
+    // emitting an error when the #endif is reached.
----------------
rsmith wrote:
> Hm, is the strict checking here appropriate? I'd expect skipping to start as soon as we hit the `#elifdef` directive, so the restriction on the form of the directive should be relaxed and we should just skip to the end of the line. ("When in a group that is skipped (15.2), the directive syntax is relaxed to allow any sequence of preprocessing tokens to occur between the directive name and the following new-line character."). For example, given:
> 
> ```
> #if 1
> #elif
> #endif
> ```
> 
> ... we don't diagnose, even though the `#elif` line doesn't match the usual form for the directive (a //constant-expression// would require at least one token between `elif` and //new-line//), and I'd expect `#elifdef` to behave the same way.
> 
> With this fixed, the handling of `#elif`, `#elifdef`, and `#elifndef` (when not skipping) should be identical other than which callback is invoked; can the implementations be combined?
Good point! I think this means we'd need a second set of callbacks though -- we want the macro information when we process a `#elifdef` or a `#elifndef` that is taken (same as with `#ifdef`), so we'd need the interface that takes a `MacroDefinition`, but if we skip reading that because we're skipping the entire conditional block, we'd need a callback that takes the condition range.

I went that route with the updated patch. LMK if that works for you.


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

https://reviews.llvm.org/D101192