[Lldb-commits] [PATCH] D40214: performance: Prevent needless DWARFCompileUnit::Clear() on freshly ctor-ed object

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

See inline comments.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:74
                                lldb::offset_t *offset_ptr) {
-  Clear();
-
+  assert(m_offset == DW_INVALID_OFFSET);
   m_offset = *offset_ptr;
----------------
Make this static as mentioned in above comment. Assertions are ok to detect things in debug build, but please use lldbassert and make sure it returns an empty shared pointer if things fail (code must function properly when assert is not compiled in the program.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:34-36
+  // Object must be clear - either constructed now or after calling Clear().
   bool Extract(const lldb_private::DWARFDataExtractor &debug_info,
                lldb::offset_t *offset_ptr);
----------------
If you want to enforce this, then make this function static and have it return a shared pointer to a DWARFCompileUnit and make the constructor private. Commenting isn't enough on its own


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:294-297
       if (!cu.unique())
         cu.reset(new DWARFCompileUnit(dwarf2Data));
+      else
+        cu->Clear();
----------------
I don't see this code in top of tree? My DWARFDebugInfo.cpp ends at line 259


https://reviews.llvm.org/D40214