labath added a comment.
I just realized a different problem with this approach. Since the constructor
DIE is only emitted when it is actually used, we could run into
problems/inconsistencies even within a single library. We currently parse only
one instance of a class description -- assuming all of them to be equivalent.
This means that if we happen to pick the class description without the
constructor as *the* definition for the class, then the we would still fall
back to the (incorrect) constructor that clang generates for us. In this test
case that could be demonstrated by having another compile unit, which only uses
`ClassWithImplicitCtor`, but never constructs it, and then arranging it such
that this file is picked for the class definition (probably involves putting
this file first on the link line, or ensuring the first breakpoint hit is in
that file).
Fixing that is going to be tricky. The best I can come up with right now, is
injecting a constructor declaration into a class even if DWARF does not
describe one. Then, if the user calls it, we go searching for the constructor
symbol in the module. If we find one, we use it. If we don't, we bail out of
the expression.
I think that approach would result in the most correct behavior, and it would
also solve the structural equivalence issues that Greg is worried about (all
classes would have the default constructor). However, it does have some serious
drawbacks: The presence of a user-specified constructor (which is how this will
appear to clang) disables certain ways of initializing the class. So for
instance, the user would not be able to do this,
code:
struct Simple { int a, b, c; };
lldb:
p Simple() # value-initialized to zero
p Simple{47} # compound initialization, a=47, b=c=0
p Simple{1, 2, 3} # compount initalization with all members
That is kind of correct, since we're actually not able to differentiate the
above class, from a class like this `struct SeeminglySimple { int a = random(),
b = random(), c = random(); };`. However, I have a feeling it would annoy a lot
of people. The best we could do is make the third expression work by
synthesizing an `(int, int, int)` constructor initializing all members, as that
will not use the default initializers (if they existed).
I have also considered the idea of trawling through all definitions of the
class, in search of the default constructor. However:
- it's possible that no compile unit uses it, but the class still contained
default initializers. Then, we will still get the initialization wrong.
- it won't help with partial compound initialization (second expression)
- it will slow us down (but maybe not so much with indexes, as we know the
constructor name)
So, it does seem like some help from the compiler is called for. Maybe the
compiler could attach a `DW_AT_default_value` with an empty expression to the
members which have default initializers, as a way of saying "this member has a
default initializer, but I can't/won't tell you what it is" ? Then we could
search for this attribute and refuse constructing classes which have it. That
would enable us to keep `p Simple{}` working, while preventing `p
SeeminglySimple{}`.
In D79811#2037636 <https://reviews.llvm.org/D79811#2037636>, @dblaikie wrote:
> In D79811#2036112 <https://reviews.llvm.org/D79811#2036112>, @labath wrote:
>
> > This example also demonstrates what I believe *is* a bug in the compiler.
> > The inherited constructor will get `DW_AT_name(A)`:
> >
> > 0x0000003f: DW_TAG_structure_type
> > DW_AT_calling_convention (DW_CC_pass_by_value)
> > DW_AT_name ("B")
> > DW_AT_byte_size (0x01)
> > DW_AT_decl_file ("/home/pavelo/ll/build/opt/<stdin>")
> > DW_AT_decl_line (1)
> >
> > 0x00000048: DW_TAG_inheritance
> > DW_AT_type (0x0000005f "A")
> > DW_AT_data_member_location (0x00)
> >
> > 0x0000004e: DW_TAG_subprogram
> > DW_AT_name ("A")
> > DW_AT_declaration (true)
> > DW_AT_artificial (true)
> > DW_AT_external (true)
> >
> >
> > That doesn't sound correct to me. Gcc emits the name as `B`, which seems to
> > be much better. Looping in @dblaikie for thoughts.
>
>
> Agreed - sounds like a bug/something to fix in Clang there - if you'd like to
> fix it/file a bug/etc.
Yeah, I guess should do that, before we start needing to work around this bug
for compatibility purposes. :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79811/new/
https://reviews.llvm.org/D79811
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
