Szelethus added inline comments.
================ Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59 + StringRef SourceText = + Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); + std::string Condition = getCondition(SourceText); ---------------- vmiklos wrote: > Szelethus wrote: > > I'm a little confused. To me, it seems like you acquired the condition > > already -- doesn't `ConditionRange` actually cover the, well, condition > > range? This is how I imagined it: > > > > ``` > > #ifdef CUTE_PANDA_CUBS > > ^~~~~~~~~~~~~~~ > > ConditionRange > > ``` > > Why is there a need for `getCondition`? Is there any? If there is (maybe > > the acquired text contains other things), can you document it? I haven't > > played with `PPCallbacks` much, so I'm fine with being in the wrong. > ConditionRange covers more than what you expect: > > ``` > #if FOO == 4 > ^~~~~~~~~ > void f(); > ~~~~~~~~~ > #endif > ~~~~~~ > ``` > > to find out if the condition of the `#if` is the same as a previous one, I > want to extract just `FOO == 4` from that, then deal with that part similar > to `#ifdef` and `#ifndef`, which are easier as you have a single Token for > the condition. But you're right, I should add a comment explaining this. Oh my god. There is no tool or a convenient method for this??? I had the displeasure of working with the preprocessor in the last couple months, and I get shocked by things like this almost every day. Yea, unfortunately you will have to write excessive amount of comments to counterweights the shortcomings of `Preprocessor` :/ https://reviews.llvm.org/D54349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
