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

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 24 11:24:59 PDT 2021


dexonsmith added a comment.

I reviewed the source minimizer sections, and the code looks correct.

It'd probably be good to have unit tests in clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp for the incremental changes.

- Do the new constructs stick around in the minimized source?
- Do they get paired correctly for the ppranges stuff? (I think you can modify or create new versions of SkippedPPRangesBasic and SkippedPPRangesNested.)



================
Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:909
       If,  // if/ifdef/ifndef
       Else // elif,else
     };
----------------
Please add the two new cases to this comment as well.


================
Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:924
     case pp_elif:
+    case pp_elifdef:
+    case pp_elifndef:
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > Hrmph, not sure I understand this part either.
> I'm not 100% certain myself.
This part LGTM.


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

https://reviews.llvm.org/D101192



More information about the cfe-commits mailing list