[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D79811#2035631 <https://reviews.llvm.org/D79811#2035631>, @clayborg wrote:

> In D79811#2035078 <https://reviews.llvm.org/D79811#2035078>, @shafik wrote:
>
> > In D79811#2034842 <https://reviews.llvm.org/D79811#2034842>, @clayborg wrote:
> >
> > > The error where two of the same classes conflicted usually only  happened in complex cases that were hard to reduce down.
> > >
> > > If I understand this correctly, the compiler should be able to auto synthesize these functions at will since the original class definition should have all of the information it needs to create them. So why do we need these to be added? Seems like the wrong approach IMHO. I would like the hear the justification for needing to add artificial functions to a class definition before we entertain this more. Can you elaborate on why you think these are needed?
> >
> >
> > Yes, the test case below shows that if we don't add the method then during expression evaluation of let's say:
> >
> >   A{}
> >
> >
> > The `default member initializer` won't be done. So `x` won't be initialized to `10` and the default constructor for `cse` will be called instead of `ClassForSideEffect(int)`.
>
>
> So that sounds like a compiler bug or something that we are not setting up right in the class type when we convert it from DWARF. Can you compile a quick example and dump the AST somehow and then compare it to the class we generate from debug info and see how they differ?


The DWARF will not contain the description of the `=10` part of `int x=10;` in the class definition. Without it, we don't have a way to know what the default constructor will do. It's true that for this variable, the compiler could emit `DW_AT_default_value(10)` to describe that, but the approach does not seem feasible in general -- the RHS can be an arbitrarily complex expression, referencing other functions, variables, etc.

It seems to me you're assigning more power to DW_AT_artificial than what it actually does. All that attribute says is that the entity was generated by the compiler ("Objects or types that are not actually declared in the source" in spec's own words). It does not mean that its semantics are fully described by the rest of the debug info. Since the compiler was able to generate that function, its semantics must have been fully described by the _source_ code, but dwarf also does not claim to describe the entirety of the source code.

Before c++11 came along with default initializers, it was mostly correct to say that "artificial" default constructors could be regenerated from dwarf at will -- because all that constructor could do is default-initialize all sub-objects. However, that is no longer the case...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79811/new/

https://reviews.llvm.org/D79811