Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Pedro Alves <palves@redhat.com>
Cc: Simon Marchi <simon.marchi@ericsson.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Class-ify ptid_t
Date: Fri, 07 Apr 2017 03:21:00 -0000	[thread overview]
Message-ID: <d932282f1fc78977ccfdd1b017cdf8a2@polymtl.ca> (raw)
In-Reply-To: <c9500f9a-5358-2fc0-e75d-718e5a053da9@redhat.com>

On 2017-04-06 21:56, Pedro Alves wrote:
>> Ah, makes sense.  I was only thinking about the instances where ptid_t
>> is embedded in structures allocated statically.  In those cases,
>> compilation would fail anyway, which is why I didn't really see the
>> point of that test.
> 
> Actually, non-PODs in structures allocated statically is perfectly 
> fine.
> The compiler arranges for the objects to have their constructors
> called.  That's why we can e.g., have global std::vector objects
> or function local static std::string objects (both non-PODs), etc.
> 
> So we could e.g., have a default ptid_t ctor that initializes
> the pid/lwpid/tid fields to zeros instead of leaving them undefined.
> Except, if we did that, then the ctor would no longer be trivial, and
> so the type would no longer be POD, meaning we couldn't create a ptid
> with malloc and use it straight away.  We'd have to either call
> the ctor manually with placement new after malloc to bring to
> object to life, and call the dtor manually too (but that's off course
> cumbersome), or we'd have to use normal new/delete instead, which is
> the natural thing to do, but we're a bit far away from doing
> that everywhere.

Err sorry, I got confused.  The error I saw initially was about the 
target_waitstatus, where the ptid_t is in an union:

   /home/simark/src/binutils-gdb/gdb/target/waitstatus.h:104:8: note: 
‘target_waitstatus::target_waitstatus()’ is implicitly deleted because 
the default definition would be ill-formed:
  struct target_waitstatus
         ^~~~~~~~~~~~~~~~~
   /home/simark/src/binutils-gdb/gdb/target/waitstatus.h:104:8: error: 
use of deleted function ‘target_waitstatus::<anonymous 
union>::<constructor>()’

If you make a simple default constructor, like I tried in the beginning:

   ptid_t () {}

you'll see this error when building target.o.  I wrongfully attributed 
this error to the fact that target_waitstatus didn't have a constructor 
of its own, but I now see it doesn't make sense.  It must've been late 
:).


>> But of course, it's important for malloc'ed
>> structures as well, for which we get now error/warning.
> 
> Hmm, I'm not quite picturing what error/warning we now get?

Oops, s/now/no/.  What I meant is that if you use malloc for an object 
of a type you shouldn't (because it's not trivial), nothing will warn 
you.  The is_pod check protects us from that, since it will tell us if 
the class ever becomes unsafe to malloc.


  reply	other threads:[~2017-04-07  3:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-06 19:03 Simon Marchi
2017-04-06 22:23 ` Pedro Alves
2017-04-07  0:04   ` Simon Marchi
2017-04-07  1:56     ` Pedro Alves
2017-04-07  3:21       ` Simon Marchi [this message]
2017-04-07  3:34   ` [pushed] " Simon Marchi
2017-04-07  9:25 ` [PATCH v2] " Philipp Rudo
2017-04-07 10:48   ` Pedro Alves
2017-04-07 14:34     ` Simon Marchi
2017-04-07 14:35   ` [pushed] ptid-selftests: Fix erroneous assert messages Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d932282f1fc78977ccfdd1b017cdf8a2@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=simon.marchi@ericsson.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox