On Sun, Nov 25, 2012 at 8:11 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks for working on this!  Comments about the patch below:<br>
<br>
On Fri, Nov 23, 2012 at 2:40 PM, Ryan Molden <<a href="mailto:ryanmolden@gmail.com">ryanmolden@gmail.com</a>> wrote:<br>
> Index: lib/Sema/SemaExprCXX.cpp<br>
> ===================================================================<br>
> --- lib/Sema/SemaExprCXX.cpp (revision 168503)<br>
> +++ lib/Sema/SemaExprCXX.cpp (working copy)<br>
> @@ -2920,10 +2920,13 @@<br>
>      // type due to the overarching C++0x type predicates being implemented<br>
>      // requiring the complete type.<br>
>    case UTT_HasNothrowAssign:<br>
> +  case UTT_HasNothrowMoveAssign:<br>
>    case UTT_HasNothrowConstructor:<br>
>    case UTT_HasNothrowCopy:<br>
>    case UTT_HasTrivialAssign:<br>
> +  case UTT_HasTrivialMoveAssign:<br>
>    case UTT_HasTrivialDefaultConstructor:<br>
> +  case UTT_HasTrivialMoveConstructor:<br>
>    case UTT_HasTrivialCopy:<br>
>    case UTT_HasTrivialDestructor:<br>
>    case UTT_HasVirtualDestructor:<br>
> @@ -3071,6 +3074,15 @@<br>
<div class="im">>            C.getBaseElementType(T)->getAs<RecordType>())<br>
>        return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialDefaultConstructor();<br>
>      return false;<br>
> +  case UTT_HasTrivialMoveConstructor:<br>
> +    //  This trait is implemented by MSVC 2012 and needed to parse the<br>
> +    //  standard library headers. Specifically this is used as the logic<br>
> +    //  behind std::has_trivial_move_constructor (20.9.4.3).<br>
> +    if (T.isPODType(Self.Context))<br>
> +      return true;<br>
</div>> +    if (const RecordType *RT = C.getBaseElementType(T)->getAs<RecordType>())<br>
<div class="im">> +      return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialMoveConstructor();<br>
> +    return false;<br>
</div>>    case UTT_HasTrivialCopy:<br>
>      // <a href="http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html" target="_blank">http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html</a>:<br>
>      //   If __is_pod (type) is true or type is a reference type then<br>
> @@ -3082,6 +3094,15 @@<br>
>      if (const RecordType *RT = T->getAs<RecordType>())<br>
>        return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialCopyConstructor();<br>
>      return false;<br>
> +  case UTT_HasTrivialMoveAssign:<br>
<div class="im">> +    //  This trait is implemented by MSVC 2012 and needed to parse the<br>
</div>> +    //  standard library headers. Specifically it is used as the logic<br>
> +    //  behind std::is_trivially_move_assignable (20.9.4.3)<br>
<div class="im">> +    if (T.isPODType(Self.Context))<br>
> +      return true;<br>
</div>> +    if (const RecordType *RT = C.getBaseElementType(T)->getAs<RecordType>())<br>
> +      return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialMoveAssignment();<br>
> +    return false;<br>
>    case UTT_HasTrivialAssign:<br>
>      // <a href="http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html" target="_blank">http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html</a>:<br>
>      //   If type is const qualified or is a reference type then the<br>
> @@ -3169,6 +3190,46 @@<br>
>        return FoundAssign;<br>
>      }<br>
>      return false;<br>
<div class="im">> +  case UTT_HasNothrowMoveAssign:<br>
> +    //  This trait is implemented by MSVC 2012 and needed to parse the<br>
> +    //  standard library headers. Specifically this is used as the logic<br>
> +    //  behind std::is_nothrow_move_assignable (20.9.4.3).<br>
> +    if (T.isPODType(Self.Context))<br>
> +      return true;<br>
> +<br>
> +    if (const RecordType *RT = C.getBaseElementType(T)->getAs<RecordType>()) {<br>
> +      CXXRecordDecl* RD = cast<CXXRecordDecl>(RT->getDecl());<br>
> +      if (RD->hasTrivialMoveAssignment())<br>
> +        return true;<br>
> +<br>
> +      bool FoundAssign = false;<br>
> +      DeclarationName Name = C.DeclarationNames.getCXXOperatorName(OO_Equal);<br>
</div>> +      DeclarationNameInfo NameInfo(Name, KeyLoc);<br>
> +      LookupResult Res(Self, NameInfo, Sema::LookupOrdinaryName);<br>
<div class="im">> +      if (Self.LookupQualifiedName(Res, RD)) {<br>
> +        Res.suppressDiagnostics();<br>
> +        for (LookupResult::iterator Op = Res.begin(), OpEnd = Res.end();<br>
> +          Op != OpEnd; ++Op) {<br>
> +          if (isa<FunctionTemplateDecl>(*Op))<br>
> +            continue;<br>
> +<br>
> +          CXXMethodDecl *Operator = cast<CXXMethodDecl>(*Op);<br>
> +          if (Operator->isMoveAssignmentOperator()) {<br>
> +            FoundAssign = true;<br>
> +            const FunctionProtoType *CPT =<br>
</div>> +              Operator->getType()->getAs<FunctionProtoType>();<br>
<div class="im">> +            CPT = Self.ResolveExceptionSpec(KeyLoc, CPT);<br>
> +            if (!CPT)<br>
> +                return false;<br>
> +            if (!CPT->isNothrow(Self.Context))<br>
> +                return false;<br>
> +          }<br>
> +        }<br>
> +      }<br>
> +<br>
> +      return FoundAssign;<br>
<br>
</div>Very minor nit: you can move the FoundAssign logic into the<br>
LookupQualifiedName if block.<br>
<br>
Given the similarity between the other HasNothrow case statements, I<br>
wonder if there's a way we can consolidate this logic.  For instance,<br>
UTT_HasNothrowAssign looks to be identical with the exception of<br>
calling isCopyAssignmentOperator or isMoveAssignmentOperator.<br>
<br>
Otherwise, the patch LGTM.<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br><div>Something more like this? It isn't ultimately generic, but it does allow for re-use of the general logic between UTT_HasNoThrowAssign and UTT_HasNoThrowMoveAssign.</div><div>
<br></div><div>Ryan</div>