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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
