[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Don't overwrite DW_AT_object_pointer of definition with that of a declaration (PR #123089)

[Pavel Labath via lldb-commits]

================
@@ -377,7 +377,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
       break;
 
     case DW_AT_object_pointer:
-      object_pointer = form_value.Reference();
+      // GetAttributes follows DW_AT_specification.
+      // DW_TAG_subprogram definitions and declarations may both
+      // have a DW_AT_object_pointer. Don't overwrite the one
+      // we parsed for the definition with the one from the declaration.
----------------
labath wrote:

Could be, but you also could be reading too much into it. There are other (better) ways to say that the attribute will appear only once and "take *precedence*" could have been meant as "it will *precede* the other attribute in the list" (It's probably not valid English, but I can see someone might say that, and it sort matches what the code does).

That said, I think that omitting the second instance of the attribute should not be a problem functionality-wise, although I am slightly concerned about the performance impact (I guess you'd have to iterate through the list to see if the attribute is not already there). This function is called from the manual dwarf index so it is somewhat performance sensitive. If you want to go down that path, I'd like to measure the impact on indexing speed.

https://github.com/llvm/llvm-project/pull/123089