[cfe-commits] [Patch] Implement compiler intrinsics for MSVC 2012 type_traits
Richard Smith
richard at metafoo.co.uk
Fri Nov 16 18:21:45 PST 2012
On Sun, Nov 11, 2012 at 10:38 AM, Ryan Molden <ryanmolden at gmail.com> wrote:
>
>
> On Fri, Nov 9, 2012 at 9:13 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Thu, Nov 8, 2012 at 5:49 PM, Ryan Molden <ryanmolden at gmail.com> wrote:
>> > This is a re-submission of an older proposed patch
>> >
>> > (http://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg55616/0001-Added-support-for-MSVC-2012-type-traits-used-in-stan.patch)
>> > that João hadn't had time to write tests for (which were requested with
>> > the
>> > original submission review).
>> >
>> > The only changes I made from the original (apart from adding tests) was
>> > to
>> > take out the bail-out for hasTrivialMoveAssignment from
>> > UTT_HasNothrowMoveAssign in EvaluateUnaryTypeTrait (in
>> > lib\Sema\SemaExprCXX.cpp).
>> >
>> > My reasoning was that trivial move assignment operators (which I
>> > understand
>> > to be implicitly generated ones, please correct me if this is mistaken)
>> > can
>> > actually have non-empty exception specifiers if any of the member
>> > move-assignment operators they invoke have such non-empty exception
>> > specifiers.
>> >
>> > Specifically:
>> >
>> > n3376 15.4 [except.spec]/14
>> >
>> > An inheriting constructor (12.9) and an implicitly declared special
>> > member
>> > function (Clause 12) have an exception-specification. If f is an
>> > inheriting
>> > constructor or an implicitly declared default constructor, copy
>> > constructor,
>> > move constructor, destructor, copy assignment operator, or move
>> > assignment
>> > operator, its implicit exception-specification specifies the type-id T
>> > if
>> > and only if T is allowed by the exception-specification of a function
>> > directly invoked by f’s implicit definition; f allows all exceptions if
>> > any
>> > function it directly invokes allows all exceptions, and f has the
>> > exception-specification noexcept(true) if every function it directly
>> > invokes
>> > allows no exceptions. [ Note: An instantiation of an inheriting
>> > constructor
>> > template has an implied exception-specification as if it were a
>> > non-template
>> > inheriting constructor.]
>> >
>> > so I would expect this class (HasMemberThrowMoveAssign) to fail for
>> > std::is_nothrow_move_assignable:
>> >
>> > struct NonPOD { NonPOD(int); }; enum Enum { EV }; struct POD { Enum e;
>> > int
>> > i; float f; NonPOD* p; };
>> >
>> > struct HasThrowMoveAssign { HasThrowMoveAssign& operator =(const
>> > HasThrowMoveAssign&&) throw(POD); };
>> > struct HasMemberThrowMoveAssign { HasThrowMoveAssign member; };
>> >
>> > even though it should have a trivial move-assignment operator generated.
>> > Please correct me if I am mistaken here as my standards reading FU
>> > is...not
>> > strong.
>>
>> You are mistaken here ;-)
>>
>> HasMemberThrowMoveAssign's move assignment is not trivial because it
>> calls a non-trivial move assignment operator. It is possible to have a
>> throwing trivial move assignment operator, but only if it is deleted.
>> In that case, the trait should presumbly return false.
>
>
> Great, thanks for the catch. So it seems that having the early bail-out for
> things with trivial move assignment operators is correct. I put it back in
> and all tests are still passing (I thought one of them was failing before I
> took it out, but that was a week+ ago, so perhaps I am just
> mis-remembering).
>
> It isn't clear how I would make a 'throwing trivial move assignment
> operator', if you have suggestions I can certainly add a test for it.
This class has a throwing trivial move assignment operator:
struct S {
S &operator=(const S&) throw(int) = delete;
};
> New patch attached that simply syncs to tip and adds back the bail-out for
> types with a trivial move-assignment operator.
I'm surprised that there's no __has_nothrow_move_constructor and
__has_nothrow_destructor. It's also surprising that we have
__has_trivial_copy for the copy ctor but
__has_trivial_move_constructor for the move ctor. I assume this
matches MSVC?
The patch itself generally looks fine, apart from some coding style issues:
Index: lib/AST/StmtPrinter.cpp
===================================================================
- case UTT_HasTrivialDefaultConstructor: return "__has_trivial_constructor";
+ case UTT_HasTrivialMoveAssign: return "__has_trivial_move_assign";
+ case UTT_HasTrivialMoveConstructor: return "__has_trivial_move_constructor";
+ case UTT_HasTrivialDefaultConstructor: return "__has_trivial_constructor";
Some trailing whitespace has been added here, please remove :)
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp (revision 167691)
+++ lib/Parse/ParseExpr.cpp (working copy)
@@ -1241,8 +1241,11 @@
case tok::kw___is_union:
case tok::kw___is_final:
case tok::kw___has_trivial_constructor:
+ case tok::kw___has_trivial_move_constructor:
case tok::kw___has_trivial_copy:
case tok::kw___has_trivial_assign:
+ case tok::kw___has_nothrow_move_assign:
+ case tok::kw___has_trivial_move_assign:
case tok::kw___has_trivial_destructor:
case tok::kw___has_nothrow_assign:
case tok::kw___has_nothrow_copy:
__has_nothrow_move_assign is not in the right order here.
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
@@ -3071,6 +3074,16 @@
C.getBaseElementType(T)->getAs<RecordType>())
return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialDefaultConstructor();
return false;
+ case UTT_HasTrivialMoveConstructor:
+ // This trait is implemented by MSVC 2012 and needed to parse the
+ // standard library headers. Specifically this is used as the logic
+ // behind std::has_trivial_move_constructor (20.9.4.3).
+ if (T.isPODType(Self.Context))
+ return true;
+ if (const RecordType *RT =
+ C.getBaseElementType(T)->getAs<RecordType>())
+ return cast<CXXRecordDecl>(RT->getDecl())->hasTrivialMoveConstructor();
+ return false;
Unnecessary line break here.
+ case UTT_HasNothrowMoveAssign:
+ // This trait is implemented by MSVC 2012 and needed to parse the
+ // standard library headers. Specifically this is used as the logic
+ // behind std::is_nothrow_move_assignable (20.9.4.3).
+ if (T.isPODType(Self.Context))
+ return true;
+
+ if (const RecordType *RT = C.getBaseElementType(T)->getAs<RecordType>()) {
+ CXXRecordDecl* RD = cast<CXXRecordDecl>(RT->getDecl());
+ if (RD->hasTrivialMoveAssignment())
+ return true;
+
+ bool FoundAssign = false;
+ DeclarationName Name = C.DeclarationNames.getCXXOperatorName(OO_Equal);
+ LookupResult Res(Self, DeclarationNameInfo(Name, KeyLoc),
Sema::LookupOrdinaryName);
+ if (Self.LookupQualifiedName(Res, RD)) {
+ Res.suppressDiagnostics();
+ for (LookupResult::iterator Op = Res.begin(), OpEnd =
Res.end(); Op != OpEnd; ++Op) {
+ if (isa<FunctionTemplateDecl>(*Op))
+ continue;
+
+ CXXMethodDecl *Operator = cast<CXXMethodDecl>(*Op);
+ if (Operator->isMoveAssignmentOperator()) {
+ FoundAssign = true;
+ const FunctionProtoType *CPT =
Operator->getType()->getAs<FunctionProtoType>();
+ CPT = Self.ResolveExceptionSpec(KeyLoc, CPT);
+ if (!CPT)
+ return false;
+ if (!CPT->isNothrow(Self.Context))
+ return false;
+ }
+ }
+ }
+
+ return FoundAssign;
+ }
+ return false;
Some lines here are over the 80 column limit.
More information about the cfe-commits
mailing list