clayborg added a comment.
Clang AST contexts know how to complete types and is done via the external AST
source code that will ask a type to complete itself. Each object file has an
AST that knows how to lazily complete a type when and only when it is needed.
Each object file also only knows about the type information in the binary
itself. So if we have a forward declaration to "Foo" with something like
"struct Foo;" that is all the object file AST will ever know about this type.
This is required because each module can be re-used on subsequent debug
sessions if they haven't changed. So if we have a forward declaration for "Foo"
in the AST for "bbb.so" that is ok. We don't want to copy some definition for
"Foo" from "foo.so" over into bbb.so's AST context because if we run again and
we get a new foo.so we would have to reload bbb.so because its copy of "Foo"
might be out of date. And we would need to track these interactions.
When we run expressions, we create a new AST and copy types as needed. It would
be great if the AST importer only copy over forward declarations of types that
can be completed later and can also complete types only as needed when asked.
If I understand correctly that is what this patch is trying to do. Seems like
we have a code path that is copying over the type and also completing it
sometimes without being asked which should be fixed. If we do fix this, complex
expressions become a lot faster. To do this right we should always import
forward declarations from the source, and be able to complete the new types in
the destination as needed. As teemperor said, the source AST should not be
mutated in any way. We should track all of this in the importer and know where
we should try to complete the type from.
When using expression AST contexts it **is** ok to try and import "Foo" from
bbb.so since that where is where we first saw the type, and if we aren't
successful, we can grab the definition from anywhere else in the debug session.
Since each expression has its own AST, it **is** ok to get the type from
anywhere. When searching for this type we should start in the current
lldb_private::Block, their pareent blocks, then the file, then the module and
then all modules. I think that works today already, but I am not sure if this
works for a type "Foo" that is mentioned in a type from a file that doesn't
have a complete definition. for example if bbb.so contains:
struct Bar : public Foo {...};
Due to "-flimit-debug-info" the definition for Foo might be forward declared
(if the vtable for Foo isn't in the current binary) and not included in this
binary. This won't happen on darwin since the default is
-fno-limit-debug-info". The DWARF parser knows how to work around this issue
when creating the type in the AST for bbb.so, but when we run an expression
with this type, we want to be able to have an AST type from bbb.so with an
incomplete definition for "Foo" that we complete during AST import. To do this,
we will need to use metadata in the bbb.so AST to indicate we have no
definition for this type when we normally would require one and be able to
complete the type from another source.
So quick things to stick to:
- no modification of the source AST context
- importer can track anything it needs to in order to complete types in complex
situations as mentioned above
- need metadata that tracks types that need to be complete but aren't in the
debug info so they can be properly imported for expressions ("struct Bar:
public Foo {}", not ok for Foo to not be complete but we allow it for object
file AST contexts otherwise clang crashes us)
- legal forward declarations should be able to be imported as needed even if
the AST form the original source doesn't have a complete type ("struct Bar {
Foo *foo_ptr; }", ok for Foo to be forward declared here)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69933/new/
https://reviews.llvm.org/D69933
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
