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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith 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.
----------------
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?


================
Comment at: clang/lib/Lex/PPDirectives.cpp:3291
+  if (MI) // Mark it used.
+    markMacroAsUsed(MI);
+
----------------
This doesn't seem right to me: the macro's existence or non-existence does not affect preprocessing, so the macro was not used. But I assume this and the references to `MD` below will also be removed once this function stops parsing a macro name.


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

https://reviews.llvm.org/D101192