[cfe-dev] [cfe-commits] r169670 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/Sema/SemaDeclCXX.cpp

Kim Gräsman kim.grasman at gmail.com
Sun Dec 9 02:31:08 PST 2012


Hi Richard,

We have an AST visitor that used the now-removed
getCopyAssignmentOperator() to collect type info for all members,
including implicit ones (see the IWYU project [1]).

Your commit comment hints that there may be more than one such
operator now... So is there a way to enumerate them? Or can we run
over all method decls and inspect them somehow?

This AST visitor is code we inherited and I'm not intimately familiar
with the flow -- maybe there's a better way to do this? There are
comments that suggest the RecursiveASTVisitor does not traverse
implicitly generated methods, maybe that's no longer true?

Thanks,
- Kim

[1] http://code.google.com/p/include-what-you-use/

On Sat, Dec 8, 2012 at 5:10 AM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Fri Dec  7 22:10:18 2012
> New Revision: 169670
>
> URL: http://llvm.org/viewvc/llvm-project?rev=169670&view=rev
> Log:
> Remove some remnants of the assumption that there is at most one of each
> flavour of special member.
>
> Modified:
>     cfe/trunk/include/clang/AST/DeclCXX.h
>     cfe/trunk/lib/AST/DeclCXX.cpp
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=169670&r1=169669&r2=169670&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Dec  7 22:10:18 2012
> @@ -748,32 +748,6 @@
>             !(data().DeclaredSpecialMembers & SMF_DefaultConstructor);
>    }
>
> -  /// hasConstCopyConstructor - Determines whether this class has a
> -  /// copy constructor that accepts a const-qualified argument.
> -  bool hasConstCopyConstructor() const;
> -
> -  /// getCopyConstructor - Returns the copy constructor for this class
> -  CXXConstructorDecl *getCopyConstructor(unsigned TypeQuals) const;
> -
> -  /// getMoveConstructor - Returns the move constructor for this class
> -  CXXConstructorDecl *getMoveConstructor() const;
> -
> -  /// \brief Retrieve the copy-assignment operator for this class, if available.
> -  ///
> -  /// This routine attempts to find the copy-assignment operator for this
> -  /// class, using a simplistic form of overload resolution.
> -  ///
> -  /// \param ArgIsConst Whether the argument to the copy-assignment operator
> -  /// is const-qualified.
> -  ///
> -  /// \returns The copy-assignment operator that can be invoked, or NULL if
> -  /// a unique copy-assignment operator could not be found.
> -  CXXMethodDecl *getCopyAssignmentOperator(bool ArgIsConst) const;
> -
> -  /// getMoveAssignmentOperator - Returns the move assignment operator for this
> -  /// class
> -  CXXMethodDecl *getMoveAssignmentOperator() const;
> -
>    /// hasUserDeclaredConstructor - Whether this class has any
>    /// user-declared constructors. When true, a default constructor
>    /// will not be implicitly declared.
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Dec  7 22:10:18 2012
> @@ -324,10 +324,6 @@
>    return !forallBases(SawBase, 0);
>  }
>
> -bool CXXRecordDecl::hasConstCopyConstructor() const {
> -  return getCopyConstructor(Qualifiers::Const) != 0;
> -}
> -
>  bool CXXRecordDecl::isTriviallyCopyable() const {
>    // C++0x [class]p5:
>    //   A trivially copyable class is a class that:
> @@ -345,126 +341,6 @@
>    return true;
>  }
>
> -/// \brief Perform a simplistic form of overload resolution that only considers
> -/// cv-qualifiers on a single parameter, and return the best overload candidate
> -/// (if there is one).
> -static CXXMethodDecl *
> -GetBestOverloadCandidateSimple(
> -  const SmallVectorImpl<std::pair<CXXMethodDecl *, Qualifiers> > &Cands) {
> -  if (Cands.empty())
> -    return 0;
> -  if (Cands.size() == 1)
> -    return Cands[0].first;
> -
> -  unsigned Best = 0, N = Cands.size();
> -  for (unsigned I = 1; I != N; ++I)
> -    if (Cands[Best].second.compatiblyIncludes(Cands[I].second))
> -      Best = I;
> -
> -  for (unsigned I = 0; I != N; ++I)
> -    if (I != Best && Cands[Best].second.compatiblyIncludes(Cands[I].second))
> -      return 0;
> -
> -  return Cands[Best].first;
> -}
> -
> -CXXConstructorDecl *CXXRecordDecl::getCopyConstructor(unsigned TypeQuals) const{
> -  ASTContext &Context = getASTContext();
> -  QualType ClassType
> -    = Context.getTypeDeclType(const_cast<CXXRecordDecl*>(this));
> -  DeclarationName ConstructorName
> -    = Context.DeclarationNames.getCXXConstructorName(
> -                                          Context.getCanonicalType(ClassType));
> -  unsigned FoundTQs;
> -  SmallVector<std::pair<CXXMethodDecl *, Qualifiers>, 4> Found;
> -  DeclContext::lookup_const_iterator Con, ConEnd;
> -  for (llvm::tie(Con, ConEnd) = this->lookup(ConstructorName);
> -       Con != ConEnd; ++Con) {
> -    // C++ [class.copy]p2:
> -    //   A non-template constructor for class X is a copy constructor if [...]
> -    if (isa<FunctionTemplateDecl>(*Con))
> -      continue;
> -
> -    CXXConstructorDecl *Constructor = cast<CXXConstructorDecl>(*Con);
> -    if (Constructor->isCopyConstructor(FoundTQs)) {
> -      if (((TypeQuals & Qualifiers::Const) == (FoundTQs & Qualifiers::Const)) ||
> -          (!(TypeQuals & Qualifiers::Const) && (FoundTQs & Qualifiers::Const)))
> -        Found.push_back(std::make_pair(
> -                                 const_cast<CXXConstructorDecl *>(Constructor),
> -                                       Qualifiers::fromCVRMask(FoundTQs)));
> -    }
> -  }
> -
> -  return cast_or_null<CXXConstructorDecl>(
> -                                        GetBestOverloadCandidateSimple(Found));
> -}
> -
> -CXXConstructorDecl *CXXRecordDecl::getMoveConstructor() const {
> -  for (ctor_iterator I = ctor_begin(), E = ctor_end(); I != E; ++I)
> -    if (I->isMoveConstructor())
> -      return *I;
> -
> -  return 0;
> -}
> -
> -CXXMethodDecl *CXXRecordDecl::getCopyAssignmentOperator(bool ArgIsConst) const {
> -  ASTContext &Context = getASTContext();
> -  QualType Class = Context.getTypeDeclType(const_cast<CXXRecordDecl *>(this));
> -  DeclarationName Name = Context.DeclarationNames.getCXXOperatorName(OO_Equal);
> -
> -  SmallVector<std::pair<CXXMethodDecl *, Qualifiers>, 4> Found;
> -  DeclContext::lookup_const_iterator Op, OpEnd;
> -  for (llvm::tie(Op, OpEnd) = this->lookup(Name); Op != OpEnd; ++Op) {
> -    // C++ [class.copy]p9:
> -    //   A user-declared copy assignment operator is a non-static non-template
> -    //   member function of class X with exactly one parameter of type X, X&,
> -    //   const X&, volatile X& or const volatile X&.
> -    const CXXMethodDecl* Method = dyn_cast<CXXMethodDecl>(*Op);
> -    if (!Method || Method->isStatic() || Method->getPrimaryTemplate())
> -      continue;
> -
> -    const FunctionProtoType *FnType
> -      = Method->getType()->getAs<FunctionProtoType>();
> -    assert(FnType && "Overloaded operator has no prototype.");
> -    // Don't assert on this; an invalid decl might have been left in the AST.
> -    if (FnType->getNumArgs() != 1 || FnType->isVariadic())
> -      continue;
> -
> -    QualType ArgType = FnType->getArgType(0);
> -    Qualifiers Quals;
> -    if (const LValueReferenceType *Ref = ArgType->getAs<LValueReferenceType>()) {
> -      ArgType = Ref->getPointeeType();
> -      // If we have a const argument and we have a reference to a non-const,
> -      // this function does not match.
> -      if (ArgIsConst && !ArgType.isConstQualified())
> -        continue;
> -
> -      Quals = ArgType.getQualifiers();
> -    } else {
> -      // By-value copy-assignment operators are treated like const X&
> -      // copy-assignment operators.
> -      Quals = Qualifiers::fromCVRMask(Qualifiers::Const);
> -    }
> -
> -    if (!Context.hasSameUnqualifiedType(ArgType, Class))
> -      continue;
> -
> -    // Save this copy-assignment operator. It might be "the one".
> -    Found.push_back(std::make_pair(const_cast<CXXMethodDecl *>(Method), Quals));
> -  }
> -
> -  // Use a simplistic form of overload resolution to find the candidate.
> -  return GetBestOverloadCandidateSimple(Found);
> -}
> -
> -CXXMethodDecl *CXXRecordDecl::getMoveAssignmentOperator() const {
> -  for (method_iterator I = method_begin(), E = method_end(); I != E; ++I)
> -    if (I->isMoveAssignmentOperator())
> -      return *I;
> -
> -  return 0;
> -}
> -
>  void CXXRecordDecl::markedVirtualFunctionPure() {
>    // C++ [class.abstract]p2:
>    //   A class is abstract if it has at least one pure virtual function.
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=169670&r1=169669&r2=169670&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Dec  7 22:10:18 2012
> @@ -4687,12 +4687,28 @@
>      if (RD->hasUserDeclaredMoveConstructor() &&
>          (!getLangOpts().MicrosoftMode || CSM == CXXCopyConstructor)) {
>        if (!Diagnose) return true;
> -      UserDeclaredMove = RD->getMoveConstructor();
> +
> +      // Find any user-declared move constructor.
> +      for (CXXRecordDecl::ctor_iterator I = RD->ctor_begin(),
> +                                        E = RD->ctor_end(); I != E; ++I) {
> +        if (I->isMoveConstructor()) {
> +          UserDeclaredMove = *I;
> +          break;
> +        }
> +      }
>        assert(UserDeclaredMove);
>      } else if (RD->hasUserDeclaredMoveAssignment() &&
>                 (!getLangOpts().MicrosoftMode || CSM == CXXCopyAssignment)) {
>        if (!Diagnose) return true;
> -      UserDeclaredMove = RD->getMoveAssignmentOperator();
> +
> +      // Find any user-declared move assignment operator.
> +      for (CXXRecordDecl::method_iterator I = RD->method_begin(),
> +                                          E = RD->method_end(); I != E; ++I) {
> +        if (I->isMoveAssignmentOperator()) {
> +          UserDeclaredMove = *I;
> +          break;
> +        }
> +      }
>        assert(UserDeclaredMove);
>      }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-dev mailing list