Skip to content

[lldb] Fix latent compiler warnings #1775

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 10, 2020
Merged

[lldb] Fix latent compiler warnings #1775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93587f5
[lldb] Fix latent compiler warnings
kastiglione Sep 10, 2020
6cb55bb
redo if-assignment fix
kastiglione Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,10 +1001,10 @@ void PrintMatrix(Stream &stream,
int num_columns, int num_rows) {
// Print each row.
stream.Printf("\n[ ");
for (unsigned J = 0; J < num_rows; ++J) {
for (int J = 0; J < num_rows; ++J) {
// Join the J-th row's elements with commas.
std::vector<std::string> row;
for (unsigned I = 0; I < num_columns; ++I)
for (int I = 0; I < num_columns; ++I)
row.emplace_back(std::move(matrix[I][J]));
std::string joined = llvm::join(row, ", ");

Expand Down Expand Up @@ -1069,14 +1069,14 @@ bool lldb_private::formatters::swift::SIMDVector_SummaryProvider(
ConstString full_type_name = simd_type.GetTypeName();
llvm::StringRef type_name = full_type_name.GetStringRef();
uint64_t num_elements = type_size / arg_size;
int generic_pos = type_name.find("<");
auto generic_pos = type_name.find("<");
if (generic_pos != llvm::StringRef::npos)
type_name = type_name.slice(0, generic_pos);
if (type_name == "Swift.SIMD3")
num_elements = 3;

std::vector<std::string> elem_vector;
for (int i = 0; i < num_elements; ++i) {
for (uint64_t i = 0; i < num_elements; ++i) {
DataExtractor elem_extractor(storage_buf, i * arg_size, arg_size);
auto simd_elem = ValueObject::CreateValueObjectFromData(
"simd_elem", elem_extractor, valobj.GetExecutionContextRef(), arg_type);
Expand Down
2 changes: 0 additions & 2 deletions lldb/source/Plugins/Language/Swift/SwiftOptionSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ void lldb_private::formatters::swift::SwiftOptionSetSummaryProvider::
return;
}

SwiftASTContext *swift_ast_ctx =
llvm::dyn_cast_or_null<SwiftASTContext>(m_type.GetTypeSystem());
auto iter = enum_decl->enumerator_begin(), end = enum_decl->enumerator_end();
for (; iter != end; ++iter) {
clang::EnumConstantDecl *case_decl = *iter;
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Plugins/Language/Swift/SwiftUnsafeTypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class SwiftUnsafeType {
UnsafePointerKind GetKind() const { return m_kind; }
virtual bool Update() = 0;

virtual ~SwiftUnsafeType() = default;

protected:
SwiftUnsafeType(ValueObject &valobj, UnsafePointerKind kind);
addr_t GetAddress(llvm::StringRef child_name);
Expand Down Expand Up @@ -531,7 +533,7 @@ bool lldb_private::formatters::swift::UnsafeTypeSyntheticFrontEnd::Update() {

for (size_t i = 0; i < num_children; i++) {
StreamString idx_name;
idx_name.Printf("[%" PRIu64 "]", i);
idx_name.Printf("[%zu]", i);
DataExtractor data(buffer_data, i * m_element_stride, m_element_stride);
m_children.push_back(CreateValueObjectFromData(
idx_name.GetString(), data, m_exe_ctx_ref, element_type));
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ class ObjectFileELF : public lldb_private::ObjectFile {
std::shared_ptr<ObjectFileELF> GetGnuDebugDataObjectFile();

llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section);
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;
};

#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_ELF_OBJECTFILEELF_H
2 changes: 1 addition & 1 deletion lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
bool SectionIsLoadable(const lldb_private::Section *section);

llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section);
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

llvm::MachO::mach_header m_header;
static lldb_private::ConstString GetSegmentNameTEXT();
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
const section_header_t &sect);

llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section);
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

typedef std::vector<section_header_t> SectionHeaderColl;
typedef SectionHeaderColl::iterator SectionHeaderCollIter;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ void ManualDWARFIndex::Index() {
// done indexing to make sure we don't pull in all DWARF dies, but we need
// to wait until all compile units have been indexed in case a DIE in one
// compile unit refers to another and the indexes accesses those DIEs.
for (int i=0; i<units_to_index.size(); ++i)
extract_fn(i);
for (size_t i = 0; i < units_to_index.size(); ++i)
extract_fn(i);
// This call can deadlock because we are sometimes holding the module lock.
// for (size_t i = 0; i < units_to_index.size(); ++i)
// pool.async(extract_fn, i);
Expand Down
19 changes: 10 additions & 9 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1262,8 +1262,6 @@ static const char *getImportFailureString(swift::serialization::Status status) {
case swift::serialization::Status::TargetTooNew:
return "The module file was built for a target newer than the current "
"target.";
default:
return "An unknown error occurred.";
}
}

Expand Down Expand Up @@ -2976,8 +2974,6 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
// Not implemented since Objective-C protocols aren't yet
// described in DWARF.
return true;
default:
return true;
}
}

Expand Down Expand Up @@ -3192,7 +3188,7 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
if (results.size())
break;
}
LOG_PRINTF(LIBLLDB_LOG_TYPES, "%d types collected.", results.size());
LOG_PRINTF(LIBLLDB_LOG_TYPES, "%zu types collected.", results.size());
return;
}

Expand Down Expand Up @@ -3223,7 +3219,7 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
return true;
});

LOG_PRINTF(LIBLLDB_LOG_TYPES, "%d types from debug info.", results.size());
LOG_PRINTF(LIBLLDB_LOG_TYPES, "%zu types from debug info.", results.size());
}
};
} // namespace lldb_private
Expand Down Expand Up @@ -3552,8 +3548,7 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
}

if (!module_decl) {
LOG_PRINTF(LIBLLDB_LOG_TYPES, "failed with no error",
module.path.front().GetCString());
LOG_PRINTF(LIBLLDB_LOG_TYPES, "failed with no error");

error.SetErrorStringWithFormat(
"failed to get module \"%s\" from AST context",
Expand Down Expand Up @@ -6839,7 +6834,7 @@ static llvm::Optional<uint64_t> GetInstanceVariableOffset_Metadata(
llvm::Optional<uint64_t> offset = runtime->GetMemberVariableOffset(
type, valobj, ConstString(ivar_name), &error);
if (offset)
LOG_PRINTF(LIBLLDB_LOG_TYPES, "for %s: %lu", ivar_name.str().c_str(),
LOG_PRINTF(LIBLLDB_LOG_TYPES, "for %s: %llu", ivar_name.str().c_str(),
*offset);
else
LOG_PRINTF(LIBLLDB_LOG_TYPES, "resolver failure: %s", error.AsCString());
Expand Down Expand Up @@ -8139,10 +8134,16 @@ static void DescribeFileUnit(Stream &s, swift::FileUnit *file_unit) {
switch (source_file->Kind) {
case swift::SourceFileKind::Library:
s.PutCString("Library");
break;
case swift::SourceFileKind::Main:
s.PutCString("Main");
break;
case swift::SourceFileKind::SIL:
s.PutCString("SIL");
break;
case swift::SourceFileKind::Interface:
s.PutCString("Interface");
break;
}
}
} break;
Expand Down
5 changes: 3 additions & 2 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,10 @@ class SwiftASTContext : public TypeSystemSwift {
swift::ClangImporter *GetClangImporter();
swift::DWARFImporterDelegate *GetDWARFImporterDelegate();

CompilerType CreateTupleType(const std::vector<TupleElement> &elements);
CompilerType
CreateTupleType(const std::vector<TupleElement> &elements) override;

CompilerType GetErrorType();
CompilerType GetErrorType() override;

bool HasErrors();

Expand Down
1 change: 1 addition & 0 deletions lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwift.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class TypeSystemSwift : public TypeSystem {
};
virtual CompilerType
CreateTupleType(const std::vector<TupleElement> &elements) = 0;
using TypeSystem::DumpTypeDescription;
virtual void DumpTypeDescription(
lldb::opaque_compiler_type_t type, bool print_help_if_available,
bool print_extensions_if_available,
Expand Down
12 changes: 6 additions & 6 deletions lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,15 +586,16 @@ swift::Demangle::NodePointer TypeSystemSwiftTypeRef::GetNodeForPrintingImpl(
NodePointer args_ty = Dem.createNode(Node::Kind::Type);
NodePointer args_tuple = Dem.createNode(Node::Kind::Tuple);
for (NodePointer child : *node) {
if (child->getKind() == Node::Kind::ImplParameter)
if (child->getKind() == Node::Kind::ImplParameter) {
for (NodePointer type : *node)
if (type->getKind() == Node::Kind::Type &&
type->getNumChildren() == 1)
rett->addChild(type->getChild(0), Dem);
else if (child->getKind() == Node::Kind::ImplResult)
} else if (child->getKind() == Node::Kind::ImplResult) {
for (NodePointer type : *node)
if (type->getKind() == Node::Kind::Type)
rett->addChild(type, Dem);
}
}
args_ty->addChild(args_tuple, Dem);
args->addChild(args_ty, Dem);
Expand Down Expand Up @@ -759,7 +760,6 @@ static uint32_t collectTypeInfo(Module *M, swift::Demangle::Demangler &Dem,
// Bug-for-bug-compatibility. Not sure if this is correct.
swift_flags |= eTypeIsPointer | eTypeHasValue;
return swift_flags;
LLVM_FALLTHROUGH;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the cases in this switch use a fallthrough. Adrian do you know if it's correct for this one to be a return, not a fall through?

case Node::Kind::BoundGenericFunction:
swift_flags |= eTypeIsGeneric | eTypeIsBound;
LLVM_FALLTHROUGH;
Expand Down Expand Up @@ -1779,7 +1779,6 @@ bool TypeSystemSwiftTypeRef::IsMeaninglessWithoutDynamicResolution(

CompilerType TypeSystemSwiftTypeRef::GetAsClangTypeOrNull(
lldb::opaque_compiler_type_t type) {
CompilerType clang_type;
using namespace swift::Demangle;
Demangler Dem;
NodePointer node = GetDemangledType(Dem, AsMangledName(type));
Expand All @@ -1792,9 +1791,10 @@ CompilerType TypeSystemSwiftTypeRef::GetAsClangTypeOrNull(
node->getChild(1)->hasText()) {
auto node_clangtype = ResolveTypeAlias(GetModule(), Dem, node,
/*prefer_clang_types*/ true);
if (clang_type = node_clangtype.second)
return clang_type;
if (node_clangtype.second)
return node_clangtype.second;
}
CompilerType clang_type;
IsImportedType(type, &clang_type);
return clang_type;
}
Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Symbol/ObjectFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,7 @@ void llvm::format_provider<ObjectFile::Strata>::format(

llvm::StringRef ObjectFile::GetReflectionSectionIdentifier(
swift::ReflectionSectionKind section) {
assert("Base class's GetReflectionSectionIdentifier should not be called");
assert(false &&
"Base class's GetReflectionSectionIdentifier should not be called");
return "";
}
14 changes: 13 additions & 1 deletion lldb/source/Target/SwiftLanguageRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ void SwiftLanguageRuntimeImpl::SetupExclusivity() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
if (log)
log->Printf(
"SwiftLanguageRuntime: _swift_disableExclusivityChecking = %lu",
"SwiftLanguageRuntime: _swift_disableExclusivityChecking = %llu",
m_dynamic_exclusivity_flag_addr ? *m_dynamic_exclusivity_flag_addr : 0);
}

Expand Down Expand Up @@ -575,6 +575,10 @@ static bool GetObjectDescription_ResultVariable(Process &process, Stream &str,
log->Printf(
"[GetObjectDescription_ResultVariable] eExpressionStoppedForDebug");
break;
case eExpressionThreadVanished:
log->Printf(
"[GetObjectDescription_ResultVariable] eExpressionThreadVanished");
break;
}
}

Expand Down Expand Up @@ -682,6 +686,10 @@ static bool GetObjectDescription_ObjectReference(Process &process, Stream &str,
log->Printf(
"[GetObjectDescription_ObjectReference] eExpressionStoppedForDebug");
break;
case eExpressionThreadVanished:
log->Printf(
"[GetObjectDescription_ObjectReference] eExpressionThreadVanished");
break;
}
}

Expand Down Expand Up @@ -840,6 +848,10 @@ static bool GetObjectDescription_ObjectCopy(SwiftLanguageRuntimeImpl *runtime,
log->Printf(
"[GetObjectDescription_ObjectCopy] eExpressionStoppedForDebug");
break;
case eExpressionThreadVanished:
log->Printf(
"[GetObjectDescription_ObjectCopy] eExpressionThreadVanished");
break;
}
}

Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Class(
auto metadata_address = remote_ast.getHeapMetadataForObject(instance_address);
if (!metadata_address) {
if (log) {
log->Printf("could not read heap metadata for object at %lu: %s\n",
log->Printf("could not read heap metadata for object at %llu: %s\n",
class_metadata_ptr,
metadata_address.getFailure().render().c_str());
}
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Target/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,7 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language,
auto type_system_or_err = m_scratch_type_system_map.GetTypeSystemForLanguage(
language, this, create_on_demand, compiler_options);
if (!type_system_or_err)
return std::move(type_system_or_err.takeError());
return type_system_or_err.takeError();

#ifdef LLDB_ENABLE_SWIFT
if (language == eLanguageTypeSwift) {
Expand Down Expand Up @@ -2249,7 +2249,7 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language,
type_system_or_err = m_scratch_type_system_map.GetTypeSystemForLanguage(
language, this, create_on_demand, compiler_options);
if (!type_system_or_err)
return std::move(type_system_or_err.takeError());
return type_system_or_err.takeError();

if (auto *new_swift_ast_ctx =
llvm::dyn_cast_or_null<SwiftASTContextForExpressions>(
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Target/ThreadPlanCallFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ bool ThreadPlanCallFunction::BreakpointsExplainStop() {
#ifdef LLDB_ENABLE_SWIFT
ConstString persistent_variable_name(
persistent_state->GetNextPersistentVariableName(/*is_error*/ true));
if (m_return_valobj_sp = SwiftLanguageRuntime::CalculateErrorValue(
frame_sp, persistent_variable_name)) {
if ((m_return_valobj_sp = SwiftLanguageRuntime::CalculateErrorValue(
frame_sp, persistent_variable_name))) {
Comment on lines +499 to +500
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another case of assignment inside an if. This one looks like it should be assignment, not equality.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the problem here that it's not a declaration? I.e., would if (auto m_return_valobj_sp = SwiftLa...) trigger a warning?

Copy link
Author

@kastiglione kastiglione Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, a declaration would not trigger the warning. I don't know the history, but it seems clang uses additional parenthesis as a way of saying "this assignment inside an if is intentional". I see there are a number of uses of this pattern in swift and llvm.


DataExtractor data;
Status data_error;
Expand Down