Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Standalone View
lldb/source/Core/Mangled.cpp
| Show First 20 Lines • Show All 64 Lines • ▼ Show 20 Lines | if (s) | ||||
| SetValue(s); | SetValue(s); | ||||
| } | } | ||||
| Mangled::Mangled(llvm::StringRef name) { | Mangled::Mangled(llvm::StringRef name) { | ||||
| if (!name.empty()) | if (!name.empty()) | ||||
| SetValue(ConstString(name)); | SetValue(ConstString(name)); | ||||
| } | } | ||||
| // Convert to pointer operator. This allows code to check any Mangled objects | // Convert to bool operator. This allows code to check any Mangled objects | ||||
| // to see if they contain anything valid using code such as: | // to see if they contain anything valid using code such as: | ||||
| // | // | ||||
| // Mangled mangled(...); | // Mangled mangled(...); | ||||
| // if (mangled) | // if (mangled) | ||||
| // { ... | // { ... | ||||
| Mangled::operator void *() const { | Mangled::operator bool() const { return m_mangled || m_demangled; } | ||||
clayborg: Looks like there is a similar bug here in this function as well. We should convert the convert… | |||||
This change breaks this test: lldb-shell :: SymbolFile/DWARF/x86/find-qualified-variable.cpp because the comment in the header file says: "This allows code to check a Mangled object to see if it contains a valid mangled name", which differs from the comment here, and apparently that's what people's intent was. Modifying lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2143 solves this. rZhBoYao: This change breaks this test: `lldb-shell :: SymbolFile/DWARF/x86/find-qualified-variable.cpp`… | |||||
| return (m_mangled) ? const_cast<Mangled *>(this) : nullptr; | |||||
| } | |||||
| // Logical NOT operator. This allows code to check any Mangled objects to see | // Logical NOT operator. This allows code to check any Mangled objects to see | ||||
| // if they are invalid using code such as: | // if they are invalid using code such as: | ||||
| // | // | ||||
| // Mangled mangled(...); | // Mangled mangled(...); | ||||
| // if (!file_spec) | // if (!file_spec) | ||||
| // { ... | // { ... | ||||
| bool Mangled::operator!() const { return !m_mangled; } | bool Mangled::operator!() const { return !m_mangled && !m_demangled; } | ||||
Should we remove Mangled::operator! altogether? I find it less confusing without. rZhBoYao: Should we remove `Mangled::operator!` altogether? I find it less confusing without. | |||||
Does !mangled work without it? Would it be sufficient to implement the ! operator in terms of bool conversion function? labath: Does `!mangled` work without it? Would it be sufficient to implement the `!` operator in terms… | |||||
Yes, I tried removing it and the unit test I added (TEST(MangledTest, LogicalNotOperator)) passed. Keeping it this way is fine tho. rZhBoYao: Yes, I tried removing it and the unit test I added (`TEST(MangledTest, LogicalNotOperator)`)… | |||||
In that case, you should definitely remove it. We don't want to have two implementations that need to be kept in sync. labath: In that case, you should definitely remove it. We don't want to have two implementations that… | |||||
| // Clear the mangled and demangled values. | // Clear the mangled and demangled values. | ||||
| void Mangled::Clear() { | void Mangled::Clear() { | ||||
| m_mangled.Clear(); | m_mangled.Clear(); | ||||
| m_demangled.Clear(); | m_demangled.Clear(); | ||||
| } | } | ||||
| // Compare the string values. | // Compare the string values. | ||||
| ▲ Show 20 Lines • Show All 426 Lines • Show Last 20 Lines | |||||
Looks like there is a similar bug here in this function as well. We should convert the convert to pointer function here to "operator bool" and add a unit test.
explicit operator bool() const { return m_mangled || m_demangled; }