[PATCH] D16700: [Clang-tidy] Make null pointer literals for fixes configurable for two checks

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.

================
Comment at: clang-tidy/readability/ImplicitBoolCastCheck.h:32
@@ -30,1 +31,3 @@
+            Options.get("NullPointerLiteral",
+            Context->getLangOpts().CPlusPlus11 ? "nullptr" : "0")) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > I know you are following the pattern used in the code here, but I think this class needs a storeOptions() function as well. See AssertSideEffectCheck as an example.
> This will need rebasing on the existing code, which is using "NULL" as the pre-C++11 fallback default, not "0".
This was in original code. I just didn't want to change default.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:175
@@ -174,2 +174,3 @@
 
-std::string replacementExpression(const MatchFinder::MatchResult &Result,
+std::string replacementExpression(SimplifyBooleanExprCheck *Check,
+                                  const MatchFinder::MatchResult &Result,
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Check can be a const pointer.
> Passing the check in here is overkill.  This helper function isn't going to ever be used for multiple checks and the only thing you ever do with the check is get the nullptr literal, so just pass in the thing it needs directly.
> 
> This will also need to be rebased onto the current code.
I had same idea, but I'm waiting for global options implementation to make other changes,

================
Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:79-81
@@ -78,1 +78,4 @@
 
+Null pointer literal for fixes could be changed with option
+``NullPointerLiteral``. The default value for C++11 or later is ``nullptr``, for
+ C++98/C++03 - ``0``.
----------------
LegalizeAdulthood wrote:
> The wording here is awkward.  Instead, I suggest:
> 
> ```
> The option ``NullPointerLiteral`` configures the text used for comparisons of pointers
> against zero to synthesize boolean expressions.  The default value for C++11 or later
> is ``nullptr``, otherwise the value is ``NULL``.
> ```
> 
> It's subjective, but my experience is that pre C++11 code bases prefer `NULL` over `0`.
English is not my native language, so help is welcomed.


Repository:
  rL LLVM

http://reviews.llvm.org/D16700