-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lldb] Mark single-argument SourceLanguage constructors explicit #166527
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
[lldb] Mark single-argument SourceLanguage constructors explicit #166527
Conversation
This avoids unintentional comparisons between `SourceLanguage` and `LanguageType`. Also marks `operator bool` explicit so we don't implicitly convert to bool.
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis avoids unintentional comparisons between Also marks Full diff: https://github.com/llvm/llvm-project/pull/166527.diff 8 Files Affected:
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index b82a2b8aa0574..aff6702af2efa 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -102,18 +102,24 @@ struct RegisterSet {
/// A type-erased pair of llvm::dwarf::SourceLanguageName and version.
struct SourceLanguage {
SourceLanguage() = default;
- SourceLanguage(lldb::LanguageType language_type);
+ explicit SourceLanguage(lldb::LanguageType language_type);
+
SourceLanguage(uint16_t name, uint32_t version)
: name(name), version(version) {}
- SourceLanguage(std::optional<std::pair<uint16_t, uint32_t>> name_vers)
+
+ explicit SourceLanguage(
+ std::optional<std::pair<uint16_t, uint32_t>> name_vers)
: name(name_vers ? name_vers->first : 0),
version(name_vers ? name_vers->second : 0) {}
- operator bool() const { return name > 0; }
+
+ explicit operator bool() const { return name > 0; }
+
lldb::LanguageType AsLanguageType() const;
llvm::StringRef GetDescription() const;
bool IsC() const;
bool IsObjC() const;
bool IsCPlusPlus() const;
+ bool IsSwift() const;
uint16_t name = 0;
uint32_t version = 0;
};
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index f25209c15e007..25285beb7ffd5 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -251,7 +251,7 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
}
m_user_expression_sp.reset(GetTarget().GetUserExpressionForLanguage(
- condition.GetText(), llvm::StringRef(), language,
+ condition.GetText(), llvm::StringRef(), SourceLanguage{language},
Expression::eResultTypeAny, EvaluateExpressionOptions(), nullptr,
error));
if (error.Fail()) {
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 0d9eb45732161..df4c5c4ba6129 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -95,9 +95,9 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StackFrame *frame = m_exe_ctx.GetFramePtr();
// Either the language was explicitly specified, or we check the frame.
- lldb::LanguageType language = m_expr_options.language;
- if (language == lldb::eLanguageTypeUnknown && frame)
- language = frame->GuessLanguage().AsLanguageType();
+ SourceLanguage language{m_expr_options.language};
+ if (!language && frame)
+ language = frame->GuessLanguage();
// Add a hint if object description was requested, but no description
// function was implemented.
@@ -119,8 +119,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
"^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
if (GetDebugger().GetShowDontUsePoHint() && target_ptr &&
- (language == lldb::eLanguageTypeSwift ||
- language == lldb::eLanguageTypeObjC) &&
+ (language.IsSwift() || language.IsObjC()) &&
std::regex_match(output.data(), swift_class_regex)) {
result.AppendNote(
@@ -193,7 +192,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Second, try `expr` as a persistent variable.
if (expr.starts_with("$"))
- if (auto *state = target.GetPersistentExpressionStateForLanguage(language))
+ if (auto *state = target.GetPersistentExpressionStateForLanguage(
+ language.AsLanguageType()))
if (auto var_sp = state->GetVariable(expr))
if (auto valobj_sp = var_sp->GetValueObject()) {
dump_val_object(*valobj_sp);
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index af4b477660eeb..3d569d250f9d2 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -246,7 +246,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
// language in the target's properties if specified, else default to the
// langage for the frame.
if (!language) {
- if (target->GetLanguage() != lldb::eLanguageTypeUnknown)
+ if (!target->GetLanguage())
language = target->GetLanguage();
else if (StackFrame *frame = exe_ctx.GetFramePtr())
language = frame->GetLanguage();
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 990074566be7e..16795247aba68 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1502,7 +1502,7 @@ lldb_private::Status ClangExpressionParser::DoPrepareForExecution(
LLDB_LOGF(log, "%s - Current expression language is %s\n", __FUNCTION__,
lang.GetDescription().data());
lldb::ProcessSP process_sp = exe_ctx.GetProcessSP();
- if (process_sp && lang != lldb::eLanguageTypeUnknown) {
+ if (process_sp && !lang) {
auto runtime = process_sp->GetLanguageRuntime(lang.AsLanguageType());
if (runtime)
runtime->GetIRPasses(custom_passes);
diff --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index 8268d4ae4bb27..cc8563d6bd1c7 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -593,3 +593,7 @@ bool SourceLanguage::IsObjC() const {
bool SourceLanguage::IsCPlusPlus() const {
return name == llvm::dwarf::DW_LNAME_C_plus_plus;
}
+
+bool SourceLanguage::IsSwift() const {
+ return name == llvm::dwarf::DW_LNAME_Swift;
+}
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index 2ed58c5331df4..95b515412d693 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1344,18 +1344,18 @@ const char *StackFrame::GetDisplayFunctionName() {
SourceLanguage StackFrame::GetLanguage() {
CompileUnit *cu = GetSymbolContext(eSymbolContextCompUnit).comp_unit;
if (cu)
- return cu->GetLanguage();
+ return SourceLanguage{cu->GetLanguage()};
return {};
}
SourceLanguage StackFrame::GuessLanguage() {
SourceLanguage lang_type = GetLanguage();
- if (lang_type == eLanguageTypeUnknown) {
+ if (!lang_type) {
SymbolContext sc =
GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol);
if (sc.function)
- lang_type = LanguageType(sc.function->GetMangled().GuessLanguage());
+ lang_type = SourceLanguage(sc.function->GetMangled().GuessLanguage());
else if (sc.symbol)
lang_type = SourceLanguage(sc.symbol->GetMangled().GuessLanguage());
}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index a23091ad09c6d..e53fc7a1e1bda 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4945,7 +4945,7 @@ void TargetProperties::SetStandardErrorPath(llvm::StringRef path) {
SourceLanguage TargetProperties::GetLanguage() const {
const uint32_t idx = ePropertyLanguage;
- return {GetPropertyAtIndexAs<LanguageType>(idx, {})};
+ return SourceLanguage{GetPropertyAtIndexAs<LanguageType>(idx, {})};
}
llvm::StringRef TargetProperties::GetExpressionPrefixContents() {
|
| bool IsC() const; | ||
| bool IsObjC() const; | ||
| bool IsCPlusPlus() const; | ||
| bool IsSwift() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the Is... accessors made sense for merged languages like ObjC++, but Swift only has one dialect, so we could just compare the LNAME?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea makes sense!
This avoids unintentional comparisons between
SourceLanguageandLanguageType.Also marks
operator boolexplicit so we don't implicitly convert to bool.