Skip to content

[clang][DebugInfo] Set EnumKind based on enum_extensibility attribute #126045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Michael137
Copy link
Member

@Michael137 Michael137 commented Feb 6, 2025

This is the 2nd part to #124752. Here we make sure to set the DICompositeType EnumKind if the enum was declared with __attribute__((enum_extensibility(...))). In DWARF this will be rendered as DW_AT_APPLE_enum_kind and will be used by LLDB when creating clang::EnumDecls from debug-info.

Depends on #126044

This is the 2nd part to llvm#124752. Here we make sure to set the `DICompositeType` `EnumKind` if the enum was declared with `__attribute__((enum_extensibility(...)))`. In DWARF this will be rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when creating `clang::EnumDecl`s from debug-info.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Michael Buch (Michael137)

Changes

This is the 2nd part to #124752. Here we make sure to set the DICompositeType EnumKind if the enum was declared with __attribute__((enum_extensibility(...))). In DWARF this will be rendered as DW_AT_APPLE_enum_kind and will be used by LLDB when creating clang::EnumDecls from debug-info.

Depends on #126044


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+5-1)
  • (added) clang/test/CodeGen/debug-info-enum-extensibility.c (+49)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index db595796c067e9..d5b584ec0f2e95 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3567,6 +3567,10 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const EnumType *Ty) {
         DBuilder.createEnumerator(Enum->getName(), Enum->getInitVal()));
   }
 
+  std::optional<EnumExtensibilityAttr::Kind> EnumKind;
+  if (auto *Attr = ED->getAttr<EnumExtensibilityAttr>())
+    EnumKind = Attr->getExtensibility();
+
   // Return a CompositeType for the enum itself.
   llvm::DINodeArray EltArray = DBuilder.getOrCreateArray(Enumerators);
 
@@ -3576,7 +3580,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const EnumType *Ty) {
   llvm::DIType *ClassTy = getOrCreateType(ED->getIntegerType(), DefUnit);
   return DBuilder.createEnumerationType(
       EnumContext, ED->getName(), DefUnit, Line, Size, Align, EltArray, ClassTy,
-      /*RunTimeLang=*/0, Identifier, ED->isScoped());
+      /*RunTimeLang=*/0, Identifier, ED->isScoped(), EnumKind);
 }
 
 llvm::DIMacro *CGDebugInfo::CreateMacro(llvm::DIMacroFile *Parent,
diff --git a/clang/test/CodeGen/debug-info-enum-extensibility.c b/clang/test/CodeGen/debug-info-enum-extensibility.c
new file mode 100644
index 00000000000000..4f8a42bff3f019
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-enum-extensibility.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-NOT:  enumKind
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type, name: "ClosedEnum"
+// CHECK-SAME:                  enumKind: DW_APPLE_ENUM_KIND_Closed)
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type, name: "OpenEnum"
+// CHECK-SAME:                  enumKind: DW_APPLE_ENUM_KIND_Open)
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type, name: "ClosedFlagEnum"
+// CHECK-SAME:                  enumKind: DW_APPLE_ENUM_KIND_Closed)
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type, name: "OpenFlagEnum"
+// CHECK-SAME:                  enumKind: DW_APPLE_ENUM_KIND_Open)
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type, name: "MixedEnum"
+// CHECK-SAME:                  enumKind: DW_APPLE_ENUM_KIND_Open)
+
+enum Enum {
+  E0, E1
+};
+
+enum FlagEnum {
+  FE0 = 1 << 0, FE1 = 1 << 1
+};
+
+enum __attribute__((enum_extensibility(closed))) ClosedEnum {
+  A0, A1
+};
+
+enum __attribute__((enum_extensibility(open))) OpenEnum {
+  B0, B1
+};
+
+enum __attribute__((enum_extensibility(closed),flag_enum)) ClosedFlagEnum {
+  C0 = 1 << 0, C1 = 1 << 1
+};
+
+enum __attribute__((enum_extensibility(open),flag_enum)) OpenFlagEnum {
+  D0 = 1 << 0, D1 = 1 << 1
+};
+
+enum __attribute__((enum_extensibility(open), enum_extensibility(closed))) MixedEnum {
+  M0, M1
+};
+
+enum Enum e;
+enum FlagEnum fe;
+enum ClosedEnum ce;
+enum OpenEnum oe;
+enum ClosedFlagEnum cfe;
+enum OpenFlagEnum ofe;
+enum MixedEnum me;

Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Feb 6, 2025
…dSkipZero is not set

I ran into this whil working on a different patch where I'm emitting a zero-valued DWARF enum field which shouldn't be skipped.

This patch checks the (currently unused) `ShouldSkipZero` before deciding to skip printing this field. Based on git history this seems like an oversight from the initial refactor that introduced this. We have a similar check in `printInt`.

Wasn't sure how to best test this, but tests in an upcoming patch rely on this functionality (see llvm#126045).

Currently the only place `ShouldSkipZero` is set to `false` is when emitting the `DW_LANG_` enum. But the language codes start at `0x1`. So it never exercised this codepath (and we should probably just make it not pass this parameter).
Michael137 added a commit that referenced this pull request Feb 6, 2025
…dSkipZero is not set (#126044)

I ran into this while working on a different patch where I'm emitting a
zero-valued DWARF enum field which shouldn't be skipped.

This patch checks the (currently unused) `ShouldSkipZero` before
deciding to skip printing this field. Based on git history this seems
like an oversight from the initial refactor that introduced this. We
have a similar check in `printInt`.

Wasn't sure how to best test this, but tests in an upcoming patch rely
on this functionality (see
#126045).

Currently the only place `ShouldSkipZero` is set to `false` is when
emitting the `DW_LANG_` enum. But the language codes start at `0x1`. So
it never exercised this codepath (and we should probably just make it
not pass this parameter).
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 6, 2025
… when ShouldSkipZero is not set (#126044)

I ran into this while working on a different patch where I'm emitting a
zero-valued DWARF enum field which shouldn't be skipped.

This patch checks the (currently unused) `ShouldSkipZero` before
deciding to skip printing this field. Based on git history this seems
like an oversight from the initial refactor that introduced this. We
have a similar check in `printInt`.

Wasn't sure how to best test this, but tests in an upcoming patch rely
on this functionality (see
llvm/llvm-project#126045).

Currently the only place `ShouldSkipZero` is set to `false` is when
emitting the `DW_LANG_` enum. But the language codes start at `0x1`. So
it never exercised this codepath (and we should probably just make it
not pass this parameter).
@Michael137
Copy link
Member Author

The one test failure unrelated:

FAIL: Clang :: Tooling/clang-linker-wrapper-spirv-elf.cpp (20542 of 21851)
******************** TEST 'Clang :: Tooling/clang-linker-wrapper-spirv-elf.cpp' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
RUN: at line 4: mkdir -p /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_tmp
+ mkdir -p /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_tmp
RUN: at line 5: cd /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_tmp
+ cd /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_tmp
RUN: at line 6: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/bin/clang --driver-mode=g++ -fopenmp -fopenmp-targets=spirv64-intel -nogpulib -c -o /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_clang-linker-wrapper-spirv-elf.o /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp
+ /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/bin/clang --driver-mode=g++ -fopenmp -fopenmp-targets=spirv64-intel -nogpulib -c -o /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/build/tools/clang/test/Tooling/Output/clang-linker-wrapper-spirv-elf.cpp.tmp_clang-linker-wrapper-spirv-elf.o /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-qxq7w-1/llvm-project/github-pull-requests/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp
clang: error: unable to execute command: Executable "llvm-spirv" doesn't exist!
clang: error: llvm-spirv command failed with exit code 1 (use -v to see invocation)

@Michael137 Michael137 merged commit e00fc80 into llvm:main Feb 7, 2025
6 of 8 checks passed
@Michael137 Michael137 deleted the clang/debug-info-enum-extensibility-frontend branch February 7, 2025 09:28
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…dSkipZero is not set (llvm#126044)

I ran into this while working on a different patch where I'm emitting a
zero-valued DWARF enum field which shouldn't be skipped.

This patch checks the (currently unused) `ShouldSkipZero` before
deciding to skip printing this field. Based on git history this seems
like an oversight from the initial refactor that introduced this. We
have a similar check in `printInt`.

Wasn't sure how to best test this, but tests in an upcoming patch rely
on this functionality (see
llvm#126045).

Currently the only place `ShouldSkipZero` is set to `false` is when
emitting the `DW_LANG_` enum. But the language codes start at `0x1`. So
it never exercised this codepath (and we should probably just make it
not pass this parameter).
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…llvm#126045)

This is the 2nd part to
llvm#124752. Here we make sure to
set the `DICompositeType` `EnumKind` if the enum was declared with
`__attribute__((enum_extensibility(...)))`. In DWARF this will be
rendered as `DW_AT_APPLE_enum_kind` and will be used by LLDB when
creating `clang::EnumDecl`s from debug-info.
 
Depends on llvm#126044
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category debuginfo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants