[lldb] Move SBLanguages.h out of API tree #111929
Closed
Conversation
This file contains hidden or 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
medismailben
requested review from
adrian-prantl,
jimingham and
chelcassanova
medismailben
requested review from
rupprecht,
keith and
JDevlieghere
as code owners
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch moves Since this is an enum, this shouldn't be part of the API tree but rather it should be included in the Also, that enum was used in internal methods in This should address the feedbacks from #111907. Full diff: https://github.com/llvm/llvm-project/pull/111929.diff 17 Files Affected:
diff --git a/lldb/bindings/CMakeLists.txt b/lldb/bindings/CMakeLists.txt
index bec694e43bd7be..befc5c9e9afc15 100644
--- a/lldb/bindings/CMakeLists.txt
+++ b/lldb/bindings/CMakeLists.txt
@@ -3,7 +3,7 @@ file(GLOB_RECURSE SWIG_SOURCES *.swig)
file(GLOB SWIG_HEADERS
${LLDB_SOURCE_DIR}/include/lldb/API/*.h
${LLDB_SOURCE_DIR}/include/lldb/*.h
- ${LLDB_BINARY_DIR}/include/lldb/API/SBLanguages.h
+ ${LLDB_BINARY_DIR}/include/lldb/SourceLanguageNames.h
)
file(GLOB SWIG_PRIVATE_HEADERS
${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h
diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig
index c0dde905f986bd..2420ea4aab843a 100644
--- a/lldb/bindings/headers.swig
+++ b/lldb/bindings/headers.swig
@@ -39,7 +39,6 @@
#include "lldb/API/SBHostOS.h"
#include "lldb/API/SBInstruction.h"
#include "lldb/API/SBInstructionList.h"
-#include "lldb/API/SBLanguages.h"
#include "lldb/API/SBLanguageRuntime.h"
#include "lldb/API/SBLaunchInfo.h"
#include "lldb/API/SBLineEntry.h"
diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig
index 8a6fed95f0b729..e48e875367f7c4 100644
--- a/lldb/bindings/interfaces.swig
+++ b/lldb/bindings/interfaces.swig
@@ -120,7 +120,6 @@
%include "lldb/API/SBHostOS.h"
%include "lldb/API/SBInstruction.h"
%include "lldb/API/SBInstructionList.h"
-%include "lldb/API/SBLanguages.h"
%include "lldb/API/SBLanguageRuntime.h"
%include "lldb/API/SBLaunchInfo.h"
%include "lldb/API/SBLineEntry.h"
diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake
index 471aeaaad3c0d3..0daff8b3e07e2b 100644
--- a/lldb/cmake/modules/LLDBFramework.cmake
+++ b/lldb/cmake/modules/LLDBFramework.cmake
@@ -71,7 +71,7 @@ endif()
# At configuration time, collect headers for the framework bundle and copy them
# into a staging directory. Later we can copy over the entire folder.
file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
-set(generated_public_headers ${LLDB_OBJ_DIR}/include/lldb/API/SBLanguages.h)
+set(generated_public_headers ${LLDB_OBJ_DIR}/include/lldb/SourceLanguageNames.h)
file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
list(REMOVE_ITEM root_public_headers ${root_private_headers})
diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h
index 40368e036e0e40..8884b21a40120f 100644
--- a/lldb/include/lldb/API/LLDB.h
+++ b/lldb/include/lldb/API/LLDB.h
@@ -42,7 +42,6 @@
#include "lldb/API/SBInstruction.h"
#include "lldb/API/SBInstructionList.h"
#include "lldb/API/SBLanguageRuntime.h"
-#include "lldb/API/SBLanguages.h"
#include "lldb/API/SBLaunchInfo.h"
#include "lldb/API/SBLineEntry.h"
#include "lldb/API/SBListener.h"
diff --git a/lldb/include/lldb/API/SBExpressionOptions.h b/lldb/include/lldb/API/SBExpressionOptions.h
index a9e929a4c0bd9d..490bdb982e8d6b 100644
--- a/lldb/include/lldb/API/SBExpressionOptions.h
+++ b/lldb/include/lldb/API/SBExpressionOptions.h
@@ -10,7 +10,6 @@
#define LLDB_API_SBEXPRESSIONOPTIONS_H
#include "lldb/API/SBDefines.h"
-#include "lldb/API/SBLanguages.h"
#include <vector>
@@ -71,7 +70,7 @@ class LLDB_API SBExpressionOptions {
/// Set the language using a pair of language code and version as
/// defined by the DWARF 6 specification.
/// WARNING: These codes may change until DWARF 6 is finalized.
- void SetLanguage(lldb::SBSourceLanguageName name, uint32_t version);
+ void SetLanguage(lldb::SourceLanguageName name, uint32_t version);
#ifndef SWIG
void SetCancelCallback(lldb::ExpressionCancelCallback callback, void *baton);
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index e4848f19e64d62..abe3f35630ce7c 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -324,7 +324,7 @@ class EvaluateExpressionOptions {
/// Set the language using a pair of language code and version as
/// defined by the DWARF 6 specification.
/// WARNING: These codes may change until DWARF 6 is finalized.
- void SetLanguage(uint16_t name, uint32_t version) {
+ void SetLanguage(lldb::SourceLanguageName name, uint32_t version) {
m_language = SourceLanguage(name, version);
}
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 938f6e3abe8f2a..c2845491e16d86 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -12,6 +12,8 @@
#include <cstdint>
#include <type_traits>
+#include "lldb/SourceLanguageNames.h"
+
#ifndef SWIG
// Macro to enable bitmask operations on an enum. Without this, Enum | Enum
// gets promoted to an int, so you have to say Enum a = Enum(eFoo | eBar). If
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index b82a2b8aa05744..da7e5c430a7dd8 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -103,18 +103,19 @@ struct RegisterSet {
struct SourceLanguage {
SourceLanguage() = default;
SourceLanguage(lldb::LanguageType language_type);
- SourceLanguage(uint16_t name, uint32_t version)
+ SourceLanguage(lldb::SourceLanguageName name, uint32_t version)
: name(name), version(version) {}
- SourceLanguage(std::optional<std::pair<uint16_t, uint32_t>> name_vers)
- : name(name_vers ? name_vers->first : 0),
+ SourceLanguage(
+ std::optional<std::pair<lldb::SourceLanguageName, uint32_t>> name_vers)
+ : name(name_vers ? std::optional(name_vers->first) : std::nullopt),
version(name_vers ? name_vers->second : 0) {}
- operator bool() const { return name > 0; }
+ operator bool() const { return name.has_value(); }
lldb::LanguageType AsLanguageType() const;
llvm::StringRef GetDescription() const;
bool IsC() const;
bool IsObjC() const;
bool IsCPlusPlus() const;
- uint16_t name = 0;
+ std::optional<lldb::SourceLanguageName> name;
uint32_t version = 0;
};
diff --git a/lldb/scripts/generate-sbapi-dwarf-enum.py b/lldb/scripts/generate-sbapi-dwarf-enum.py
index 7fd6037986317e..6996bbd2840be2 100755
--- a/lldb/scripts/generate-sbapi-dwarf-enum.py
+++ b/lldb/scripts/generate-sbapi-dwarf-enum.py
@@ -5,7 +5,7 @@
import os
HEADER = """\
-//===-- SBLanguages.h -----------------------------------------*- C++ -*-===//
+//===-- SourceLanguageNames.h -----------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -13,14 +13,13 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLDB_API_SBLANGUAGE_H
-#define LLDB_API_SBLANGUAGE_H
+#ifndef LLDB_SOURCELANGUAGENAMES_H
+#define LLDB_SOURCELANGUAGENAMES_H
namespace lldb {
-/// Used by \\ref SBExpressionOptions.
/// These enumerations use the same language enumerations as the DWARF
/// specification for ease of use and consistency.
-enum SBSourceLanguageName : uint16_t {
+enum SourceLanguageName {
"""
FOOTER = """\
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index a32bc58507d8eb..3105e061759791 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -20,24 +20,24 @@ if(LLDB_ENABLE_LUA)
set(lldb_lua_wrapper ${lua_bindings_dir}/LLDBWrapLua.cpp)
endif()
-# Generate SBLanguages.h from Dwarf.def.
-set(sb_languages_file
- ${CMAKE_CURRENT_BINARY_DIR}/../../include/lldb/API/SBLanguages.h)
-set(sb_languages_generator
+# Generate SourceLanguageNames.h from Dwarf.def.
+set(source_language_names_file
+ ${CMAKE_CURRENT_BINARY_DIR}/../../include/lldb/SourceLanguageNames.h)
+set(source_language_names_generator
${LLDB_SOURCE_DIR}/scripts/generate-sbapi-dwarf-enum.py)
add_custom_command(
- COMMENT "Generating SBLanguages.h from Dwarf.def"
+ COMMENT "Generating SourceLanguageNames.h from Dwarf.def"
COMMAND "${Python3_EXECUTABLE}"
- ${sb_languages_generator}
+ ${source_language_names_generator}
${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat/Dwarf.def
- -o ${sb_languages_file}
- OUTPUT ${sb_languages_file}
+ -o ${source_language_names_file}
+ OUTPUT ${source_language_names_file}
DEPENDS ${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat/Dwarf.def
- ${sb_languages_generator}
+ ${source_language_names_generator}
WORKING_DIRECTORY ${LLVM_LIBRARY_OUTPUT_INTDIR}
)
add_custom_target(lldb-sbapi-dwarf-enums
- DEPENDS ${sb_languages_file})
+ DEPENDS ${source_language_names_file})
set_target_properties(lldb-sbapi-dwarf-enums PROPERTIES FOLDER "LLDB/Tablegenning")
add_lldb_library(liblldb SHARED ${option_framework}
diff --git a/lldb/source/API/SBExpressionOptions.cpp b/lldb/source/API/SBExpressionOptions.cpp
index 15ed403eaaea12..647ba1fed6c643 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -156,7 +156,7 @@ void SBExpressionOptions::SetLanguage(lldb::LanguageType language) {
m_opaque_up->SetLanguage(language);
}
-void SBExpressionOptions::SetLanguage(lldb::SBSourceLanguageName name,
+void SBExpressionOptions::SetLanguage(lldb::SourceLanguageName name,
uint32_t version) {
LLDB_INSTRUMENT_VA(this, name, version);
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index 30c44b974988d0..a951fe43212cf2 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -1027,7 +1027,7 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
SourceLanguage language = target->GetLanguage();
if (!language)
language = frame->GetLanguage();
- options.SetLanguage((SBSourceLanguageName)language.name, language.version);
+ options.SetLanguage(*language.name, language.version);
return EvaluateExpression(expr, options);
} else {
Status error;
@@ -1059,7 +1059,7 @@ SBFrame::EvaluateExpression(const char *expr,
language = target->GetLanguage();
if (!language && frame)
language = frame->GetLanguage();
- options.SetLanguage((SBSourceLanguageName)language.name, language.version);
+ options.SetLanguage(*language.name, language.version);
return EvaluateExpression(expr, options);
}
@@ -1082,7 +1082,7 @@ SBValue SBFrame::EvaluateExpression(const char *expr,
language = target->GetLanguage();
if (!language && frame)
language = frame->GetLanguage();
- options.SetLanguage((SBSourceLanguageName)language.name, language.version);
+ options.SetLanguage(*language.name, language.version);
return EvaluateExpression(expr, options);
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 52d52826c946d1..e12a100bbf7d6d 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -71,14 +71,19 @@ ClangUserExpression::ClangUserExpression(
m_type_system_helper(*m_target_wp.lock(), options.GetExecutionPolicy() ==
eExecutionPolicyTopLevel),
m_result_delegate(exe_scope.CalculateTarget()), m_ctx_obj(ctx_obj) {
- switch (m_language.name) {
- case llvm::dwarf::DW_LNAME_C_plus_plus:
+ if (!m_language.name) {
+ m_allow_cxx = true;
+ m_allow_objc = true;
+ return;
+ }
+ switch (*m_language.name) {
+ case lldb::SourceLanguageName::eLanguageNameC_plus_plus:
m_allow_cxx = true;
break;
- case llvm::dwarf::DW_LNAME_ObjC:
+ case lldb::SourceLanguageName::eLanguageNameObjC:
m_allow_objc = true;
break;
- case llvm::dwarf::DW_LNAME_ObjC_plus_plus:
+ case lldb::SourceLanguageName::eLanguageNameObjC_plus_plus:
default:
m_allow_cxx = true;
m_allow_objc = true;
diff --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index d0bffe441f6395..9c79f02f334f3e 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -535,19 +535,22 @@ Language::Language() = default;
Language::~Language() = default;
SourceLanguage::SourceLanguage(lldb::LanguageType language_type) {
- auto lname =
- llvm::dwarf::toDW_LNAME((llvm::dwarf::SourceLanguage)language_type);
+ auto lname = llvm::dwarf::toDW_LNAME(
+ static_cast<llvm::dwarf::SourceLanguage>(language_type));
if (!lname)
return;
- name = lname->first;
+ name.emplace(static_cast<SourceLanguageName>(lname->first));
version = lname->second;
}
lldb::LanguageType SourceLanguage::AsLanguageType() const {
- if (auto lang = llvm::dwarf::toDW_LANG((llvm::dwarf::SourceLanguageName)name,
- version))
- return (lldb::LanguageType)*lang;
- return lldb::eLanguageTypeUnknown;
+ if (!name)
+ return lldb::eLanguageTypeUnknown;
+ auto lang = llvm::dwarf::toDW_LANG(
+ static_cast<llvm::dwarf::SourceLanguageName>(*name), version);
+ if (!lang)
+ return lldb::eLanguageTypeUnknown;
+ return static_cast<lldb::LanguageType>(*lang);
}
llvm::StringRef SourceLanguage::GetDescription() const {
@@ -555,7 +558,7 @@ llvm::StringRef SourceLanguage::GetDescription() const {
if (type)
return Language::GetNameForLanguageType(type);
return llvm::dwarf::LanguageDescription(
- (llvm::dwarf::SourceLanguageName)name);
+ static_cast<llvm::dwarf::SourceLanguageName>(name ? *name : 0));
}
bool SourceLanguage::IsC() const { return name == llvm::dwarf::DW_LNAME_C; }
diff --git a/llvm/utils/gn/secondary/lldb/include/lldb/API/BUILD.gn b/llvm/utils/gn/secondary/lldb/include/lldb/BUILD.gn
similarity index 72%
rename from llvm/utils/gn/secondary/lldb/include/lldb/API/BUILD.gn
rename to llvm/utils/gn/secondary/lldb/include/lldb/BUILD.gn
index 2fe295d136569d..d8df32761f89e3 100644
--- a/llvm/utils/gn/secondary/lldb/include/lldb/API/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/include/lldb/BUILD.gn
@@ -1,6 +1,6 @@
-action("SBLanguages") {
+action("SourceLanguageNames") {
script = "//lldb/scripts/generate-sbapi-dwarf-enum.py"
- outputs = [ "$target_gen_dir/SBLanguages.h" ]
+ outputs = [ "$target_gen_dir/SourceLanguageNames.h" ]
sources = [ "//llvm/include/llvm/BinaryFormat/Dwarf.def" ]
args = [
rebase_path(sources[0], root_build_dir),
diff --git a/utils/bazel/llvm-project-overlay/lldb/BUILD.bazel b/utils/bazel/llvm-project-overlay/lldb/BUILD.bazel
index 3ed4f552290da4..e4e12d44406038 100644
--- a/utils/bazel/llvm-project-overlay/lldb/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/lldb/BUILD.bazel
@@ -195,7 +195,7 @@ py_binary(
genrule(
name = "lldb-sbapi-dwarf-enums",
srcs = ["//llvm:include/llvm/BinaryFormat/Dwarf.def"],
- outs = ["include/lldb/API/SBLanguages.h"],
+ outs = ["include/lldb/SourceLanguageNames.h"],
cmd = "$(location :generate-sbapi-dwarf-enum) $(location //llvm:include/llvm/BinaryFormat/Dwarf.def) --output $@",
tools = [":generate-sbapi-dwarf-enum"],
)
|
|
This looks better to me. I'm not sure why we were using defines from llvm::DWARF that seem to overlap the ones here, however. So I'll defer to Adrian or someone more knowledgeable about that side of things. |
Yeah, that wasn't clear to me either, why didn't we just use the types in |
|
I'm just wondering, would this patch run into the same issue that I ran into on #111907 where this could cause a clean build to fail since the |
Possibly, we'll see when it lands 😛 |
bulbazord
requested changes
Oct 11, 2024
|
I would feel a little worse about that if the API didn't explicitly say that the values of the enum might change meanings. So it explicitly says it isn't stable... |
medismailben
force-pushed
the
source-language-names
branch
from
1e40254 to
82b48c6
Compare
This patch moves `SBLanguages.h` out of the API tree. This file gets generated at build time using DWARF table-gen file and contains an enumeration of the DWARF supported source language names and there respective code/identifier. Since this is an enum, this shouldn't be part of the API tree but rather it should be included in the `lldb-enumeration` header. Also, that enum was used in internal methods in `lldb_private` by down-casting the enum value type to an `uint16_t`. This patch changes that by making those internal methods use the enum type. This should address the feedbacks from llvm#111907. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
force-pushed
the
source-language-names
branch
from
82b48c6 to
5744d94
Compare
bulbazord
reviewed
Oct 14, 2024
| @@ -136,6 +136,8 @@ class LLDB_API SBWatchpoint; | |||
| class LLDB_API SBWatchpointOptions; | |||
| class LLDB_API SBUnixSignals; | |||
|
|
|||
| typedef SourceLanguageName SBSourceLanguageName; | |||
There was a problem hiding this comment.
I'm not sure a typedef is enough here. It ends up being little more than syntactic sugar, the final symbol will still contain SourceLanguageName instead of SBSourceLanguageName. To verify this, you can perform a build with this patch and search for SBExpressionOptions::SetLanguage and make sure it still retains the SBSourceLanguageName symbol.
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 patch moves
SBLanguages.hout of the API tree. This file gets generated at build time using DWARF table-gen file and contains an enumeration of the DWARF supported source language names and there respective code/identifier.Since this is an enum, this shouldn't be part of the API tree but rather it should be included in the
lldb-enumerationheader.Also, that enum was used in internal methods in
lldb_privateby down-casting the enum value type to anuint16_t. This patch changes that by making those internal methods use the enum type.This should address the feedbacks from #111907.