[PATCH] Warn on explicit copy constructors.

[Alexander Kornienko]

On Tue, Apr 29, 2014 at 5:28 PM, Arthur O'Dwyer
<arthur.j.odwyer at gmail.com>wrote:

> On Tue, Apr 29, 2014 at 6:58 AM, Alexander Kornienko <alexfh at google.com>
> wrote:
> >
> > http://reviews.llvm.org/D3541
>
> > +  if (Ctor->isOutOfLine() || Ctor->isImplicit() || Ctor->isDeleted())
> >      return;
> > +  if (Ctor->isExplicit() || Ctor->isCopyOrMoveConstructor()) {
> > +    if (Ctor->isExplicit() && Ctor->isCopyOrMoveConstructor()) {
> > +      SourceRange ExplicitTokenRange = FindToken(
> > +          *Result.SourceManager, Result.Context->getLangOpts(),
> > +          Ctor->getOuterLocStart(), Ctor->getLocEnd(), [](const Token
> &Tok) {
> > +            return Tok.is(tok::raw_identifier) &&
> > +                   StringRef(Tok.getRawIdentifierData(),
> Tok.getLength()) ==
> > +                       "explicit";
> > +          });
> > +      if (ExplicitTokenRange.isValid()) {
> > +        DiagnosticBuilder Diag =
> > +            diag(ExplicitTokenRange.getBegin(),
> > +                 "Copy constructor declared explicit.");
>
> You mean "copy or move constructor"; and you could use a test for the
> move-constructor case.
>

Thanks for noting, I'll update the message and make the test cases more
diverse.


>
> There are corner cases such as
>
>     C(const C&&);
>     C(const C&, int i = 0);
>     template<typename T> C(const T&);
>
> that would be interesting to test.
>

$ cat q.cc
class A {
public:
  A(const A&&) {}
  A(const A&, int i = 0) {}
  template<typename T> A(const T&) {}
};
$ clang-tidy -checks=google -disable-checks='' q.cc  --
q.cc:5:24: warning: Single-argument constructors must be explicit
[google-explicit-constructor]
  template<typename T> A(const T&) {}
                       ^
                       explicit

Is this what you expected? To me it seems reasonable.


>
> –Arthur
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140429/91de25df/attachment.html>