[cfe-commits] [Patch] Implement compiler intrinsics for MSVC 2012 type_traits
Ryan Molden
ryanmolden at gmail.com
Fri Nov 23 11:40:52 PST 2012
On Sat, Nov 17, 2012 at 11:05 AM, Ryan Molden <ryanmolden at gmail.com> wrote:
> On Fri, Nov 16, 2012 at 6:21 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> 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.
>>
>
> >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?
>
> You and your 'we should be exhaustive in our coverage' :) I believe there
> are more, the ones above were simply the ones I ran into while trying to
> use Clang to process some code. I believe the ones you mention (like
> __has_nothrow_move_constructor) are currently hiding behind other errors
> (specifically Clang's intense dislike of the VC STL variadac template
> 'emulating' macros). I will try and do a search of the type_traits header
> looking for all the compiler intrinsics they rely on and cross-referencing
> with what Clang currently has, adding any that aren't there.
>
> I will also address the other comments, thanks much!
>
> Ryan
>
Okay, I was mistaken, it looks like Clang covers all intrinsics necessary
(after my patch). The spreadsheet is here (
https://docs.google.com/spreadsheet/ccc?key=0Avvye004lP64dG5jU1ZkWWpLM3ZqOVlWYWgtV1RBV3c)
with my investigative results on this.
There are a couple of type traits that MSVC 2012 has marked as
approximations. Currently these don't require intrinsics, eventually I
suspect they will, but since we don't know for sure what MSVC will call
them it doesn't seem to make sense to try and pre-emptively include them,
so I am not going to bother there.
>I'm surprised that there's no __has_nothrow_move_constructor
they don't have a __has_nothrow_move_constructor, I presume it would be
used for something like std::is_nothrow_move_constructible, but that is
defined in their headers as:
// TEMPLATE CLASS is_nothrow_move_constructible
template<class _Ty>
struct is_nothrow_move_constructible
: _Cat_base<!is_array<_Ty>::value
&& is_nothrow_constructible<
typename remove_volatile<_Ty>::type,
typename add_rvalue_reference<
typename remove_volatile<_Ty>::type>::type>::value>
{ // determine whether _Ty has a nothrow move constructor
};
and is_nothrow_constructible utilizes __has_nothrow_constructor, which
already existed in Clang before this patch.
>and __has_nothrow_destructor.
There is_nothrow_destructible is marked as approximation and is set to
derive from true_type at the moment. They will likely need something like
__has_nothrow_destructor to implement this fully, but until they release an
STL relying on that intrinsic adding it to Clang seems premature.
>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?
__has_trivial_move_constructor is the instrinc that MSVC STL expects to
exist.
It looks like __has_trivial_copy is likely for GNU supports, not MSVC. MSVC
relies on __is_trivially_copyable intrinsic, which existed in Clang before
this changeset.
New patch attached, cleanup various formatting issues from above + add some
more unit tests around deleted functions.
Ryan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121123/2149e416/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Support-For-MSVC-2012-Type-Traits-For-STL.patch
Type: application/octet-stream
Size: 17742 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121123/2149e416/attachment.obj>
More information about the cfe-commits
mailing list