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

[Douglas Gregor]

On Jun 23, 2009, at 4:50 PM, Fariborz Jahanian wrote:

>
> On Jun 23, 2009, at 1:52 PM, Douglas Gregor wrote:
>
>>
>> On Jun 22, 2009, at 4:34 PM, Fariborz Jahanian wrote:
>>
>>> Author: fjahanian
>>> Date: Mon Jun 22 18:34:40 2009
>>> New Revision: 73922
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=73922&view=rev
>>> Log:
>>> patch to mark use of implicit copy constructors.
>>>
>>>
>>> Modified:
>>>  cfe/trunk/include/clang/AST/DeclCXX.h
>>>  cfe/trunk/lib/AST/DeclCXX.cpp
>>>  cfe/trunk/lib/Sema/Sema.h
>>>  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>>  cfe/trunk/lib/Sema/SemaExpr.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=73922&r1=73921&r2=73922&view=diff
>>>
>>> =
>>> ====================================================================
>>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Jun 22 18:34:40 2009
>>> @@ -283,6 +283,10 @@
>>> /// copy constructor that accepts a const-qualified argument.
>>> bool hasConstCopyConstructor(ASTContext &Context) const;
>>>
>>> +  /// getCopyConstructor - Returns the copy constructor for this  
>>> class
>>> +  CXXConstructorDecl *getCopyConstructor(ASTContext &Context,
>>> +                                         unsigned TypeQuals) const;
>>> +
>>> /// hasConstCopyAssignment - Determines whether this class has a
>>> /// copy assignment operator that accepts a const-qualified  
>>> argument.
>>> bool hasConstCopyAssignment(ASTContext &Context) const;
>>>
>>> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=73922&r1=73921&r2=73922&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
>>> +++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Jun 22 18:34:40 2009
>>> @@ -66,21 +66,29 @@
>>> }
>>>
>>> bool CXXRecordDecl::hasConstCopyConstructor(ASTContext &Context)  
>>> const {
>>> +  return getCopyConstructor(Context, QualType::Const) != 0;
>>> +}
>>> +
>>> +CXXConstructorDecl *CXXRecordDecl::getCopyConstructor(ASTContext  
>>> &Context,
>>> +                                                      unsigned  
>>> TypeQuals) const{
>>> QualType ClassType
>>>   = Context.getTypeDeclType(const_cast<CXXRecordDecl*>(this));
>>> DeclarationName ConstructorName
>>>   = Context.DeclarationNames.getCXXConstructorName(
>>> -                                            
>>> Context.getCanonicalType(ClassType));
>>> -  unsigned TypeQuals;
>>> +                                          Context.getCanonicalType 
>>> (ClassType));
>>> +  unsigned FoundTQs;
>>> DeclContext::lookup_const_iterator Con, ConEnd;
>>> for (llvm::tie(Con, ConEnd) = this->lookup(Context,  
>>> ConstructorName);
>>>      Con != ConEnd; ++Con) {
>>> -    if (cast<CXXConstructorDecl>(*Con)->isCopyConstructor 
>>> (Context, TypeQuals) &&
>>> -        (TypeQuals & QualType::Const) != 0)
>>> -      return true;
>>> +    if (cast<CXXConstructorDecl>(*Con)->isCopyConstructor(Context,
>>> +                                                           
>>> FoundTQs)) {
>>> +      if (((TypeQuals & QualType::Const) == (FoundTQs &  
>>> QualType::Const)) ||
>>> +          (!(TypeQuals & QualType::Const) && (FoundTQs &  
>>> QualType::Const)))
>>> +        return cast<CXXConstructorDecl>(*Con);
>>> +
>>> +    }
>>> }
>>> -
>>> -  return false;
>>> +  return 0;
>>> }
>>
>> This looks like it's doing overload resolution... why not just use  
>> the existing overload-resolution code in Sema instead? (Or am I  
>> misunderstanding the role of this code?).
>
> This is essentially what I am doing. For a current class's copy  
> ctor;  I am looking up a valid copy constructor
> of its base(s) and data members. So, for 'const X&' I am only  
> looking for 'const B&' and for 'X&' I am
> looking for either 'B&' or 'const B&'. I could be using  
> PerformInitializationByConstructor for this purpose.
> But it is overkill IMO and requires building and passing argument  
> expression which I don't have. But
> I am open for suggestion if  suitable API is available which does  
> not have the extra overhead.

We could build a DeclRefExpr that points to the ParmVarDecl in the  
constructor we're synthesizing. But, you are correct that this implies  
extra overhead, so I withdraw my suggestion. Thanks for the  
clarification!

> Other aspect of review is taken care of in:
> http://llvm.org/viewvc/llvm-project?view=rev&revision=74025

Thanks.

	- Doug