alexfh added a comment.
Thank you for working on this check!
I'd like to note that there is a library-side way to mitigate this issue using
the `[[clang::warn_unused_result]]` attribute on `vector::empty()` and similar
methods:
$ cat q.cc
#if defined(__clang__)
# if __cplusplus >= 201103L && __has_feature(cxx_attributes)
# define CLANG_WARN_UNUSED_RESULT [[clang::warn_unused_result]]
# else
# define CLANG_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
# endif
#else
# define CLANG_WARN_UNUSED_RESULT
#endif
struct Vector {
CLANG_WARN_UNUSED_RESULT bool empty() const;
};
void f() {
Vector v;
v.empty();
}
$ clang_check q.cc -- -std=c++11
q.cc:17:3: warning: ignoring return value of function declared with
warn_unused_result attribute [-Wunused-result]
v.empty();
^~~~~~~
1 warning generated.
It would be super-awesome, if someone could add appropriate annotations to all
methods without side effects in libc++ ;)
================
Comment at: clang-tidy/misc/NoSideEffectsCheck.cpp:42
@@ +41,3 @@
+
+ std::string QualName;
+ llvm::raw_string_ostream OS{QualName};
----------------
I think, there's a slightly more effective way of doing this in
clang-tidy/readability/ContainerSizeEmptyCheck.cpp. Also, Samuel is working on
a hasAnyName matcher that would allow checking whether a declaration name is
one of the declaration names in a list.
================
Comment at: clang-tidy/misc/NoSideEffectsCheck.cpp:52
@@ +51,3 @@
+AST_MATCHER(CXXMethodDecl, isAPurelyInformationalMethod) {
+ static const std::unordered_set<std::string> whitelist{
+ "get_allocator",
----------------
Can we assume that all const methods shouldn't be called without using their
result? This will make this list much shorter.
================
Comment at: clang-tidy/misc/NoSideEffectsCheck.cpp:113
@@ +112,3 @@
+
+ diag(Call->getCallee()->getExprLoc(), "method '%0' has no side effects and
its return value is not used")
+ << Call->getMethodDecl()->getName()
----------------
The line violates 80-columns limit. There are other formatting issues as well,
so I suggest just running clang-format on the file.
================
Comment at: docs/clang-tidy/checks/list.rst:59
@@ -58,2 +58,3 @@
misc-new-delete-overloads
+ misc-no-side-effects
misc-noexcept-move-constructor
----------------
Maybe "misc-unused-result" (like a similar -Wunused-result warning that can be
used for the same purpose, if the methods are properly annotated)?
================
Comment at: test/clang-tidy/misc-no-side-effects.cpp:2
@@ +1,3 @@
+// RUN: %check_clang_tidy %s misc-no-side-effects %t
+#include <vector>
+
----------------
We don't use actual library headers in tests for multiple reasons (longer
testing times, changing the system library can break tests, we can't test
different libraries that way, some testing setups don't support this, etc.).
Instead, we use stubs that model the part of the API required for testing.
http://reviews.llvm.org/D17043
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
