xazax.hun marked 4 inline comments as done.
xazax.hun added inline comments.
================
Comment at: test/clang-tidy/cert-dcl21-cpp.cpp:1
+// RUN: %check_clang_tidy %s cert-dcl21-cpp %t
+
----------------
aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > As usual, please add tests with macros and templates with multiple
> > > > instantiations. When diagnostics in macros are ignored, the tests
> > > > should demonstrate this as well.
> > > In addition to these tests, I'd like to see a test for the following:
> > > ```
> > > struct S {};
> > > typedef S& SRef;
> > >
> > > SRef operator++(SRef, int);
> > >
> > > struct T {
> > > typedef T& TRef;
> > >
> > > TRef operator++(int);
> > > };
> > >
> > > struct U {
> > > typedef const U& ConstURef;
> > >
> > > ConstURef& operator++(int); // There's an extra ref shoved on here for
> > > fun.
> > > };
> > >
> > > struct V {
> > > V *operator++(int);
> > > V *const operator--(int);
> > > };
> > > ```
> > I added those test cases.
> >
> > I choose not to warn about primitive and pointer types since it makes no
> > sense to make them const. Do you agree?
> >
> > Unfortunately, the current fixits might use the underlying type instead of
> > the typedef. Is that a blocker for this to be commited?
> > In same cases it is not possible to preserve the typedef, e.g. when the
> > underlying type is a reference. (Or we need to insert a
> > `std::remove_reference` meta call).
> >
> > In one case the const is not added. This is because the underlying type is
> > already const. Rewriting the fixit is not trivial, because it is not easy
> > to get the source range for the whole return type. Unfortunately,
> > `getReturnTypeSourceRange` ignores the qualifiers.
> > I choose not to warn about primitive and pointer types since it makes no
> > sense to make them const. Do you agree?
>
> I think that's reasonable behavior, yes.
>
> > Unfortunately, the current fixits might use the underlying type instead of
> > the typedef. Is that a blocker for this to be commited?
>
> That's what I was wondering about. You may want to simply disable the fixit
> if the underlying type is a typedef (or alias, etc).
>
> > In same cases it is not possible to preserve the typedef, e.g. when the
> > underlying type is a reference. (Or we need to insert a
> > std::remove_reference meta call).
>
> Yes, I think it's a complex problem that doesn't need to be solved right now.
>
> > In one case the const is not added. This is because the underlying type is
> > already const. Rewriting the fixit is not trivial, because it is not easy
> > to get the source range for the whole return type. Unfortunately,
> > getReturnTypeSourceRange ignores the qualifiers.
>
> That is unfortunate, but I don't think it's a deal-breaker, either.
Good idea, done.
https://reviews.llvm.org/D32743
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
