-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb][Language] Simplify SourceLanguage::GetDescription #161804
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][Language] Simplify SourceLanguage::GetDescription #161804
Conversation
Currently we don't benefit from the user-friendly names that `LanguageDescription` returns because we would always use `Language::GetNameForLanguageType`. I'm not aware of a situation where `GetDescription` should prefer the non-human readable form of the name with. This patch removes the call to `GetNameForLanguageType`.
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesCurrently we don't benefit from the user-friendly names that Full diff: https://github.com/llvm/llvm-project/pull/161804.diff 1 Files Affected:
diff --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index 484d9badde397..7ce85dd93f091 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -559,11 +559,8 @@ lldb::LanguageType SourceLanguage::AsLanguageType() const {
}
llvm::StringRef SourceLanguage::GetDescription() const {
- LanguageType type = AsLanguageType();
- if (type)
- return Language::GetNameForLanguageType(type);
return llvm::dwarf::LanguageDescription(
- (llvm::dwarf::SourceLanguageName)name);
+ static_cast<llvm::dwarf::SourceLanguageName>(name));
}
bool SourceLanguage::IsC() const { return name == llvm::dwarf::DW_LNAME_C; }
|
89bda08
to
29d911d
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
65aceb1
to
bc4952c
Compare
lldb/source/Target/Language.cpp
Outdated
default: break; | ||
} | ||
|
||
llvm_unreachable("Unhandled language type"); |
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.
Are we sure we cannot get here even if. we parse DWARF with e.g., a DW_LANG_user_low+n language attribute?
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.
Otherwise a fail-safe that doesn't crash might be more appropriate.
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.
true if the debuggee was compiled with a compiler that emitted a vendor language that LLDB isn't aware of. That would hit this codepath. Let me relax this a bit
…1803) The intention for this API is to be used when presenting language names to the user, e.g., in expression evaluation diagnostics or LLDB errors. Most uses of `GetNameForLanguageType` can be probably replaced with `GetDisplayNameForLanguageType`, but that's out of scope of this PR. This uses `llvm::dwarf::LanguageDescription` under the hood, so we would lose the version numbers in the names. If we deem those to be important, we could switch to an implementation that hardcodes a list of user-friendly names with version numbers included. The intention is to use it from #161688 Depends on #161804
…pe API (#161803) The intention for this API is to be used when presenting language names to the user, e.g., in expression evaluation diagnostics or LLDB errors. Most uses of `GetNameForLanguageType` can be probably replaced with `GetDisplayNameForLanguageType`, but that's out of scope of this PR. This uses `llvm::dwarf::LanguageDescription` under the hood, so we would lose the version numbers in the names. If we deem those to be important, we could switch to an implementation that hardcodes a list of user-friendly names with version numbers included. The intention is to use it from llvm/llvm-project#161688 Depends on llvm/llvm-project#161804
Currently we don't benefit from the user-friendly names that
LanguageDescription
returns because we would always useLanguage::GetNameForLanguageType
. I'm not aware of a situation whereGetDescription
should prefer the non-human readable form of the name with. This patch removes the call toGetNameForLanguageType
.LanguageDescription
already handles languages that it doesn't know about. For those it would returnUnknown
. The LLDB language types should all be available via DWARF. If there are languages that don't map cleanly betweenlldb::LanguageType
andDW_LANG
, then we should add explicit support for that in theSourceLanguage::SourceLanguage
constructor.