[clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

[vmiklos]

Summary

And also enable it by default to be consistent with e.g. modernize-use-using.

This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.

[vmiklos]

In practice cppunit's CPPUNIT_TEST_SUITE_END macro triggers this problem.

[JonasToth]

LGTM, but could you please add a short notice in the release notes?

[vmiklos]

In D53454#1270298, @JonasToth wrote:

LGTM, but could you please add a short notice in the release notes?

Sure, I will do that.

clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.h

 /// \code
 ///   ptr.get()->Foo()  ==>  ptr->Foo()
 ///   *ptr.get()  ==>  *ptr
 ///   *ptr->get()  ==>  **ptr
 /// \endcode
 class RedundantSmartptrGetCheck : public ClangTidyCheck {
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTSMARTPTRGETCHECK_H

clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp

Finder->addMatcher(
                      hasEitherOperand(callToGet(IsAKnownSmartptr))),
       Callback);
 
   // FIXME: Match and fix if (l.get() == r.get()).
 }
 
 } // namespace
 
+void RedundantSmartptrGetCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
   if (!getLangOpts().CPlusPlus)
     return;
 
   registerMatchersForGetArrowStart(Finder, this);
   registerMatchersForGetEquals(Finder, this);

 
 void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
   if (!allReturnTypesMatch(Result))
     return;
 
   bool IsPtrToPtr = Result.Nodes.getNodeAs<Decl>("ptr_to_ptr") != nullptr;
   bool IsMemberExpr = Result.Nodes.getNodeAs<Expr>("memberExpr") != nullptr;
   const auto *GetCall = Result.Nodes.getNodeAs<Expr>("redundant_get");
+  if (GetCall->getBeginLoc().isMacroID() && IgnoreMacros)
+    return;
+
   const auto *Smartptr = Result.Nodes.getNodeAs<Expr>("smart_pointer");
 
   if (IsPtrToPtr && IsMemberExpr) {
     // Ignore this case (eg. Foo->get()->DoSomething());
     return;
   }
 
   StringRef SmartptrText = Lexer::getSourceText(

clang-tools-extra/trunk/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
+- The :doc:`readability-redundant-smartptr-get
+  <clang-tidy/checks/readability-redundant-smartptr-get>` check does not warn
+  about calls inside macros anymore by default.
+
 - New :doc:`abseil-duration-division
   <clang-tidy/checks/abseil-duration-division>` check.
 
   Checks for uses of ``absl::Duration`` division that is done in a
   floating-point context, and recommends the use of a function that
   returns a floating-point value.
 
 - New :doc:`abseil-faster-strsplit-delimiter

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst

 .. title:: clang-tidy - readability-redundant-smartptr-get
 
 readability-redundant-smartptr-get
 ==================================
 
 Find and remove redundant calls to smart pointer's ``.get()`` method.
 
 Examples:
 
 .. code-block:: c++
 
   ptr.get()->Foo()  ==>  ptr->Foo()
   *ptr.get()  ==>  *ptr
   *ptr->get()  ==>  **ptr
   if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about calls inside macros.

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get-macros.cpp

+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-smartptr-get.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+namespace std {
+
+template <typename T>
+struct shared_ptr {
+  T &operator*() const;
+  T *operator->() const;
+  T *get() const;
+  explicit operator bool() const noexcept;
+};
+
+} // namespace std
+
+#define MACRO(p) p.get()
+
+void Positive() {
+  std::shared_ptr<int> x;
+  if (MACRO(x) == nullptr)
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant get() call on smart pointer
+};

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp

void Positive() {
   // CHECK-MESSAGES: if (x.get() == NULL);
   // CHECK-FIXES: if (x == NULL);
   if (NULL == x.get());
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call
   // CHECK-MESSAGES: if (NULL == x.get());
   // CHECK-FIXES: if (NULL == x);
 }
 
+#define MACRO(p) p.get()
+
 void Negative() {
   struct NegPtr {
     int* get();
     int* operator->() {
       return &*this->get();
     }
     int& operator*() {
       return *get();
void Negative() {
   Fail2().get()->Do();
   const Bar& b = *Fail1().get();
   (*Fail2().get()).Do();
 
   int_ptr ip;
   bool bb = ip.get() == nullptr;
   bb = !ip.get();
   bb = ip.get() ? true : false;
+  std::unique_ptr<int> x;
+  if (MACRO(x) == nullptr)
+    ;
 }