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

Bjorn Pettersson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 1 11:32:52 PDT 2021


bjope added inline comments.


================
Comment at: clang/lib/Lex/PPDirectives.cpp:594
+        if (CondInfo.FoundElse)
+          Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
 
----------------
aaron.ballman wrote:
> bjope wrote:
> > Hi @aaron.ballman 
> > 
> > This change is missing from https://reviews.llvm.org/rG8edd3464afbff65d7d5945b3a8b20009d6ff5deb , while you instead got it when doing the pp_err_else_after_else diagnostic a couple of lines above this.
> > 
> > It causes asserts (in DIagnostic::getArgKind) for me in test cases verifying the "elif after else" scenario using a test case basically doing
> > ```
> > #if 1
> > #else
> > #elif
> > #endif
> > ```
> > So maybe such a test case should be added as well?
> > 
> > But it seems like the patch you committed doesn't match with the reviewed patch. So maybe you can look into this and fix it?
> > (let me know if you need some additional help)
> Great catch and thank you for the test case! It seems that when I applied the patch, it misapplied, but not in a way that caused a test failure. I have no idea how that happened, but I assume I screwed up something with git somewhere along the way and caused the failure.
> 
> I've pushed baa2b8d08502acfa91a8dfd699d25f7b4e25edbb to fix it. Thanks!
Big thanks for the quick fix! While I managed to track down this problem I didn't have time to properly fix it right away. Now I'll have one thing less to worry about tomorrow :-)


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

https://reviews.llvm.org/D101192



More information about the cfe-commits mailing list