Skip to content

Conversation

@fsfod
Copy link
Contributor

@fsfod fsfod commented Oct 20, 2024

These will be needed for when llvm is built as a shared library on windows with explicit visibility macros enabled.
Change UnionRecord to class instead of a struct so we can use X macros from CodeViewTypes.def to forward declare all record classes.

This is part of the work to enable LLVM_BUILD_LLVM_DYLIB and LLVM plugins on window.

@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2024

@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-debuginfo

Author: Thomas Fransham (fsfod)

Changes

These will be needed for when llvm is built as a shared library on windows with explicit visibility macros enabled.
Change UnionRecord to class instead of a struct so we can use X macros from CodeViewTypes.def to forward declare all record classes.

This is part of the work to enable LLVM_BUILD_LLVM_DYLIB and LLVM plugins on window.


Full diff: https://github.com/llvm/llvm-project/pull/113102.diff

4 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h (+1-1)
  • (modified) llvm/include/llvm/DebugInfo/CodeView/ContinuationRecordBuilder.h (+14)
  • (modified) llvm/include/llvm/DebugInfo/CodeView/SimpleTypeSerializer.h (+13)
  • (modified) llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h (+2-1)
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
index 669c44aa131edc..297f11451afb75 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -32,7 +32,7 @@ class ClassRecord;
 class EnumRecord;
 class ModifierRecord;
 class PointerRecord;
-struct UnionRecord;
+class UnionRecord;
 } // namespace codeview
 } // namespace llvm
 
diff --git a/llvm/include/llvm/DebugInfo/CodeView/ContinuationRecordBuilder.h b/llvm/include/llvm/DebugInfo/CodeView/ContinuationRecordBuilder.h
index 84cef520a2f460..8347ef870d0676 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/ContinuationRecordBuilder.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/ContinuationRecordBuilder.h
@@ -50,6 +50,20 @@ class ContinuationRecordBuilder {
 
   std::vector<CVType> end(TypeIndex Index);
 };
+
+// Needed by RandomAccessVisitorTest.cpp
+#define TYPE_RECORD(EnumName, EnumVal, Name)
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
+  extern template LLVM_TEMPLATE_ABI void ContinuationRecordBuilder::writeMemberType(    \
+      Name##Record &Record);
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#include "llvm/DebugInfo/CodeView/CodeViewTypes.def"
+#undef TYPE_RECORD
+#undef TYPE_RECORD_ALIAS
+#undef MEMBER_RECORD
+#undef MEMBER_RECORD_ALIAS
+
 } // namespace codeview
 } // namespace llvm
 
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SimpleTypeSerializer.h b/llvm/include/llvm/DebugInfo/CodeView/SimpleTypeSerializer.h
index fcc0452a6ae9a7..713798fc38d2d8 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SimpleTypeSerializer.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SimpleTypeSerializer.h
@@ -32,6 +32,19 @@ class SimpleTypeSerializer {
   ArrayRef<uint8_t> serialize(const FieldListRecord &Record) = delete;
 };
 
+// Needed by RandomAccessVisitorTest.cpp
+#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
+  class Name##Record;                                                          \
+  extern template LLVM_TEMPLATE_ABI ArrayRef<uint8_t> llvm::codeview::SimpleTypeSerializer::serialize(  \
+      Name##Record &Record);
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#define MEMBER_RECORD(EnumName, EnumVal, Name)
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#include "llvm/DebugInfo/CodeView/CodeViewTypes.def"
+#undef TYPE_RECORD
+#undef TYPE_RECORD_ALIAS
+#undef MEMBER_RECORD
+#undef MEMBER_RECORD_ALIAS
 } // end namespace codeview
 } // end namespace llvm
 
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
index 5a84fac5f59034..484e05b5adc672 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -495,7 +495,8 @@ class ClassRecord : public TagRecord {
 };
 
 // LF_UNION
-struct UnionRecord : public TagRecord {
+class UnionRecord : public TagRecord {
+public:
   UnionRecord() = default;
   explicit UnionRecord(TypeRecordKind Kind) : TagRecord(Kind) {}
   UnionRecord(uint16_t MemberCount, ClassOptions Options, TypeIndex FieldList,

@vgvassilev
Copy link
Contributor

@JDevlieghere, ping.

@labath
Copy link
Collaborator

labath commented Oct 30, 2024

The lldb change looks good (you can submit that in its own patch without a review), but you'll probably want to find a different reviewer for the codeview part (I have no idea who's responsible for that these days).

@vgvassilev
Copy link
Contributor

The lldb change looks good (you can submit that in its own patch without a review), but you'll probably want to find a different reviewer for the codeview part (I have no idea who's responsible for that these days).

Thank you, maybe @adrian-prantl will know?

@vgvassilev vgvassilev requested a review from dwblaikie November 7, 2024 15:12
@dwblaikie
Copy link
Collaborator

@zmodem perhaps you know someone who might be able to look at a CodeView change?

@vgvassilev
Copy link
Contributor

@zmodem, ping.

@vgvassilev vgvassilev requested a review from compnerd November 19, 2024 20:58
#undef TYPE_RECORD
#undef TYPE_RECORD_ALIAS
#undef MEMBER_RECORD
#undef MEMBER_RECORD_ALIAS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would double check that this works with modular builds, this seems like it could cause some issues.

#undef TYPE_RECORD
#undef TYPE_RECORD_ALIAS
#undef MEMBER_RECORD
#undef MEMBER_RECORD_ALIAS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise

// LF_UNION
struct UnionRecord : public TagRecord {
class UnionRecord : public TagRecord {
public:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change here? The use of struct should be fine. Or was it that there was a conflict with the declaration from the XMACRO?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it wasn't usable in the XMACRO to forward declare classes used for the function declarations it creates

@vgvassilev
Copy link
Contributor

@fsfod, can you address the comments?

fsfod added 2 commits January 29, 2025 17:03
…ions

These will be needed for when llvm is built as a shared library on windows with
explicit visibility macros enabled.
Change UnionRecord to class instead of a struct so we can use X macros from
CodeViewTypes.def to forward declare all record classes.

This is part of the work to enable LLVM_BUILD_LLVM_DYLIB and LLVM plugins on window.
@fsfod fsfod force-pushed the exported-api/debuginfo-extern-templates branch from d9de70a to 0d42923 Compare January 29, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants