aaron.ballman added inline comments.
================
Comment at: test/clang-tidy/cert-dcl21-cpp.cpp:1
+// RUN: %check_clang_tidy %s cert-dcl21-cpp %t
+
----------------
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.
https://reviews.llvm.org/D32743
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
