lebedev.ri added inline comments.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30
+bool isOperator(const FunctionDecl *D) {
+ return D->getNameAsString().find("operator") != std::string::npos;
+}
----------------
Can't you use `isOverloadedOperator()`?
No way going to `std::string` is fast.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32-34
+bool isInternalFunction(const FunctionDecl *D) {
+ return D->getNameAsString().find("_") == 0;
+}
----------------
Motivation?
This should be configurable in some way.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:47
+void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus17)
+ return;
----------------
This should be smarter, since you provide `ReplacementString`.
E.g. if `ReplacementString` is default, require C++17.
Else, only require C++.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:63-95
+ // if the location is invalid forget it
+ if (!MatchedDecl->getLocation().isValid())
+ return;
+
+ if (MatchedDecl->isThisDeclarationADefinition() &&
MatchedDecl->isOutOfLine())
+ return;
+
----------------
All this should be done in `registerMatchers()`.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:108
+ << MatchedDecl
+ << FixItHint::CreateInsertion(retLoc, NoDiscardMacro + " ");
+ return;
----------------
Can you actually provide fix-it though?
If the result will be unused, it will break build under `-Werror`.
================
Comment at: docs/ReleaseNotes.rst:79-81
+ Checks for non void const member functions which should have
+ ``[[nodiscard]]`` added to tell the compiler a return type shoud not be
+ ignored
----------------
The actual rules should be spelled out here, right now this sounds a bit too
magical.
================
Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:6
+
+This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member
functions to highlight at compile time where the return value of a function
should not be ignored
+
----------------
Eugene.Zelenko wrote:
> Please use 80 characters limit.
Same, the rules need to be spelled out, this sounds a bit too magical right now,
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55433/new/
https://reviews.llvm.org/D55433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
