Skip to content

Commit d9aa2f7

Browse files
committed
[lldb][TypeSystem] Remove count parameter from TypeSystem::GetEncoding (llvm#165702)
There were a couple of quirks with this parameter: 1. It wasn't being set consistently. E.g., vector types would be of count `1` but complex types would be `2`. Hence, it wasn't clear what count was referring to. 2. `count` was not being set if the input type was invalid, possibly leaving the input reference uninitialized. 3. Only one callsite actually made use of `count`, and that in itself seems like it could be improved (added a FIXME). If we ever need a "how many elements does this type represent", we can implement one with a new `TypeSystem` API that does exactly that. (cherry picked from commit a6eac9e)
1 parent 9959c2e commit d9aa2f7

File tree

8 files changed

+16
-24
lines changed

8 files changed

+16
-24
lines changed

lldb/include/lldb/Symbol/CompilerType.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ class CompilerType {
433433
std::optional<uint64_t>
434434
GetByteStride(ExecutionContextScope *exe_scope) const;
435435

436-
lldb::Encoding GetEncoding(uint64_t &count) const;
436+
lldb::Encoding GetEncoding() const;
437437

438438
lldb::Format GetFormat() const;
439439

lldb/include/lldb/Symbol/Type.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ class Type : public std::enable_shared_from_this<Type>, public UserID {
507507

508508
lldb::Format GetFormat();
509509

510-
lldb::Encoding GetEncoding(uint64_t &count);
510+
lldb::Encoding GetEncoding();
511511

512512
SymbolContextScope *GetSymbolContextScope() { return m_context; }
513513
const SymbolContextScope *GetSymbolContextScope() const { return m_context; }

lldb/include/lldb/Symbol/TypeSystem.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,7 @@ class TypeSystem : public PluginInterface,
349349
GetByteStride(lldb::opaque_compiler_type_t type,
350350
ExecutionContextScope *exe_scope) = 0;
351351

352-
virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
353-
uint64_t &count) = 0;
352+
virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type) = 0;
354353

355354
virtual lldb::Format GetFormat(lldb::opaque_compiler_type_t type) = 0;
356355

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4955,12 +4955,10 @@ TypeSystemClang::GetTypeBitAlign(lldb::opaque_compiler_type_t type,
49554955
return {};
49564956
}
49574957

4958-
lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
4959-
uint64_t &count) {
4958+
lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type) {
49604959
if (!type)
49614960
return lldb::eEncodingInvalid;
49624961

4963-
count = 1;
49644962
clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
49654963

49664964
switch (qual_type->getTypeClass()) {
@@ -4996,7 +4994,6 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
49964994
case clang::Type::DependentVector:
49974995
case clang::Type::ExtVector:
49984996
case clang::Type::Vector:
4999-
// TODO: Set this to more than one???
50004997
break;
50014998

50024999
case clang::Type::BitInt:
@@ -5196,11 +5193,10 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
51965193
const clang::ComplexType *complex_type =
51975194
qual_type->getAsComplexIntegerType();
51985195
if (complex_type)
5199-
encoding = GetType(complex_type->getElementType()).GetEncoding(count);
5196+
encoding = GetType(complex_type->getElementType()).GetEncoding();
52005197
else
52015198
encoding = lldb::eEncodingSint;
52025199
}
5203-
count = 2;
52045200
return encoding;
52055201
}
52065202

@@ -5256,7 +5252,7 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
52565252
case clang::Type::HLSLInlineSpirv:
52575253
break;
52585254
}
5259-
count = 0;
5255+
52605256
return lldb::eEncodingInvalid;
52615257
}
52625258

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -907,8 +907,7 @@ class TypeSystemClang : public TypeSystem {
907907
return {};
908908
}
909909

910-
lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
911-
uint64_t &count) override;
910+
lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type) override;
912911

913912
lldb::Format GetFormat(lldb::opaque_compiler_type_t type) override;
914913

lldb/source/Symbol/CompilerType.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -841,10 +841,10 @@ CompilerType::GetTypeBitAlign(ExecutionContextScope *exe_scope) const {
841841
return {};
842842
}
843843

844-
lldb::Encoding CompilerType::GetEncoding(uint64_t &count) const {
844+
lldb::Encoding CompilerType::GetEncoding() const {
845845
if (IsValid())
846846
if (auto type_system_sp = GetTypeSystem())
847-
return type_system_sp->GetEncoding(m_type, count);
847+
return type_system_sp->GetEncoding(m_type);
848848
return lldb::eEncodingInvalid;
849849
}
850850

@@ -1146,10 +1146,10 @@ bool CompilerType::GetValueAsScalar(const lldb_private::DataExtractor &data,
11461146
if (0 == (GetTypeInfo() & eTypeHasValue)) {
11471147
return false; // Aggregate types don't have scalar values
11481148
} else {
1149-
uint64_t count = 0;
1150-
lldb::Encoding encoding = GetEncoding(count);
1149+
// FIXME: check that type is scalar instead of checking encoding?
1150+
lldb::Encoding encoding = GetEncoding();
11511151

1152-
if (encoding == lldb::eEncodingInvalid || count != 1)
1152+
if (encoding == lldb::eEncodingInvalid || (GetTypeInfo() & eTypeIsComplex))
11531153
return false;
11541154

11551155
auto byte_size_or_err = GetByteSize(exe_scope);

lldb/source/Symbol/Type.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,9 @@ lldb::TypeSP Type::GetTypedefType() {
531531

532532
lldb::Format Type::GetFormat() { return GetForwardCompilerType().GetFormat(); }
533533

534-
lldb::Encoding Type::GetEncoding(uint64_t &count) {
534+
lldb::Encoding Type::GetEncoding() {
535535
// Make sure we resolve our type if it already hasn't been.
536-
return GetForwardCompilerType().GetEncoding(count);
536+
return GetForwardCompilerType().GetEncoding();
537537
}
538538

539539
bool Type::ReadFromMemory(ExecutionContext *exe_ctx, lldb::addr_t addr,

lldb/source/ValueObject/ValueObject.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -801,8 +801,7 @@ bool ValueObject::SetData(DataExtractor &data, Status &error) {
801801
return false;
802802
}
803803

804-
uint64_t count = 0;
805-
const Encoding encoding = GetCompilerType().GetEncoding(count);
804+
const Encoding encoding = GetCompilerType().GetEncoding();
806805

807806
const size_t byte_size = llvm::expectedToOptional(GetByteSize()).value_or(0);
808807

@@ -1679,8 +1678,7 @@ bool ValueObject::SetValueFromCString(const char *value_str, Status &error) {
16791678
return false;
16801679
}
16811680

1682-
uint64_t count = 0;
1683-
const Encoding encoding = GetCompilerType().GetEncoding(count);
1681+
const Encoding encoding = GetCompilerType().GetEncoding();
16841682

16851683
const size_t byte_size = llvm::expectedToOptional(GetByteSize()).value_or(0);
16861684

0 commit comments

Comments
 (0)