JonasToth added a comment.
Mostly nits left. I think the check is good to go afterwards :)
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23
+static bool isAliasedTemplateParamType(QualType ParamType) {
+ return (ParamType.getCanonicalType()->isTemplateTypeParmType() ||
+ ParamType->isInstantiationDependentType());
----------------
I think you don't need the first part of that condition.
`isInstantiationDependent` should include that, no?
I would prefer having this as a matcher, but i don't insist.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:111
+ hasAnyParameter(anyOf(
+ parmVarDecl(hasType(references(functionObj))),
+ parmVarDecl(hasType(functionObj)),
----------------
you can merge these two lines with `anyOf(functionObj,
references(functionObj))`, i think thats cleaner
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:128
+
+ auto &Context = *Result.Context;
+ // Check for the existence of the keyword being used as the
``[[nodiscard]]``.
----------------
I would say ok-ish, but `ASTContext &Context = *Result.Context;` would be
better.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:130
+ // Check for the existence of the keyword being used as the
``[[nodiscard]]``.
+ if (!doesNoDiscardMacroExist(Context, NoDiscardMacro)) {
+ diag(retLoc, "function %0 should be marked " + NoDiscardMacro)
----------------
Please remove the duplication with `diag`.
You can store the diagnostic in flight and append something afterwards:
```
auto Diag = diag(...);
if (canTransform)
Diag << Change;
```
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
