[PATCH] D26526: Clean up DWARFFormValue by reducing duplicated code and removing DWARFFormValue::getFixedFormSizes()

[David Blaikie via llvm-commits]

On Thu, Nov 10, 2016 at 6:01 PM Greg Clayton <clayborg at gmail.com> wrote:

> It can be NULL so it needs to stay a pointer. An upcoming change will call
> DWARFFormValue::getFixedByteSize() for each form in an abbreviation with a
> NULL DWARFUnit and expect to get None back for anything that can change
> size due to version, addrsize or DWARF32/DWARF64.
>

Rightio - usually we add functionality when it's needed/live, so we can
properly assess the test coverage, etc. But as you see fit.


>
> > On Nov 10, 2016, at 4:27 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Sure - still feels a bit like overengineering, but anyway.
> >
> > The parameter should be const ref, rather than pointer (it can't be
> null, so prefer a reference) & the template parameter type could perhaps
> have a more meaningful name like 'FormSizes' or somesuch.
> >
> > On Thu, Nov 10, 2016 at 4:06 PM Greg Clayton <clayborg at gmail.com> wrote:
> > David,
> >
> > Take a look at the latest changes I posted. I think this will solve the
> issues we were both worried about.
> >
> > https://reviews.llvm.org/D26526
> >
> > Let me know what you think.
> >
> > Greg
> >
> >
> >
> > > On Nov 10, 2016, at 3:39 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > >
> > >
> > >
> > > On Thu, Nov 10, 2016 at 3:34 PM Greg Clayton <clayborg at gmail.com>
> wrote:
> > > clayborg added a comment.
> > >
> > > In order to skip a form to access the 10th attribute, we need to skip
> 9 forms. Each one of those will individually extract 3 piece of data for
> which 2 surely never be used and 1 might be used 10% of the time. So it
> seems it would be worth being lazy if we can especially since it is so easy
> to do.
> > >
> > > Still just seems a bit premature to me - you'll still have to
> initialize the vptr every time through there, for example, and when you do
> access it it'll be dynamic calls and all that.
> > >
> > > In order of preference:
> > >
> > > 1) just punt on the whole thing & gather the three values up-front
> > > 2) make it a template (if performance is this important here - let's
> avoid the virtual calls, even if it's on relatively few forms)
> > > 3) as-is, except with the dtor non-virtual (protected in the base
> class, derived classes final)
> > >
> > > I'll leave it to you & Adrian, though - just my 2c.
> > >
> > > - Dave
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161111/ad5f2f2e/attachment.html>