[DWARF] Generalize DWARFTypePrinter to a template class #109459
Merged
+751
−697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-debuginfo Author: Zequan Wu (ZequanWu) ChangesThis generalizes DWARFTypePrinter class to a template class so that it can be reused for lldb's DWARFDIE type. This is a split of #90008. The difference is that this doesn't have Patch is 49.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109459.diff 4 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index e05271740e6157..5315ac194f8ede 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -9,9 +9,10 @@
#ifndef LLVM_DEBUGINFO_DWARF_DWARFTYPEPRINTER_H
#define LLVM_DEBUGINFO_DWARF_DWARFTYPEPRINTER_H
+#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Dwarf.h"
-#include "llvm/DebugInfo/DWARF/DWARFDie.h"
+#include "llvm/DebugInfo/DWARF/DWARFUnit.h"
#include <string>
@@ -19,9 +20,55 @@ namespace llvm {
class raw_ostream;
+inline std::optional<uint64_t> getLanguage(DWARFDie D) {
+ if (std::optional<DWARFFormValue> LV =
+ D.getDwarfUnit()->getUnitDIE().find(dwarf::DW_AT_language))
+ return LV->getAsUnsignedConstant();
+ return std::nullopt;
+}
+
+namespace detail {
+inline llvm::SmallString<128> toString(const llvm::format_object_base &Fmt) {
+ size_t NextBufferSize = 127;
+ llvm::SmallString<128> V;
+
+ while (true) {
+ V.resize_for_overwrite(NextBufferSize);
+
+ // Try formatting into the SmallVector.
+ size_t BytesUsed = Fmt.print(V.data(), NextBufferSize);
+
+ // If BytesUsed fit into the vector, we win.
+ if (BytesUsed <= NextBufferSize) {
+ V.resize(BytesUsed);
+ return V;
+ }
+
+ // Otherwise, try again with a new size.
+ assert(BytesUsed > NextBufferSize && "Didn't grow buffer!?");
+ NextBufferSize = BytesUsed;
+ }
+}
+
+/// Returns True if the DIE TAG is one of the ones that is scopped.
+inline bool scopedTAGs(dwarf::Tag Tag) {
+ switch (Tag) {
+ case dwarf::DW_TAG_structure_type:
+ case dwarf::DW_TAG_class_type:
+ case dwarf::DW_TAG_union_type:
+ case dwarf::DW_TAG_namespace:
+ case dwarf::DW_TAG_enumeration_type:
+ return true;
+ default:
+ break;
+ }
+ return false;
+}
+} // namespace detail
+
// FIXME: We should have pretty printers per language. Currently we print
// everything as if it was C++ and fall back to the TAG type name.
-struct DWARFTypePrinter {
+template <typename DieType> struct DWARFTypePrinter {
raw_ostream &OS;
bool Word = true;
bool EndedWithTemplate = false;
@@ -31,37 +78,738 @@ struct DWARFTypePrinter {
/// Dump the name encoded in the type tag.
void appendTypeTagName(dwarf::Tag T);
- void appendArrayType(const DWARFDie &D);
+ void appendArrayType(const DieType &D);
- DWARFDie skipQualifiers(DWARFDie D);
+ DieType skipQualifiers(DieType D);
- bool needsParens(DWARFDie D);
+ bool needsParens(DieType D);
- void appendPointerLikeTypeBefore(DWARFDie D, DWARFDie Inner, StringRef Ptr);
+ void appendPointerLikeTypeBefore(DieType D, DieType Inner, StringRef Ptr);
- DWARFDie appendUnqualifiedNameBefore(DWARFDie D,
- std::string *OriginalFullName = nullptr);
+ DieType appendUnqualifiedNameBefore(DieType D,
+ std::string *OriginalFullName = nullptr);
- void appendUnqualifiedNameAfter(DWARFDie D, DWARFDie Inner,
+ void appendUnqualifiedNameAfter(DieType D, DieType Inner,
bool SkipFirstParamIfArtificial = false);
- void appendQualifiedName(DWARFDie D);
- DWARFDie appendQualifiedNameBefore(DWARFDie D);
- bool appendTemplateParameters(DWARFDie D, bool *FirstParameter = nullptr);
- void decomposeConstVolatile(DWARFDie &N, DWARFDie &T, DWARFDie &C,
- DWARFDie &V);
- void appendConstVolatileQualifierAfter(DWARFDie N);
- void appendConstVolatileQualifierBefore(DWARFDie N);
+ void appendQualifiedName(DieType D);
+ DieType appendQualifiedNameBefore(DieType D);
+ bool appendTemplateParameters(DieType D, bool *FirstParameter = nullptr);
+ void appendAndTerminateTemplateParameters(DieType D);
+ void decomposeConstVolatile(DieType &N, DieType &T, DieType &C, DieType &V);
+ void appendConstVolatileQualifierAfter(DieType N);
+ void appendConstVolatileQualifierBefore(DieType N);
/// Recursively append the DIE type name when applicable.
- void appendUnqualifiedName(DWARFDie D,
+ void appendUnqualifiedName(DieType D,
std::string *OriginalFullName = nullptr);
- void appendSubroutineNameAfter(DWARFDie D, DWARFDie Inner,
+ void appendSubroutineNameAfter(DieType D, DieType Inner,
bool SkipFirstParamIfArtificial, bool Const,
bool Volatile);
- void appendScopes(DWARFDie D);
+ void appendScopes(DieType D);
};
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendTypeTagName(dwarf::Tag T) {
+ StringRef TagStr = TagString(T);
+ static constexpr StringRef Prefix = "DW_TAG_";
+ static constexpr StringRef Suffix = "_type";
+ if (!TagStr.starts_with(Prefix) || !TagStr.ends_with(Suffix))
+ return;
+ OS << TagStr.substr(Prefix.size(),
+ TagStr.size() - (Prefix.size() + Suffix.size()))
+ << " ";
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendArrayType(const DieType &D) {
+ for (const DieType &C : D.children()) {
+ if (C.getTag() != dwarf::DW_TAG_subrange_type)
+ continue;
+ std::optional<uint64_t> LB;
+ std::optional<uint64_t> Count;
+ std::optional<uint64_t> UB;
+ std::optional<unsigned> DefaultLB;
+ if (std::optional<DWARFFormValue> L = C.find(dwarf::DW_AT_lower_bound))
+ LB = L->getAsUnsignedConstant();
+ if (std::optional<DWARFFormValue> CountV = C.find(dwarf::DW_AT_count))
+ Count = CountV->getAsUnsignedConstant();
+ if (std::optional<DWARFFormValue> UpperV = C.find(dwarf::DW_AT_upper_bound))
+ UB = UpperV->getAsUnsignedConstant();
+ if (std::optional<DWARFFormValue> LV =
+ D.getDwarfUnit()->getUnitDIE().find(dwarf::DW_AT_language))
+ if (std::optional<uint64_t> LC = LV->getAsUnsignedConstant())
+ if ((DefaultLB =
+ LanguageLowerBound(static_cast<dwarf::SourceLanguage>(*LC))))
+ if (LB && *LB == *DefaultLB)
+ LB = std::nullopt;
+ if (!LB && !Count && !UB)
+ OS << "[]";
+ else if (!LB && (Count || UB) && DefaultLB)
+ OS << '[' << (Count ? *Count : *UB - *DefaultLB + 1) << ']';
+ else {
+ OS << "[[";
+ if (LB)
+ OS << *LB;
+ else
+ OS << '?';
+ OS << ", ";
+ if (Count)
+ if (LB)
+ OS << *LB + *Count;
+ else
+ OS << "? + " << *Count;
+ else if (UB)
+ OS << *UB + 1;
+ else
+ OS << '?';
+ OS << ")]";
+ }
+ }
+ EndedWithTemplate = false;
+}
+
+namespace detail {
+template <typename DieType>
+DieType resolveReferencedType(DieType D,
+ dwarf::Attribute Attr = dwarf::DW_AT_type) {
+ return D.getAttributeValueAsReferencedDie(Attr).resolveTypeUnitReference();
+}
+template <typename DieType>
+DieType resolveReferencedType(DieType D, DWARFFormValue F) {
+ return D.getAttributeValueAsReferencedDie(F).resolveTypeUnitReference();
+}
+} // namespace detail
+
+template <typename DieType>
+DieType DWARFTypePrinter<DieType>::skipQualifiers(DieType D) {
+ while (D && (D.getTag() == dwarf::DW_TAG_const_type ||
+ D.getTag() == dwarf::DW_TAG_volatile_type))
+ D = detail::resolveReferencedType(D);
+ return D;
+}
+
+template <typename DieType>
+bool DWARFTypePrinter<DieType>::needsParens(DieType D) {
+ D = skipQualifiers(D);
+ return D && (D.getTag() == dwarf::DW_TAG_subroutine_type ||
+ D.getTag() == dwarf::DW_TAG_array_type);
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendPointerLikeTypeBefore(DieType D,
+ DieType Inner,
+ StringRef Ptr) {
+ appendQualifiedNameBefore(Inner);
+ if (Word)
+ OS << ' ';
+ if (needsParens(Inner))
+ OS << '(';
+ OS << Ptr;
+ Word = false;
+ EndedWithTemplate = false;
+}
+
+template <typename DieType>
+DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
+ DieType D, std::string *OriginalFullName) {
+ Word = true;
+ if (!D) {
+ OS << "void";
+ return DieType();
+ }
+ DieType InnerDIE;
+ auto Inner = [&] { return InnerDIE = detail::resolveReferencedType(D); };
+ const dwarf::Tag T = D.getTag();
+ switch (T) {
+ case dwarf::DW_TAG_pointer_type: {
+ appendPointerLikeTypeBefore(D, Inner(), "*");
+ break;
+ }
+ case dwarf::DW_TAG_subroutine_type: {
+ appendQualifiedNameBefore(Inner());
+ if (Word) {
+ OS << ' ';
+ }
+ Word = false;
+ break;
+ }
+ case dwarf::DW_TAG_array_type: {
+ appendQualifiedNameBefore(Inner());
+ break;
+ }
+ case dwarf::DW_TAG_reference_type:
+ appendPointerLikeTypeBefore(D, Inner(), "&");
+ break;
+ case dwarf::DW_TAG_rvalue_reference_type:
+ appendPointerLikeTypeBefore(D, Inner(), "&&");
+ break;
+ case dwarf::DW_TAG_ptr_to_member_type: {
+ appendQualifiedNameBefore(Inner());
+ if (needsParens(InnerDIE))
+ OS << '(';
+ else if (Word)
+ OS << ' ';
+ if (DieType Cont =
+ detail::resolveReferencedType(D, dwarf::DW_AT_containing_type)) {
+ appendQualifiedName(Cont);
+ EndedWithTemplate = false;
+ OS << "::";
+ }
+ OS << "*";
+ Word = false;
+ break;
+ }
+ case dwarf::DW_TAG_LLVM_ptrauth_type:
+ appendQualifiedNameBefore(Inner());
+ break;
+ case dwarf::DW_TAG_const_type:
+ case dwarf::DW_TAG_volatile_type:
+ appendConstVolatileQualifierBefore(D);
+ break;
+ case dwarf::DW_TAG_namespace: {
+ if (const char *Name = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr))
+ OS << Name;
+ else
+ OS << "(anonymous namespace)";
+ break;
+ }
+ case dwarf::DW_TAG_unspecified_type: {
+ StringRef TypeName = D.getShortName();
+ if (TypeName == "decltype(nullptr)")
+ TypeName = "std::nullptr_t";
+ Word = true;
+ OS << TypeName;
+ EndedWithTemplate = false;
+ break;
+ }
+ /*
+ case DW_TAG_structure_type:
+ case DW_TAG_class_type:
+ case DW_TAG_enumeration_type:
+ case DW_TAG_base_type:
+ */
+ default: {
+ const char *NamePtr = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr);
+ if (!NamePtr) {
+ appendTypeTagName(D.getTag());
+ return DieType();
+ }
+ Word = true;
+ StringRef Name = NamePtr;
+ static constexpr StringRef MangledPrefix = "_STN|";
+ if (Name.consume_front(MangledPrefix)) {
+ auto Separator = Name.find('|');
+ assert(Separator != StringRef::npos);
+ StringRef BaseName = Name.substr(0, Separator);
+ StringRef TemplateArgs = Name.substr(Separator + 1);
+ if (OriginalFullName)
+ *OriginalFullName = (BaseName + TemplateArgs).str();
+ Name = BaseName;
+ } else
+ EndedWithTemplate = Name.ends_with(">");
+ OS << Name;
+ // This check would be insufficient for operator overloads like
+ // "operator>>" - but for now Clang doesn't try to simplify them, so this
+ // is OK. Add more nuanced operator overload handling here if/when needed.
+ if (Name.ends_with(">"))
+ break;
+ if (!appendTemplateParameters(D))
+ break;
+
+ if (EndedWithTemplate)
+ OS << ' ';
+ OS << '>';
+ EndedWithTemplate = true;
+ Word = true;
+ break;
+ }
+ }
+ return InnerDIE;
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendUnqualifiedNameAfter(
+ DieType D, DieType Inner, bool SkipFirstParamIfArtificial) {
+ if (!D)
+ return;
+ switch (D.getTag()) {
+ case dwarf::DW_TAG_subroutine_type: {
+ appendSubroutineNameAfter(D, Inner, SkipFirstParamIfArtificial, false,
+ false);
+ break;
+ }
+ case dwarf::DW_TAG_array_type: {
+ appendArrayType(D);
+ break;
+ }
+ case dwarf::DW_TAG_const_type:
+ case dwarf::DW_TAG_volatile_type:
+ appendConstVolatileQualifierAfter(D);
+ break;
+ case dwarf::DW_TAG_ptr_to_member_type:
+ case dwarf::DW_TAG_reference_type:
+ case dwarf::DW_TAG_rvalue_reference_type:
+ case dwarf::DW_TAG_pointer_type: {
+ if (needsParens(Inner))
+ OS << ')';
+ appendUnqualifiedNameAfter(Inner, detail::resolveReferencedType(Inner),
+ /*SkipFirstParamIfArtificial=*/D.getTag() ==
+ dwarf::DW_TAG_ptr_to_member_type);
+ break;
+ }
+ case dwarf::DW_TAG_LLVM_ptrauth_type: {
+ auto getValOrNull = [&](dwarf::Attribute Attr) -> uint64_t {
+ if (auto Form = D.find(Attr))
+ return *Form->getAsUnsignedConstant();
+ return 0;
+ };
+ SmallVector<const char *, 2> optionsVec;
+ if (getValOrNull(dwarf::DW_AT_LLVM_ptrauth_isa_pointer))
+ optionsVec.push_back("isa-pointer");
+ if (getValOrNull(dwarf::DW_AT_LLVM_ptrauth_authenticates_null_values))
+ optionsVec.push_back("authenticates-null-values");
+ if (auto AuthenticationMode =
+ D.find(dwarf::DW_AT_LLVM_ptrauth_authentication_mode)) {
+ switch (*AuthenticationMode->getAsUnsignedConstant()) {
+ case 0:
+ case 1:
+ optionsVec.push_back("strip");
+ break;
+ case 2:
+ optionsVec.push_back("sign-and-strip");
+ break;
+ default:
+ // Default authentication policy
+ break;
+ }
+ }
+ std::string options;
+ for (const auto *option : optionsVec) {
+ if (options.size())
+ options += ",";
+ options += option;
+ }
+ if (options.size())
+ options = ", \"" + options + "\"";
+ std::string PtrauthString;
+ llvm::raw_string_ostream PtrauthStream(PtrauthString);
+ PtrauthStream
+ << "__ptrauth(" << getValOrNull(dwarf::DW_AT_LLVM_ptrauth_key) << ", "
+ << getValOrNull(dwarf::DW_AT_LLVM_ptrauth_address_discriminated)
+ << ", 0x0"
+ << utohexstr(
+ getValOrNull(dwarf::DW_AT_LLVM_ptrauth_extra_discriminator),
+ true)
+ << options << ")";
+ OS << PtrauthStream.str();
+ break;
+ }
+ /*
+ case DW_TAG_structure_type:
+ case DW_TAG_class_type:
+ case DW_TAG_enumeration_type:
+ case DW_TAG_base_type:
+ case DW_TAG_namespace:
+ */
+ default:
+ break;
+ }
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendQualifiedName(DieType D) {
+ if (D && detail::scopedTAGs(D.getTag()))
+ appendScopes(D.getParent());
+ appendUnqualifiedName(D);
+}
+
+template <typename DieType>
+DieType DWARFTypePrinter<DieType>::appendQualifiedNameBefore(DieType D) {
+ if (D && detail::scopedTAGs(D.getTag()))
+ appendScopes(D.getParent());
+ return appendUnqualifiedNameBefore(D);
+}
+
+template <typename DieType>
+bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
+ bool *FirstParameter) {
+ bool FirstParameterValue = true;
+ bool IsTemplate = false;
+ if (!FirstParameter)
+ FirstParameter = &FirstParameterValue;
+ for (const DieType &C : D) {
+ auto Sep = [&] {
+ if (*FirstParameter)
+ OS << '<';
+ else
+ OS << ", ";
+ IsTemplate = true;
+ EndedWithTemplate = false;
+ *FirstParameter = false;
+ };
+ if (C.getTag() == dwarf::DW_TAG_GNU_template_parameter_pack) {
+ IsTemplate = true;
+ appendTemplateParameters(C, FirstParameter);
+ }
+ if (C.getTag() == dwarf::DW_TAG_template_value_parameter) {
+ DieType T = detail::resolveReferencedType(C);
+ Sep();
+ if (T.getTag() == dwarf::DW_TAG_enumeration_type) {
+ OS << '(';
+ appendQualifiedName(T);
+ OS << ')';
+ auto V = C.find(dwarf::DW_AT_const_value);
+ OS << std::to_string(*V->getAsSignedConstant());
+ continue;
+ }
+ // /Maybe/ we could do pointer/reference type parameters, looking for the
+ // symbol in the ELF symbol table to get back to the variable...
+ // but probably not worth it.
+ if (T.getTag() == dwarf::DW_TAG_pointer_type ||
+ T.getTag() == dwarf::DW_TAG_reference_type)
+ continue;
+ const char *RawName = dwarf::toString(T.find(dwarf::DW_AT_name), nullptr);
+ assert(RawName);
+ StringRef Name = RawName;
+ auto V = C.find(dwarf::DW_AT_const_value);
+ bool IsQualifiedChar = false;
+ if (Name == "bool") {
+ OS << (*V->getAsUnsignedConstant() ? "true" : "false");
+ } else if (Name == "short") {
+ OS << "(short)";
+ OS << std::to_string(*V->getAsSignedConstant());
+ } else if (Name == "unsigned short") {
+ OS << "(unsigned short)";
+ OS << std::to_string(*V->getAsSignedConstant());
+ } else if (Name == "int")
+ OS << std::to_string(*V->getAsSignedConstant());
+ else if (Name == "long") {
+ OS << std::to_string(*V->getAsSignedConstant());
+ OS << "L";
+ } else if (Name == "long long") {
+ OS << std::to_string(*V->getAsSignedConstant());
+ OS << "LL";
+ } else if (Name == "unsigned int") {
+ OS << std::to_string(*V->getAsUnsignedConstant());
+ OS << "U";
+ } else if (Name == "unsigned long") {
+ OS << std::to_string(*V->getAsUnsignedConstant());
+ OS << "UL";
+ } else if (Name == "unsigned long long") {
+ OS << std::to_string(*V->getAsUnsignedConstant());
+ OS << "ULL";
+ } else if (Name == "char" ||
+ (IsQualifiedChar =
+ (Name == "unsigned char" || Name == "signed char"))) {
+ // FIXME: check T's DW_AT_type to see if it's signed or not (since
+ // char signedness is implementation defined).
+ auto Val = *V->getAsSignedConstant();
+ // Copied/hacked up from Clang's CharacterLiteral::print - incomplete
+ // (doesn't actually support different character types/widths, sign
+ // handling's not done, and doesn't correctly test if a character is
+ // printable or needs to use a numeric escape sequence instead)
+ if (IsQualifiedChar) {
+ OS << '(';
+ OS << Name;
+ OS << ')';
+ }
+ switch (Val) {
+ case '\\':
+ OS << "'\\\\'";
+ break;
+ case '\'':
+ OS << "'\\''";
+ break;
+ case '\a':
+ // TODO: K&R: the meaning of '\\a' is different in traditional C
+ OS << "'\\a'";
+ break;
+ case '\b':
+ OS << "'\\b'";
+ break;
+ case '\f':
+ OS << "'\\f'";
+ break;
+ case '\n':
+ OS << "'\\n'";
+ break;
+ case '\r':
+ OS << "'\\r'";
+ break;
+ case '\t':
+ OS << "'\\t'";
+ break;
+ case '\v':
+ OS << "'\\v'";
+ break;
+ default:
+ if ((Val & ~0xFFu) == ~0xFFu)
+ Val &= 0xFFu;
+ if (Val < 127 && Val >= 32) {
+ OS << "'";
+ OS << (char)Val;
+ OS << "'";
+ } else if (Val < 256)
+ OS << llvm::format("'\\x%02" PRIx64 "'", Val);
+ else if (Val <= 0xFFFF)
+ OS << llvm::format("'\\u%04" PRIx64 "'", Val);
+ else
+ OS << llvm::format("'\\U%08" PRIx64 "'", Val);
+ }
+ }
+ continue;
+ }
+ if (C.getTag() == dwarf::DW_TAG_GNU_template_template_param) {
+ const char *RawName =
+ dwarf::toString(C.find(dwarf::DW_AT_GNU_template_name), nullptr);
+ assert(RawName);
+ StringRef Name = RawName;
+ Sep();
+ OS << Name;
+ continue;
+ }
+ if (C.getTag() != dwarf::DW_TAG_template_type_parameter)
+ continue;
+ auto TypeAttr = C.find(dwarf::DW_AT_type);
+ Sep();
+ appendQualifiedName(TypeAttr ? detail::resolveReferencedType(C, *TypeAttr)
+ : DieType());
+ }
+ if (IsTemplate && *FirstParameter && FirstParameter == &FirstParameterValue) {
+ OS << '<';
+ EndedWithTemplate = false;
+ }
+ return IsTemplate;
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::appendAndTerminateTemplateParameters(DieType D) {
+ bool R = appendTemplateParameters(D);
+ if (!R)
+ return;
+
+ if (EndedWithTemplate)
+ OS << " ";
+ OS << ">";
+ EndedWithTemplate = true;
+ Word = true;
+}
+
+template <typename DieType>
+void DWARFTypePrinter<DieType>::decomposeConstVolatile(DieType &N, DieT...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
dwblaikie
reviewed
Sep 20, 2024
|
Seems reasonable. I guess LLDB's |
The LLVM one is a decade+ old fork of the LLDB one. It would be nice to unify them at some point - I'm not sure this is that point (but it might be - though it'd be a wider discussion) but at least it's probably a step along that path. (eg: probably moving the LLDB DWARFDIE APIs to use the LLVM naming convention (eg: change the lldb one from GetParent to getParent - or add an overload with the intent that eventually the names would match and the lldb one would be deprecated/removed as part of the transition to use the LLVM API/move the lldb one into LLVM, and the difference between the lldb DWARFDIE having a So, if some folks have opinions on if/how much we should try to merge the forks today, motivated in part by this work, I think we should have that discussion. |
labath
reviewed
Sep 24, 2024
There was a problem hiding this comment.
So, if some folks have opinions on if/how much we should try to merge the forks today, motivated in part by this work, I think we should have that discussion.
The problem with merging the forks is that the DWARFUnit, DWARFDie and DWARFFormValue classes are very tightly coupled (and also quite big/complex), so it's not possible to start using just one of them. The only way I see towards unification is to gradually morph one or both of the implementation towards the other until they become basically identical, and swapping them becomes a no-op.
With that in mind, my opinion (others can disagree) is that we should use this opportunity to start that process and unify the interfaces (not implementations) to the extent necessary to make this template trick work. That will also include changes to the naming convention, where the obvious choice is to unify towards the llvm style. As for differences in behavior, we can discuss which choice is better/easier on a case-by-case basis.
I started a discussion about this on https://discourse.llvm.org/t/adding-llvm-dwarfdie-apis-to-lldb-dwarfdie/81390 to unifying lldb's interfaces toward llvm's. |
labath
approved these changes
Oct 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This generalizes DWARFTypePrinter class to a template class so that it can be reused for lldb's DWARFDIE type.
This is a split of #90008. The difference is that this doesn't have
Visitortemplate parameter which can be added later if necessary.