[DebugInfo][CodeView] Move codeview::SourceLanguage enumerators to CodeViewLanguages.def (NFC)

[jalopezg-git]

This PR proposes moving out enumerators for codeview::SourceLanguage to a separate CodeViewLanguages.def file, following the same guideline that in other parts of LLVM, and in particular the TypeRecordKind (enumerators in CodeViewTypes.def) or SymbolRecordKind (enumerators in CodeViewSymbols.def).

This is a non-functional change, and has been labeled as such. This change helps for #137223, and possibly other future changes.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Javier Lopez-Gomez (jalopezg-git)

Changes

Per @CarlosAlbertoEnciso comment, this change was split from its original PR #137223. In his own words,
> I think this is good change, but it should be done in another PR (before the current one) and get the feedback from the
> CodeView teams. In that way it would be a global benefit for other tools (including llvm-debuginfo-analyzer).

This PR proposes moving out enumerators for codeview::SourceLanguage to a separate CodeViewLanguages.def file, following the same guideline that in other parts of LLVM, and in particular the TypeRecordKind (enumerators in CodeViewTypes.def) or SymbolRecordKind (enumerators in CodeViewSymbols.def).

This is a non-functional change, and has been labeled as such. This change helps for #137223, and possibly other future changes.

FYI, @CarlosAlbertoEnciso.

---

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

2 Files Affected:

• (modified) llvm/include/llvm/DebugInfo/CodeView/CodeView.h (+2-30)
• (added) llvm/include/llvm/DebugInfo/CodeView/CodeViewLanguages.def (+51)

diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeView.h b/llvm/include/llvm/DebugInfo/CodeView/CodeView.h
index 5cdff5ff0e82f..12d589db03b25 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CodeView.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeView.h
@@ -144,36 +144,8 @@ enum class CPUType : uint16_t {
 /// Debug Interface Access SDK, and are documented here:
 /// https://learn.microsoft.com/en-us/visualstudio/debugger/debug-interface-access/cv-cfl-lang
 enum SourceLanguage : uint8_t {
-  C = 0x00,
-  Cpp = 0x01,
-  Fortran = 0x02,
-  Masm = 0x03,
-  Pascal = 0x04,
-  Basic = 0x05,
-  Cobol = 0x06,
-  Link = 0x07,
-  Cvtres = 0x08,
-  Cvtpgd = 0x09,
-  CSharp = 0x0a,
-  VB = 0x0b,
-  ILAsm = 0x0c,
-  Java = 0x0d,
-  JScript = 0x0e,
-  MSIL = 0x0f,
-  HLSL = 0x10,
-  ObjC = 0x11,
-  ObjCpp = 0x12,
-  Swift = 0x13,
-  AliasObj = 0x14,
-  Rust = 0x15,
-  Go = 0x16,
-
-  /// The DMD compiler emits 'D' for the CV source language. Microsoft does not
-  /// have an enumerator for it yet.
-  D = 'D',
-  /// The Swift compiler used to emit 'S' for the CV source language, but
-  /// current versions emit the enumerator defined above.
-  OldSwift = 'S',
+#define CV_LANGUAGE(NAME, ID) NAME = ID,
+#include "CodeViewLanguages.def"
 };
 
 /// These values correspond to the CV_call_e enumeration, and are documented
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CodeViewLanguages.def b/llvm/include/llvm/DebugInfo/CodeView/CodeViewLanguages.def
new file mode 100644
index 0000000000000..5c6335fffd4dd
--- /dev/null
+++ b/llvm/include/llvm/DebugInfo/CodeView/CodeViewLanguages.def
@@ -0,0 +1,51 @@
+//===-- CodeViewLanguages.def - All CodeView languages ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// These values correspond to the CV_CFL_LANG enumeration in the Microsoft
+// Debug Interface Access SDK, and are documented here:
+// https://learn.microsoft.com/en-us/visualstudio/debugger/debug-interface-access/cv-cfl-lang
+// This should match the constants there.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CV_LANGUAGE
+#define CV_LANGUAGE(NAME, ID)
+#endif
+
+CV_LANGUAGE(C, 0x00)
+CV_LANGUAGE(Cpp, 0x01)
+CV_LANGUAGE(Fortran, 0x02)
+CV_LANGUAGE(Masm, 0x03)
+CV_LANGUAGE(Pascal, 0x04)
+CV_LANGUAGE(Basic, 0x05)
+CV_LANGUAGE(Cobol, 0x06)
+CV_LANGUAGE(Link, 0x07)
+CV_LANGUAGE(Cvtres, 0x08)
+CV_LANGUAGE(Cvtpgd, 0x09)
+CV_LANGUAGE(CSharp, 0x0a)
+CV_LANGUAGE(VB, 0x0b)
+CV_LANGUAGE(ILAsm, 0x0c)
+CV_LANGUAGE(Java, 0x0d)
+CV_LANGUAGE(JScript, 0x0e)
+CV_LANGUAGE(MSIL, 0x0f)
+CV_LANGUAGE(HLSL, 0x10)
+CV_LANGUAGE(ObjC, 0x11)
+CV_LANGUAGE(ObjCpp, 0x12)
+CV_LANGUAGE(Swift, 0x13)
+CV_LANGUAGE(AliasObj, 0x14)
+CV_LANGUAGE(Rust, 0x15)
+CV_LANGUAGE(Go, 0x16)
+
+// The DMD compiler emits 'D' for the CV source language. Microsoft does not
+// have an enumerator for it yet.
+CV_LANGUAGE(D, 'D')
+// The Swift compiler used to emit 'S' for the CV source language, but
+// current versions emit the enumerator defined above.
+CV_LANGUAGE(OldSwift, 'S')
+
+#undef CV_LANGUAGE

[dwblaikie]

@zmodem for CodeView things, maybe - or might know other folks interested in it.

[CarlosAlbertoEnciso]

Maybe @rnk for CodeView related work.

[zmodem]

LLVM does this a lot, but I think there has to be some benefit, otherwise it's just extra indirection over a having a regular enum.

I don't know if the enum you're creating in #137223 is enough of a motivation. Does it really have much benefit over just storing the original enum values + tag in an unsigned? The new enum seems to be mostly used by looking at the tag and then casting to one of the original enums anyway.

[jalopezg-git]

I don't know if the enum you're creating in #137223 is enough of a motivation. Does it really have much benefit over just storing the original enum values + tag in an unsigned? The new enum seems to be mostly used by looking at the tag and then casting to one of the original enums anyway.

Thanks for the review, @zmodem! Partially agreed; actually, the previous incarnation of #137223 was using a std::variant of all possibly underlying enum types, but I tend to like the 'unified' enum a bit more (e.g. values can be compared directly, w/o adding any other boilerplate).

Actually, given that DWARF header file llvm/include/llvm/BinaryFormat/Dwarf.h specifies SourceLanguages using a PP macro in a .def file, I believe this change adds a bit of 'symmetry' in that regard.

Also, this provides the additional benefit of making it possible to reuse these constants somewhere else in the future. What do you think?

[rnk]

I'm in favor. I think it's kind of inevitable that all large/important enums end up as x-macros, because at the end of the day, most enums need to be printed textually, and x-macros are the main way to do that without repeating the names.

[CarlosAlbertoEnciso]

LGTM

[zmodem]

Fair enough, I'm convinced :)

[CarlosAlbertoEnciso]

@jalopezg-git Have you solved your permission access?

[jalopezg-git]

@jalopezg-git Have you solved your permission access?

No news yet; apparently, it's taking a bit long. @CarlosAlbertoEnciso, could you merge this on my behalf?

[CarlosAlbertoEnciso]

@jalopezg-git Have you solved your permission access?

No news yet; apparently, it's taking a bit long. @CarlosAlbertoEnciso, could you merge this on my behalf?

Happy to do it.
As the PR description contains other comments, would you be happy with the following text?
Added the CodeView label.

[DebugInfo][CodeView] Move codeview::SourceLanguage enumerators to CodeViewLanguages.def (NFC) #141750

This PR proposes moving out enumerators for codeview::SourceLanguage
to a separate CodeViewLanguages.def file, following the same guideline
that in other parts of LLVM, and in particular the TypeRecordKind
(enumerators in CodeViewTypes.def) or SymbolRecordKind (enumerators
in CodeViewSymbols.def).

This is a non-functional change, and has been labeled as such.
This change helps for https://github.com/llvm/llvm-project/pull/137223,
and possibly other future changes.

[jalopezg-git]

@jalopezg-git Have you solved your permission access?

No news yet; apparently, it's taking a bit long. @CarlosAlbertoEnciso, could you merge this on my behalf?

Happy to do it. As the PR description contains other comments, would you be happy with the following text? Added the CodeView label.

[DebugInfo][CodeView] Move codeview::SourceLanguage enumerators to CodeViewLanguages.def (NFC) #141750

This PR proposes moving out enumerators for codeview::SourceLanguage
to a separate CodeViewLanguages.def file, following the same guideline
that in other parts of LLVM, and in particular the TypeRecordKind
(enumerators in CodeViewTypes.def) or SymbolRecordKind (enumerators
in CodeViewSymbols.def).

This is a non-functional change, and has been labeled as such.
This change helps for https://github.com/llvm/llvm-project/pull/137223,
and possibly other future changes.

Done 👍.