[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425
   Finder->addMatcher(
-      ifStmt(isExpansionInMainFile(),
              hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId)))
----------------
njames93 wrote:
> aaron.ballman wrote:
> > This is changing the behavior so that now it will diagnose in header files, no? Why is the correct change to replace this with `unless(isInTemplateInstantiation())` instead of adding the new matcher?
> It's changing behaviour that arguably shouldn't have been in the first place. But perhaps that change should go on a new patch or update the description of this one
I'll admit that the original code seems a bit suspect to me. I sort of wonder if it was being used to suppress diagnosing macros unless they're considered to be under the user's control. e.g., macros in headers may not be plausible to change but macros in source files are.

If changes should be made here, I don't have strong opinions on whether it requires a separate patch or can be done in this one, but I'd like to better understand why the original code was incorrect (if it is in fact incorrect).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81336