This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/2] Class-ify ptid_t
On 04/06/2017 03:15 AM, Simon Marchi wrote:
> On 2017-04-05 17:31, Pedro Alves wrote:
>>>> It's probably going to be worth it to sprinkle "constexpr"
>>>> all over the new API. Helps with static_asserts in
>>>> unit testing too. *cough* :-)
>>>
>>> Ok, will look into it.
>>
>> Thanks. constexpr in C++11 forces you to write a single
>> return statement (C++14 gets rid of that requirement),
>> but it looks quite doable.
>>
>> Also, note that it's not true that this type can't have a
>> constructor. It can, as long as the type remains POD.
>
> Ah, so I was just missing the defaulted default constructor. Adding it
> makes the type trivial, which then makes it POD.
Right, almost. There's a couple other requirements beyond trivial,
but they're fulfilled as well. See:
http://en.cppreference.com/w/cpp/concept/PODType
>
>>> Is the following ok?
>>>
>>> struct thread_resume default_action { null_ptid };
>>
>> ISTR that in C++11 you'll need the double "{{" levels, like:
>>
>> thread_resume default_action {{null_ptid}};
>>
>> and that C++14 removed that requirement (brace elision).
>> But I haven't confirmed. Whatever works with -std=c++11.
>
> It seems to work with a single pair of braces with c++11. I'll still
> check that it does what we want at runtime, but I'd be surprised if it
> didn't do the right thing.
Sorry, yes, it's not necessary -- I somehow confused myself into
thinking that the current double "{{" was because the ptid field was
inside a structure that itself is inside the thread_resume structure.
struct thread_resume
{
struct something_else
{
ptid_t thread;
but "something_else" doesn't really exist... If it existed, then
the double {{ would be necessary in C++11 to initialize the sub field, but
not in C++. See brace elision here:
http://en.cppreference.com/w/cpp/language/aggregate_initialization
But now that I try, I can't make g++ nor clang++ error in c++11 with
those constructs. Maybe other compilers would, though. In any
case, the workaround would be trivial if we needed it -- just
add the equals sign.
>>
>>
>> +static_assert (std::is_pod<ptid_t>::value, "");
>> +
>> +static_assert (ptid_t(1).pid () == 1, "");
>
> Wow, nice. So all the tests are probably going to be static.
>
> Just to be clear, do you suggest that we make a test that ensures ptid_t
> is a POD, or you wrote that one just to show me it works? I We don't
> really care if it is, it's just that the current situation (it being
> used in another POD) requires it.
Yes, I think we should put that in the unit test with a comment
so that if someone tries to add something that would make it
non-pod, gdb won't even compile. If/when we get to the point where
we can/want to make it non-pod, we can remove the assertion then.
Thanks,
Pedro Alves