[PATCH] D30448: [DebugInfo] Show implicit_const values when dumping .debug_info section

[David Blaikie via llvm-commits]

The usual reason to split initialization out of the ctor is if it's not
possible to initialize at the point of construction. (it looks like
DWARFormValue is used in this way - though even its current ctor (that
takes the form enum) has a non-ctor option (setForm - as seen in
DWARFDie::attribute_iterator::updateForIndex))

If DWARFFormValue is copy/movable, this ctor/init split could also, perhaps
more simply, be avoided (ie: use construction/move assignment to initialize
post-construction, rather than combinations of setForm/extract/etc). Though
it still looks like some parts of the codebase would require a bit more
refactoring if there were no way to split the "give the DWARFFormValue its
abbrev parts" from the "read the DIE parts using the abbrev parts". (see:
DWARFAcceleratorTable.cpp, specifically - though probably not too bad).

This is one of the other reasons not to special case here - it's not the
only place using DWARFFormValue, and with a special case here - those other
places will still be incorrect/not fully general. (indeed the code in
DWARFAbbreviationDeclaration has a special case already - if the API can be
refactored to make it not a special case we can remove that one and avoid
adding another one, as well as fixing the Accelerator Table case which
currently doesn't handle this)

(hmm, maybe the accelerator table format doesn't support this encoding
anyway, so it won't come up there)

I think the best thing is probably to have DWARFFormValue take either the
whole AttributeSpec, or at least a (new) FormSpec (which would be held in
the AttributeSpec, and contain the Form and SizeOrValue). Then it can use
that in extractValue to decide on the right thing to do.

If you want to change the API of DWARFFormValue (ensuring it supports move
assignment) and have a (probably named) ctor to take the AttributeSpec or
FormSpec, and the stuff from extractValue, and do the whole thing in
one-shot, that's OK too, I suppose. Not sure it really matters/changes
things very much, though.

- Dave

On Tue, Feb 28, 2017 at 10:13 AM Victor Leschuk <vleschuk at accesssoftek.com>
wrote:

>
>
> On 02/28/2017 08:58 PM, David Blaikie wrote:
>
>
>
> On Tue, Feb 28, 2017 at 9:55 AM Victor Leschuk <vleschuk at accesssoftek.com>
> wrote:
>
> Maybe we should just add one more constructor for DWARFFormValue which
> would set both Form and Value fields?
>
> That's /sort/ of what I'm suggesting, but providing a wrapper around that
> data so the code here doesn't have to have explicit support for values. It
> should be part of some 'form' abstraction/concept (that carries both the
> form enumerator and any other data is needed - the value).
>
> I understand, but isn't DWARFFormValue already such a wrapper itself? It
> holds enumerator and a union field which hides the actual data
> representation. We just need to widen it's API in order to be able
> construct it with one call, not construct+initialize.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170228/d4997284/attachment.html>