MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23
+static bool isAliasedTemplateParamType(QualType ParamType) {
+ return (ParamType.getCanonicalType()->isTemplateTypeParmType() ||
+ ParamType->isInstantiationDependentType());
----------------
JonasToth wrote:
> 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.
I agree, cleaner less code and easier to understand..moving it to a matcher
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:111
+ hasAnyParameter(anyOf(
+ parmVarDecl(hasType(references(functionObj))),
+ parmVarDecl(hasType(functionObj)),
----------------
JonasToth wrote:
> you can merge these two lines with `anyOf(functionObj,
> references(functionObj))`, i think thats cleaner
I had to move the hasType inside the anyOf?
================
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)
----------------
JonasToth wrote:
> Please remove the duplication with `diag`.
> You can store the diagnostic in flight and append something afterwards:
>
> ```
> auto Diag = diag(...);
>
> if (canTransform)
> Diag << Change;
> ```
that feels cleaner
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
