[PATCH] D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D126059#3532264 <https://reviews.llvm.org/D126059#3532264>, @clayborg wrote:

> A 1 to 2 percent improvement is probably going to go away if the asserts are enabled when it has to do the check each time.

Though the asserts would only be done in +Asserts builds - still providing the performance in release/non-asserts builds.

> If would still hesitate to add this kind of API as it will encourage others to use the APIs. I would be ok with it if the methods are protected or private and if we can add the DWARFLinker as a friend class to allow access to these performant but dangerous APIs only in certain situations that really require it. I really want to avoid most people from being able to use these APIs if possible to make sure we don't end up with bugs. We had many many bugs that stemmed from mismatching DWARFUnit + DWARFDebugIntoEntry before the DWARFDie class was created, mostly when doing cross CU references or in Fission cases where you have the skeleton CU vs the actual DWO CU + a DWARFDebugInfoEntry that comes from the actual DWO file. So there is a real danger to the safety and correctness of the code here.

Fair enough. I don't feel super strongly about it/sounds OK to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126059