MyDeveloperDay updated this revision to Diff 178367.
MyDeveloperDay added a comment.

-Refactor the checker to remove the llvm::any_of by using a QualType 
ast-matcher with hasAnyParameter(hasType(x))

- i.e. ...unless(hasAnyParameter(hasType(isNonConstReferenceOrPointer())))...

-Reduce the SNR ratio of using a macro as the ReplacementString option (e.g. 
LLVM_NODISCARD), when that macro is not define in-scope

- This was seen when performing clang-tidy on LLVM itself, some of the very low 
level classes did not have the LLVM_NODISCARD macro (from 
include/llvm/Support/Compiler.h) defined in scope.
- In these cases the fix-it caused too many additional changes to be needed of 
including the additional header file in order to build.
- Instead emit a warning to say that it could not be marked as LLVM_NODISCARD 
and don't apply the fix-it
- Add documentation and unit tests to document and validate the above


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
  test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
  test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
  test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
  test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
  test/clang-tidy/modernize-use-nodiscard.cpp
  test/clang-tidy/modernize-use-nodiscard.h

Index: test/clang-tidy/modernize-use-nodiscard.h
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.h
@@ -0,0 +1,5 @@
+
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
Index: test/clang-tidy/modernize-use-nodiscard.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,244 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
+#include <functional>
+
+#include "modernize-use-nodiscard.h"
+
+#define BOOLEAN_FUNC bool f23() const
+
+typedef unsigned my_unsigned;
+typedef unsigned& my_unsigned_reference;
+typedef const unsigned& my_unsigned_const_reference;
+
+class Foo
+{
+public:
+    using size_type = unsigned;
+
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+    // negative tests
+
+    void f5() const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool f6();
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool f7(int &);
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool f8(int &) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool f9(int *) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool f10(const int &,int &) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    NO_DISCARD bool f12() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    MUST_USE_RESULT bool f13() const;
+    // CHECK-MESSAGES-NOT: warning:
+   
+    [[nodiscard]] bool f11() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool _f20() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function '_f20' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool _f20() const;
+
+    NO_RETURN bool f21() const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    ~Foo();
+    // CHECK-MESSAGES-NOT: warning:
+
+    bool operator +=(int) const;
+    // CHECK-MESSAGES-NOT: warning:
+   
+    // extra keywords (virtual,inline,const) on return type
+    
+    virtual bool f14() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+    const bool f15() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD const bool f15() const;
+    
+    inline const bool f16() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+ 
+    inline const std::string &f45() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f45' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline const std::string &f45() const;
+
+    inline virtual const bool f17() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+    // inline with body
+    bool f18() const {
+        // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+        // CHECK-FIXES: NO_DISCARD bool f18() const {
+        return true;
+    }
+
+    bool f19() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f19() const;
+
+    BOOLEAN_FUNC;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f24(size_type) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f24' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f24(size_type) const;
+    
+    bool f28(my_unsigned) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f28' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f28(my_unsigned) const;
+    
+    bool f29(my_unsigned_reference) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f30(my_unsigned_const_reference) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f30' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f30(my_unsigned_const_reference) const;
+
+    template <class F>
+    F f37 (F a, F b) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    template <class F>
+    bool f38 (F a) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f39 (const std::function<bool()>& predicate) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    template<class ...Args>
+    bool ParameterPack(Args...args) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    template<typename... Targs>
+    bool ParameterPack2(Targs... Fargs) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool VariadicFunctionTest (const int&, ...) const;
+    // CHECK-MESSAGES-NOT: warning:
+
+    static bool not_empty();
+    // CHECK-MESSAGES-NOT: warning:
+};
+
+bool Foo::f19() const
+    // CHECK-MESSAGES-NOT: warning:
+{
+    return true;
+}
+
+template<class T>
+class Bar
+{
+    public:
+    using value_type = T;
+    using reference = value_type&;
+    using const_reference = const value_type&;
+
+    bool empty() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'empty' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool empty() const;
+   
+    // we cannot assume that the template parameter isn't a pointer
+    bool f25(value_type) const;
+    // CHECK-MESSAGES-NOT: warning:
+   
+    bool f27(reference) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    typename T::value_type f35() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    T f34() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f31(T) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f33(T&) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f26(const_reference) const;
+    // CHECK-MESSAGES-NOT: warning:
+   
+    bool f32(const T&) const;
+    // CHECK-MESSAGES-NOT: warning:
+};
+
+template<class T>
+class Bar2
+{
+    public:
+    typedef T value_type;
+    typedef value_type& reference;
+    typedef const value_type& const_reference;
+
+    // we cannot assume that the template parameter isn't a pointer
+    bool f40(value_type) const;
+    // CHECK-MESSAGES-NOT: warning:
+   
+    bool f41(reference) const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    value_type f42() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    typename T::value_type f43() const;
+    // CHECK-MESSAGES-NOT: warning:
+    
+    bool f44(const_reference) const;
+    // CHECK-MESSAGES-NOT: warning:
+};
+
+template<class T>
+bool Bar<T>::empty() const
+// CHECK-MESSAGES-NOT: warning:
+{
+    return true;
+}
+
+// don't mark typical ``[[nodisscard]]`` candidates if the class
+// has mutable member variables
+class MutableExample
+{
+    mutable bool m_isempty;
+    public:
+
+    bool empty() const;
+    // CHECK-MESSAGES-NOT: warning:
+
+};
+
+
+
Index: test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN: -- -std=c++17 \
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f4(void) const;
+
+};
Index: test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'CUSTOM_NO_DISCARD'}]}" \
+// RUN: -- -std=c++11 \
+
+// As if the macro was not defined.
+// #define CUSTOM_NO_DISCARD __attribute_((warn_unused_result))
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' could not be marked CUSTOM_NO_DISCARD due to missing macro definition [modernize-use-nodiscard]
+    // CHECK-FIXES: bool f1() const;
+};
+
Index: test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '[[gcc::warn_unused_result]]'}]}" \
+// RUN: -- -std=c++11 \
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f4(void) const;
+
+};
+
Index: test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '__attribute__((warn_unused_result))'}]}" \
+// RUN: -- -std=c++11 \
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f4(void) const;
+
+};
+
Index: test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '[[clang::warn_unused_result]]'}]}" \
+// RUN: -- -std=c++11 \
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f4(void) const;
+
+};
+
Index: docs/clang-tidy/checks/modernize-use-nodiscard.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-nodiscard.rst
@@ -0,0 +1,75 @@
+.. title:: clang-tidy - modernize-use-nodiscard
+
+modernize-use-nodiscard
+=======================
+
+Adds ``[[nodiscard]]`` attributes (introduced in C++17) to member functions in
+order to highlight at compile time which return values should not be ignored.
+
+Member functions need to satisfy the following conditions to be considered by
+this check:
+  - no ``[[nodiscard]]``, ``[[noreturn]]``, ``__attribute__((warn_unused_result))``, ``[[clang::warn_unused_result]]`` nor ``[[gcc::warn_unused_result]]`` attribute,
+  - non-void return type,
+  - non-template return types,
+  - const member function,
+  - non-variadic functions,
+  - no non-const reference parameters,
+  - no pointer parameters,
+  - no template parameters,
+  - no template function parameters,
+  - not be a member of a class with mutable member variables.
+
+Such functions have no means of altering any state or passing values other than
+via the return type. Unless the member functions are altering state via some 
+external call (e.g. I/O).
+
+Example
+-------
+
+.. code-block:: c++
+
+    bool empty() const;
+    bool empty(int i) const;
+
+transforms to:
+
+.. code-block:: c++
+
+    [[nodiscard] bool empty() const;
+    [[nodiscard] bool empty(int i) const;
+
+Options
+-------
+
+.. option:: ReplacementString
+
+Specifies a macro to use instead of ``[[nodiscard]]``. This is useful when
+maintaining source code that needs to compile with a pre-C++17 compiler.
+
+Example
+^^^^^^^
+
+.. code-block:: c++
+
+    bool empty() const;
+    bool empty(int i) const;
+
+transforms to:
+
+.. code-block:: c++
+
+    NO_DISCARD bool empty() const;
+    NO_DISCARD bool empty(int i) const;
+
+if the :option:`ReplacementString` option is set to `NO_DISCARD`.
+
+.. note::
+
+If the :option:`ReplacementString` is not a C++ attribute, but instead a macro,
+then that macro must be defined in scope or the fix-it will not be applied.
+
+.. note::
+
+   For alternative ``__attribute__`` syntax options to mark functions as
+   ``[[nodiscard]]`` in non-c++17 source code.
+   See https://clang.llvm.org/docs/AttributeReference.html#nodiscard-warn-unused-result
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -207,6 +207,7 @@
    modernize-use-emplace
    modernize-use-equals-default
    modernize-use-equals-delete
+   modernize-use-nodiscard
    modernize-use-noexcept
    modernize-use-nullptr
    modernize-use-override
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -23,9 +23,9 @@
 feature work. All LLVM releases may be downloaded from the `LLVM releases web
 site <https://llvm.org/releases/>`_.
 
-For more information about Clang or LLVM, including information about
-the latest release, please see the `Clang Web Site <https://clang.llvm.org>`_ or
-the `LLVM Web Site <https://llvm.org>`_.
+For more information about Clang or LLVM, including information about the latest
+release, please see the `Clang Web Site <https://clang.llvm.org>`_ or the 
+`LLVM Web Site <https://llvm.org>`_.
 
 Note that if you are reading this file from a Subversion checkout or the
 main Clang web page, this document applies to the *next* release, not
@@ -180,6 +180,12 @@
   Detects usage of the deprecated member types of ``std::ios_base`` and replaces
   those that have a non-deprecated equivalent.
 
+- New :doc:`modernize-use-nodiscard
+  <clang-tidy/checks/modernize-use-nodiscard>` check.
+
+  Adds ``[[nodiscard]]`` attributes (introduced in C++17) to member functions 
+  to highlight at compile time which return values should not be ignored.
+
 - New :doc:`readability-isolate-decl
   <clang-tidy/checks/readability-isolate-declaration>` check.
 
Index: clang-tidy/modernize/UseNodiscardCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseNodiscardCheck.h
@@ -0,0 +1,50 @@
+//===--- UseNodiscardCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USENODISCARDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USENODISCARDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// \brief Add ``[[nodiscard]]`` to non-void const-member functions with no
+/// arguments or pass-by-value or pass by const-reference arguments.
+/// \code
+///    bool empty() const;
+///    bool empty(const Bar &) const;
+///    bool empty(int bar) const;
+/// \endcode
+/// Is converted to:
+/// \code
+///    [[nodiscard]] bool empty() const;
+///    [[nodiscard]] bool empty(const Bar &) const;
+///    [[nodiscard]] bool empty(int bar) const;
+/// \endcode
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nodiscard.html
+class UseNodiscardCheck : public ClangTidyCheck {
+public:
+  UseNodiscardCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const std::string NoDiscardMacro;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USENODISCARDCHECK_H
Index: clang-tidy/modernize/UseNodiscardCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseNodiscardCheck.cpp
@@ -0,0 +1,146 @@
+//===--- UseNodiscardCheck.cpp - clang-tidy -------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseNodiscardCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+// TODO: Find a better way of detecting const-reference-template type
+// parameters via using alias not detected by ``isTemplateTypeParamType()``.
+static bool isAliasedTemplateParamType(const QualType &ParamType) {
+  return (ParamType.getCanonicalType().getAsString().find("type-parameter-") !=
+          std::string::npos);
+}
+
+// TODO: Find a better way of detecting a function template.
+static bool isFunctionTemplate(const QualType &ParamType) {
+  // Try to catch both std::function and boost::function
+  return (ParamType.getAsString().find("::function") != std::string::npos);
+}
+
+static bool isNonConstReferenceType(const QualType &ParamType) {
+  return ParamType->isReferenceType() &&
+         !ParamType.getNonReferenceType().isConstQualified();
+}
+
+static bool doesNoDiscardMacroExist(ASTContext &Context,
+                                    const std::string &MacroId) {
+  // Don't check for the Macro existence if we are using an attribute
+  // either a C++17 standard attribute or pre C++17 syntax
+  if (MacroId.find("[[") == 0 || MacroId.find("__attribute__") == 0)
+    return true;
+
+  // Otherwise look up the macro name in the context to see if its defined.
+  return Context.Idents.get(MacroId).hasMacroDefinition();
+}
+
+namespace {
+AST_MATCHER(CXXMethodDecl, isOverloadedOperator) {
+  // Don't put [[nodiscard]] in front of operators.
+  return Node.isOverloadedOperator();
+}
+AST_MATCHER(CXXMethodDecl, hasClassMutableFields) {
+  // Don't put [[nodiscard]] in front functions on classes with
+  // mutable member variables.
+  return Node.getParent()->hasMutableFields();
+}
+AST_MATCHER(CXXMethodDecl, hasTemplateReturnType) {
+  // Don't put [[nodiscard]] in front of functions returning a template type
+  return (Node.getReturnType()->isTemplateTypeParmType() ||
+          isAliasedTemplateParamType(Node.getReturnType()));
+}
+AST_MATCHER(CXXMethodDecl, isDefinitionOrInline) {
+  // A function definition, with optional inline but not the declaration.
+  return !(Node.isThisDeclarationADefinition() && Node.isOutOfLine());
+}
+AST_MATCHER(QualType, isNonConstReferenceOrPointer) {
+  // If the function has any non-const-reference arguments
+  //    bool foo(A &a)
+  // or pointer arguments
+  //    bool foo(A*)
+  // then they may not care about the return value because of passing data
+  // via the arguments.
+  QualType ParType = Node;
+  if (ParType->isTemplateTypeParmType() || ParType->isPointerType() ||
+      isNonConstReferenceType(ParType) || isFunctionTemplate(ParType))
+    return true;
+
+  return isAliasedTemplateParamType(ParType);
+}
+} // namespace
+
+UseNodiscardCheck::UseNodiscardCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      NoDiscardMacro(Options.get("ReplacementString", "[[nodiscard]]")) {}
+
+void UseNodiscardCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ReplacementString", NoDiscardMacro);
+}
+
+void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
+  // If we use ``[[nodiscard]]`` attribute, we require at least C++17. Use a
+  // macro or ``__attribute__`` with pre c++17 compilers by using
+  // ReplacementString option.
+  if ((NoDiscardMacro == "[[nodiscard]]" && !getLangOpts().CPlusPlus17) ||
+      !getLangOpts().CPlusPlus)
+    return;
+
+  // Find all non-void const methods which have not already been marked to
+  // warn on unused result.
+  Finder->addMatcher(
+      cxxMethodDecl(
+          allOf(
+              unless(returns(voidType())), isConst(), unless(isNoReturn()),
+              unless(isOverloadedOperator()), isDefinitionOrInline(),
+              unless(isVariadic()), unless(hasTemplateReturnType()),
+              unless(hasClassMutableFields()),
+              unless(hasAnyParameter(hasType(isNonConstReferenceOrPointer()))),
+              unless(hasAttr(clang::attr::WarnUnusedResult))))
+          .bind("no_discard"),
+      this);
+} // namespace modernize
+
+void UseNodiscardCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("no_discard");
+  // Don't make replacements if the location is invalid or in a macro.
+  SourceLocation Loc = MatchedDecl->getLocation();
+  if (Loc.isInvalid() || Loc.isMacroID())
+    return;
+
+  SourceLocation retLoc = MatchedDecl->getInnerLocStart();
+
+  auto &Context = *Result.Context;
+  // check for the existance of the keyword being used as the ``[[nodiscard]]``
+  if (!doesNoDiscardMacroExist(Context, NoDiscardMacro)) {
+    diag(retLoc, "function %0 could not be marked " + NoDiscardMacro +
+                     " due to missing macro definition")
+        << MatchedDecl;
+    return;
+  }
+
+  // Possible false positives include:
+  // 1. A const member function which returns a variable which is ignored
+  // but performs some external I/O operation and the return value could be
+  // ignored.
+  diag(retLoc, "function %0 should be marked " + NoDiscardMacro)
+      << MatchedDecl
+      << FixItHint::CreateInsertion(retLoc, NoDiscardMacro + " ");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -32,6 +32,7 @@
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
 #include "UseEqualsDeleteCheck.h"
+#include "UseNodiscardCheck.h"
 #include "UseNoexceptCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
@@ -82,6 +83,8 @@
     CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
     CheckFactories.registerCheck<UseEqualsDeleteCheck>(
         "modernize-use-equals-delete");
+    CheckFactories.registerCheck<UseNodiscardCheck>(
+        "modernize-use-nodiscard");
     CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -26,6 +26,7 @@
   UseEmplaceCheck.cpp
   UseEqualsDefaultCheck.cpp
   UseEqualsDeleteCheck.cpp
+  UseNodiscardCheck.cpp
   UseNoexceptCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to