Skip to content

Commit 1bb089a

Browse files
Michael137Debadri Basak
authored andcommitted
[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.
1 parent e95e64f commit 1bb089a

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
@@ -400,7 +400,7 @@ class CompilerType {
400400
/// Return the size of the type in bits.
401401
llvm::Expected<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
402402

403-
lldb::Encoding GetEncoding(uint64_t &count) const;
403+
lldb::Encoding GetEncoding() const;
404404

405405
lldb::Format GetFormat() const;
406406

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
@@ -317,8 +317,7 @@ class TypeSystem : public PluginInterface,
317317
GetBitSize(lldb::opaque_compiler_type_t type,
318318
ExecutionContextScope *exe_scope) = 0;
319319

320-
virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
321-
uint64_t &count) = 0;
320+
virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type) = 0;
322321

323322
virtual lldb::Format GetFormat(lldb::opaque_compiler_type_t type) = 0;
324323

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4864,12 +4864,10 @@ TypeSystemClang::GetTypeBitAlign(lldb::opaque_compiler_type_t type,
48644864
return {};
48654865
}
48664866

4867-
lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
4868-
uint64_t &count) {
4867+
lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type) {
48694868
if (!type)
48704869
return lldb::eEncodingInvalid;
48714870

4872-
count = 1;
48734871
clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
48744872

48754873
switch (qual_type->getTypeClass()) {
@@ -4903,7 +4901,6 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
49034901
case clang::Type::DependentVector:
49044902
case clang::Type::ExtVector:
49054903
case clang::Type::Vector:
4906-
// TODO: Set this to more than one???
49074904
break;
49084905

49094906
case clang::Type::BitInt:
@@ -5104,11 +5101,10 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
51045101
const clang::ComplexType *complex_type =
51055102
qual_type->getAsComplexIntegerType();
51065103
if (complex_type)
5107-
encoding = GetType(complex_type->getElementType()).GetEncoding(count);
5104+
encoding = GetType(complex_type->getElementType()).GetEncoding();
51085105
else
51095106
encoding = lldb::eEncodingSint;
51105107
}
5111-
count = 2;
51125108
return encoding;
51135109
}
51145110

@@ -5165,7 +5161,7 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
51655161
case clang::Type::SubstBuiltinTemplatePack:
51665162
break;
51675163
}
5168-
count = 0;
5164+
51695165
return lldb::eEncodingInvalid;
51705166
}
51715167

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -837,8 +837,7 @@ class TypeSystemClang : public TypeSystem {
837837
GetBitSize(lldb::opaque_compiler_type_t type,
838838
ExecutionContextScope *exe_scope) override;
839839

840-
lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
841-
uint64_t &count) override;
840+
lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type) override;
842841

843842
lldb::Format GetFormat(lldb::opaque_compiler_type_t type) override;
844843

lldb/source/Symbol/CompilerType.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -793,10 +793,10 @@ CompilerType::GetTypeBitAlign(ExecutionContextScope *exe_scope) const {
793793
return {};
794794
}
795795

796-
lldb::Encoding CompilerType::GetEncoding(uint64_t &count) const {
796+
lldb::Encoding CompilerType::GetEncoding() const {
797797
if (IsValid())
798798
if (auto type_system_sp = GetTypeSystem())
799-
return type_system_sp->GetEncoding(m_type, count);
799+
return type_system_sp->GetEncoding(m_type);
800800
return lldb::eEncodingInvalid;
801801
}
802802

@@ -1093,10 +1093,10 @@ bool CompilerType::GetValueAsScalar(const lldb_private::DataExtractor &data,
10931093
if (IsAggregateType()) {
10941094
return false; // Aggregate types don't have scalar values
10951095
} else {
1096-
uint64_t count = 0;
1097-
lldb::Encoding encoding = GetEncoding(count);
1096+
// FIXME: check that type is scalar instead of checking encoding?
1097+
lldb::Encoding encoding = GetEncoding();
10981098

1099-
if (encoding == lldb::eEncodingInvalid || count != 1)
1099+
if (encoding == lldb::eEncodingInvalid || (GetTypeInfo() & eTypeIsComplex))
11001100
return false;
11011101

11021102
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
@@ -790,8 +790,7 @@ bool ValueObject::SetData(DataExtractor &data, Status &error) {
790790
return false;
791791
}
792792

793-
uint64_t count = 0;
794-
const Encoding encoding = GetCompilerType().GetEncoding(count);
793+
const Encoding encoding = GetCompilerType().GetEncoding();
795794

796795
const size_t byte_size = llvm::expectedToOptional(GetByteSize()).value_or(0);
797796

@@ -1669,8 +1668,7 @@ bool ValueObject::SetValueFromCString(const char *value_str, Status &error) {
16691668
return false;
16701669
}
16711670

1672-
uint64_t count = 0;
1673-
const Encoding encoding = GetCompilerType().GetEncoding(count);
1671+
const Encoding encoding = GetCompilerType().GetEncoding();
16741672

16751673
const size_t byte_size = llvm::expectedToOptional(GetByteSize()).value_or(0);
16761674

0 commit comments

Comments
 (0)