<br><br><div class="gmail_quote">On Sun, Dec 9, 2012 at 11:31 AM, Kim Gräsman <span dir="ltr"><<a href="mailto:kim.grasman@gmail.com" target="_blank">kim.grasman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Richard,<br>
<br>
We have an AST visitor that used the now-removed<br>
getCopyAssignmentOperator() to collect type info for all members,<br>
including implicit ones (see the IWYU project [1]).<br>
<br>
Your commit comment hints that there may be more than one such<br>
operator now... So is there a way to enumerate them? Or can we run<br>
over all method decls and inspect them somehow?<br>
<br>
This AST visitor is code we inherited and I'm not intimately familiar<br>
with the flow -- maybe there's a better way to do this? There are<br>
comments that suggest the RecursiveASTVisitor does not traverse<br>
implicitly generated methods, maybe that's no longer true?<br>
<br>
Thanks,<br>
- Kim<br>
<br>
[1] <a href="http://code.google.com/p/include-what-you-use/" target="_blank">http://code.google.com/p/include-what-you-use/</a><br>
<br></blockquote><div><br>I am not too well versed on the AST, but I can explain what multiple copy constructors are.<br><br>The following *four* methods are copy constructors:<br><br><div style="margin-left:40px">Foo(Foo&);<br>
Foo(Foo const&);<br>Foo(Foo volatile&);<br>Foo(Foo const volatile&);<br></div> <br>-- Matthieu<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

On Sat, Dec 8, 2012 at 5:10 AM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Fri Dec  7 22:10:18 2012<br>
> New Revision: 169670<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=169670&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=169670&view=rev</a><br>
> Log:<br>
> Remove some remnants of the assumption that there is at most one of each<br>
> flavour of special member.<br>
><br>
> Modified:<br>
>     cfe/trunk/include/clang/AST/DeclCXX.h<br>
>     cfe/trunk/lib/AST/DeclCXX.cpp<br>
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=169670&r1=169669&r2=169670&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=169670&r1=169669&r2=169670&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)<br>
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Dec  7 22:10:18 2012<br>
> @@ -748,32 +748,6 @@<br>
>             !(data().DeclaredSpecialMembers & SMF_DefaultConstructor);<br>
>    }<br>
><br>
> -  /// hasConstCopyConstructor - Determines whether this class has a<br>
> -  /// copy constructor that accepts a const-qualified argument.<br>
> -  bool hasConstCopyConstructor() const;<br>
> -<br>
> -  /// getCopyConstructor - Returns the copy constructor for this class<br>
> -  CXXConstructorDecl *getCopyConstructor(unsigned TypeQuals) const;<br>
> -<br>
> -  /// getMoveConstructor - Returns the move constructor for this class<br>
> -  CXXConstructorDecl *getMoveConstructor() const;<br>
> -<br>
> -  /// \brief Retrieve the copy-assignment operator for this class, if available.<br>
> -  ///<br>
> -  /// This routine attempts to find the copy-assignment operator for this<br>
> -  /// class, using a simplistic form of overload resolution.<br>
> -  ///<br>
> -  /// \param ArgIsConst Whether the argument to the copy-assignment operator<br>
> -  /// is const-qualified.<br>
> -  ///<br>
> -  /// \returns The copy-assignment operator that can be invoked, or NULL if<br>
> -  /// a unique copy-assignment operator could not be found.<br>
> -  CXXMethodDecl *getCopyAssignmentOperator(bool ArgIsConst) const;<br>
> -<br>
> -  /// getMoveAssignmentOperator - Returns the move assignment operator for this<br>
> -  /// class<br>
> -  CXXMethodDecl *getMoveAssignmentOperator() const;<br>
> -<br>
>    /// hasUserDeclaredConstructor - Whether this class has any<br>
>    /// user-declared constructors. When true, a default constructor<br>
>    /// will not be implicitly declared.<br>
><br>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Dec  7 22:10:18 2012<br>
> @@ -324,10 +324,6 @@<br>
>    return !forallBases(SawBase, 0);<br>
>  }<br>
><br>
> -bool CXXRecordDecl::hasConstCopyConstructor() const {<br>
> -  return getCopyConstructor(Qualifiers::Const) != 0;<br>
> -}<br>
> -<br>
>  bool CXXRecordDecl::isTriviallyCopyable() const {<br>
>    // C++0x [class]p5:<br>
>    //   A trivially copyable class is a class that:<br>
> @@ -345,126 +341,6 @@<br>
>    return true;<br>
>  }<br>
><br>
> -/// \brief Perform a simplistic form of overload resolution that only considers<br>
> -/// cv-qualifiers on a single parameter, and return the best overload candidate<br>
> -/// (if there is one).<br>
> -static CXXMethodDecl *<br>
> -GetBestOverloadCandidateSimple(<br>
> -  const SmallVectorImpl<std::pair<CXXMethodDecl *, Qualifiers> > &Cands) {<br>
> -  if (Cands.empty())<br>
> -    return 0;<br>
> -  if (Cands.size() == 1)<br>
> -    return Cands[0].first;<br>
> -<br>
> -  unsigned Best = 0, N = Cands.size();<br>
> -  for (unsigned I = 1; I != N; ++I)<br>
> -    if (Cands[Best].second.compatiblyIncludes(Cands[I].second))<br>
> -      Best = I;<br>
> -<br>
> -  for (unsigned I = 0; I != N; ++I)<br>
> -    if (I != Best && Cands[Best].second.compatiblyIncludes(Cands[I].second))<br>
> -      return 0;<br>
> -<br>
> -  return Cands[Best].first;<br>
> -}<br>
> -<br>
> -CXXConstructorDecl *CXXRecordDecl::getCopyConstructor(unsigned TypeQuals) const{<br>
> -  ASTContext &Context = getASTContext();<br>
> -  QualType ClassType<br>
> -    = Context.getTypeDeclType(const_cast<CXXRecordDecl*>(this));<br>
> -  DeclarationName ConstructorName<br>
> -    = Context.DeclarationNames.getCXXConstructorName(<br>
> -                                          Context.getCanonicalType(ClassType));<br>
> -  unsigned FoundTQs;<br>
> -  SmallVector<std::pair<CXXMethodDecl *, Qualifiers>, 4> Found;<br>
> -  DeclContext::lookup_const_iterator Con, ConEnd;<br>
> -  for (llvm::tie(Con, ConEnd) = this->lookup(ConstructorName);<br>
> -       Con != ConEnd; ++Con) {<br>
> -    // C++ [class.copy]p2:<br>
> -    //   A non-template constructor for class X is a copy constructor if [...]<br>
> -    if (isa<FunctionTemplateDecl>(*Con))<br>
> -      continue;<br>
> -<br>
> -    CXXConstructorDecl *Constructor = cast<CXXConstructorDecl>(*Con);<br>
> -    if (Constructor->isCopyConstructor(FoundTQs)) {<br>
> -      if (((TypeQuals & Qualifiers::Const) == (FoundTQs & Qualifiers::Const)) ||<br>
> -          (!(TypeQuals & Qualifiers::Const) && (FoundTQs & Qualifiers::Const)))<br>
> -        Found.push_back(std::make_pair(<br>
> -                                 const_cast<CXXConstructorDecl *>(Constructor),<br>
> -                                       Qualifiers::fromCVRMask(FoundTQs)));<br>
> -    }<br>
> -  }<br>
> -<br>
> -  return cast_or_null<CXXConstructorDecl>(<br>
> -                                        GetBestOverloadCandidateSimple(Found));<br>
> -}<br>
> -<br>
> -CXXConstructorDecl *CXXRecordDecl::getMoveConstructor() const {<br>
> -  for (ctor_iterator I = ctor_begin(), E = ctor_end(); I != E; ++I)<br>
> -    if (I->isMoveConstructor())<br>
> -      return *I;<br>
> -<br>
> -  return 0;<br>
> -}<br>
> -<br>
> -CXXMethodDecl *CXXRecordDecl::getCopyAssignmentOperator(bool ArgIsConst) const {<br>
> -  ASTContext &Context = getASTContext();<br>
> -  QualType Class = Context.getTypeDeclType(const_cast<CXXRecordDecl *>(this));<br>
> -  DeclarationName Name = Context.DeclarationNames.getCXXOperatorName(OO_Equal);<br>
> -<br>
> -  SmallVector<std::pair<CXXMethodDecl *, Qualifiers>, 4> Found;<br>
> -  DeclContext::lookup_const_iterator Op, OpEnd;<br>
> -  for (llvm::tie(Op, OpEnd) = this->lookup(Name); Op != OpEnd; ++Op) {<br>
> -    // C++ [class.copy]p9:<br>
> -    //   A user-declared copy assignment operator is a non-static non-template<br>
> -    //   member function of class X with exactly one parameter of type X, X&,<br>
> -    //   const X&, volatile X& or const volatile X&.<br>
> -    const CXXMethodDecl* Method = dyn_cast<CXXMethodDecl>(*Op);<br>
> -    if (!Method || Method->isStatic() || Method->getPrimaryTemplate())<br>
> -      continue;<br>
> -<br>
> -    const FunctionProtoType *FnType<br>
> -      = Method->getType()->getAs<FunctionProtoType>();<br>
> -    assert(FnType && "Overloaded operator has no prototype.");<br>
> -    // Don't assert on this; an invalid decl might have been left in the AST.<br>
> -    if (FnType->getNumArgs() != 1 || FnType->isVariadic())<br>
> -      continue;<br>
> -<br>
> -    QualType ArgType = FnType->getArgType(0);<br>
> -    Qualifiers Quals;<br>
> -    if (const LValueReferenceType *Ref = ArgType->getAs<LValueReferenceType>()) {<br>
> -      ArgType = Ref->getPointeeType();<br>
> -      // If we have a const argument and we have a reference to a non-const,<br>
> -      // this function does not match.<br>
> -      if (ArgIsConst && !ArgType.isConstQualified())<br>
> -        continue;<br>
> -<br>
> -      Quals = ArgType.getQualifiers();<br>
> -    } else {<br>
> -      // By-value copy-assignment operators are treated like const X&<br>
> -      // copy-assignment operators.<br>
> -      Quals = Qualifiers::fromCVRMask(Qualifiers::Const);<br>
> -    }<br>
> -<br>
> -    if (!Context.hasSameUnqualifiedType(ArgType, Class))<br>
> -      continue;<br>
> -<br>
> -    // Save this copy-assignment operator. It might be "the one".<br>
> -    Found.push_back(std::make_pair(const_cast<CXXMethodDecl *>(Method), Quals));<br>
> -  }<br>
> -<br>
> -  // Use a simplistic form of overload resolution to find the candidate.<br>
> -  return GetBestOverloadCandidateSimple(Found);<br>
> -}<br>
> -<br>
> -CXXMethodDecl *CXXRecordDecl::getMoveAssignmentOperator() const {<br>
> -  for (method_iterator I = method_begin(), E = method_end(); I != E; ++I)<br>
> -    if (I->isMoveAssignmentOperator())<br>
> -      return *I;<br>
> -<br>
> -  return 0;<br>
> -}<br>
> -<br>
>  void CXXRecordDecl::markedVirtualFunctionPure() {<br>
>    // C++ [class.abstract]p2:<br>
>    //   A class is abstract if it has at least one pure virtual function.<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Dec  7 22:10:18 2012<br>
> @@ -4687,12 +4687,28 @@<br>
>      if (RD->hasUserDeclaredMoveConstructor() &&<br>
>          (!getLangOpts().MicrosoftMode || CSM == CXXCopyConstructor)) {<br>
>        if (!Diagnose) return true;<br>
> -      UserDeclaredMove = RD->getMoveConstructor();<br>
> +<br>
> +      // Find any user-declared move constructor.<br>
> +      for (CXXRecordDecl::ctor_iterator I = RD->ctor_begin(),<br>
> +                                        E = RD->ctor_end(); I != E; ++I) {<br>
> +        if (I->isMoveConstructor()) {<br>
> +          UserDeclaredMove = *I;<br>
> +          break;<br>
> +        }<br>
> +      }<br>
>        assert(UserDeclaredMove);<br>
>      } else if (RD->hasUserDeclaredMoveAssignment() &&<br>
>                 (!getLangOpts().MicrosoftMode || CSM == CXXCopyAssignment)) {<br>
>        if (!Diagnose) return true;<br>
> -      UserDeclaredMove = RD->getMoveAssignmentOperator();<br>
> +<br>
> +      // Find any user-declared move assignment operator.<br>
> +      for (CXXRecordDecl::method_iterator I = RD->method_begin(),<br>
> +                                          E = RD->method_end(); I != E; ++I) {<br>
> +        if (I->isMoveAssignmentOperator()) {<br>
> +          UserDeclaredMove = *I;<br>
> +          break;<br>
> +        }<br>
> +      }<br>
>        assert(UserDeclaredMove);<br>
>      }<br>
><br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</blockquote></div><br>