MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.
================
Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126
+template<class T>
+class Bar
+{
----------------
MyDeveloperDay wrote:
> JonasToth wrote:
> > MyDeveloperDay wrote:
> > > curdeius wrote:
> > > > JonasToth wrote:
> > > > > I think the template tests should be improved.
> > > > > What happens for `T empty()`, `typename T::value_type empty()` and so
> > > > > on. THe same for the parameters for functions/methods (including
> > > > > function templates).
> > > > >
> > > > > Thinking of it, it might be a good idea to ignore functions where the
> > > > > heuristic depends on the template-paramters.
> > > > It might be a good idea to add a note in the documentation about
> > > > handling of function templates and functions in class templates.
> > > I think I need some help to determine if the parameter is a template
> > > parameter (specially a const T & or a const_reference)
> > >
> > > I'm trying to remove functions which have any type of Template parameter
> > > (at least for now)
> > >
> > > I've modified the hasNonConstReferenceOrPointerArguments matcher to use
> > > isTemplateTypeParamType()
> > >
> > > but this doesn't seem to work though an Alias or even just with a const &
> > >
> > > ```
> > > return llvm::any_of(Node.parameters(), [](const ParmVarDecl *Par) {
> > > QualType ParType = Par->getType();
> > >
> > > if (ParType->isTemplateTypeParmType())
> > > return true;
> > >
> > > if (ParType->isPointerType() || isNonConstReferenceType(ParType))
> > > return true;
> > >
> > > return false;
> > > });
> > > ```
> > >
> > > mostly the tests cases work for T and T& (see below)
> > >
> > > but it does not seem to work for const T&, or const_reference, where it
> > > still wants to add the [[nodiscard]]
> > >
> > > Could anyone give me any pointers, or somewhere I can look to learn? I
> > > was thinking I needed to look at the getUnqualifiedDeSugared() but it
> > > didn't seem to work the way I expected.
> > >
> > > ```
> > > 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:
> > > // CHECK-FIXES: bool f25(value_type) const;
> > >
> > > bool f27(reference) const;
> > > // CHECK-MESSAGES-NOT: warning:
> > > // CHECK-FIXES: bool f27(reference) const
> > >
> > > typename T::value_type f35() const;
> > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f35' should be
> > > marked NO_DISCARD [modernize-use-nodiscard]
> > > // CHECK-FIXES: NO_DISCARD typename T::value_type f35() const
> > >
> > > T f34() const;
> > > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f34' should be
> > > marked NO_DISCARD [modernize-use-nodiscard]
> > > // CHECK-FIXES: NO_DISCARD T f34() const
> > >
> > > bool f31(T) const;
> > > // CHECK-MESSAGES-NOT: warning:
> > > // CHECK-FIXES: bool f31(T) const
> > >
> > > bool f33(T&) const;
> > > // CHECK-MESSAGES-NOT: warning:
> > > // CHECK-FIXES: bool f33(T&) const
> > >
> > > // ------------- FIXME TESTS BELOW FAIL ------------- //
> > > bool f26(const_reference) const;
> > > // CHECK-MESSAGES-NOT: warning:
> > > // CHECK-FIXES: bool f26(const_reference) const;
> > >
> > > bool f32(const T&) const;
> > > // CHECK-MESSAGES-NOT: warning:
> > > // CHECK-FIXES: bool f32(const T&) const
> > > };
> > >
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > Is this resolved, as you marked it done?
> Still working on these last 2 cases, they don't seem to be excluded with the
> isTemplateTypeParmType() call
>
> // ------------- FIXME TESTS BELOW FAIL ------------- //
> bool f26(const_reference) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f26(const_reference) const;
>
> bool f32(const T&) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f32(const T&) const
>
> Let me fix what I can and I'll send an updated revision
template arguments are now ignored so these tests are fixed
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
