MyDeveloperDay updated this revision to Diff 178654. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment.
Fix review comments - Fix minor spelling mistakes - add additional boost::function unit test 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> + +namespace boost { +template <class > +class function; +} + +#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: + + bool f45(const boost::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 ``[[nodiscard]]`` 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,24 @@ +// 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