Skip to content

Commit

Permalink
[lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE
Browse files Browse the repository at this point in the history
Right now, ParseStructureLikeDIE begins the class definition (which
amounts to parsing the opening "{" of a class and promising to be able
to fill it in later) if it finds a definition DIE.

This makes sense in the current setup, where we eagerly search for the
definition die (so that we will either find it in the beginning or don't
find it at all), but with delayed definition searching (llvm#92328), this
created an (in my view, undesirable) inconsistency, where the final
state of the type (whether it has begun its definition) depended on
whether we happened to start out with a definition DIE or not.

This patch attempts to pre-emptively rectify that by establishing a new
invariant: the definition is never started eagerly. It can only be
started in one of two ways:
- we're completing the type, in which case we will start the definition,
  parse everything and immediately finish it
- we need to parse a member (typedef, nested class, method) of the class
  without needing the definition itself. In this case, we just start the
  definition to insert the member we need.

Besides the delayed definition search, I believe this setup has a couple
of other benefits:
- It treats ObjC and C++ classes the same way (we were never starting
  the definition of those)
- unifies the handling of types that types that have a definition and
  those that do. When adding (e.g.) a nested class we would previously
  be going down a different code path depending on whether we've found a
  definition DIE for that type. Now, we're always taking the
  definition-not-found path (*)
- it reduces the amount of time a class spends in the funny "definition
  started". Aside from the addition of stray addition of nested classes,
  we always finish the definition right after we start it.

(*) Herein lies a danger, where if we're missing some calls to
    PrepareContextToReceiveMembers, we could trigger a crash when
    trying to add a member to the not-yet-started-to-be-defined classes.
    However, this is something that could happen before as well (if we
    did not have a definition for the class), and is something that
    would be exacerbated by llvm#92328 (because it could happen even if we
    the definition exists, but we haven't found it yet). This way, it
    will at least happen consistently, and the fix should consist of
    adding a PrepareContextToReceiveMembers in the appropriate place.
  • Loading branch information
labath committed Jun 26, 2024
1 parent f782ff8 commit a9076e2
Showing 1 changed file with 144 additions and 234 deletions.
Loading

0 comments on commit a9076e2

Please sign in to comment.