[all-commits] [llvm/llvm-project] 918057: [lldb/DWARF] Don't start class definitions in Pars...

[Pavel Labath via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 918057c43aed90583eed5fe14450b2d75366b662
      https://github.com/llvm/llvm-project/commit/918057c43aed90583eed5fe14450b2d75366b662
  Author: Pavel Labath <pavel at labath.sk>
  Date:   2024-06-28 (Fri, 28 Jun 2024)

  Changed paths:
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

  Log Message:
  -----------
  [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (#96755)

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 (#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 #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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications