diff --git a/lldb/include/lldb/DataFormatters/StringPrinter.h b/lldb/include/lldb/DataFormatters/StringPrinter.h --- a/lldb/include/lldb/DataFormatters/StringPrinter.h +++ b/lldb/include/lldb/DataFormatters/StringPrinter.h @@ -133,9 +133,9 @@ ReadBufferAndDumpToStreamOptions( const ReadStringAndDumpToStreamOptions &options); - void SetData(DataExtractor d) { m_data = d; } + void SetData(DataExtractor &&d) { m_data = std::move(d); } - lldb_private::DataExtractor GetData() const { return m_data; } + const lldb_private::DataExtractor &GetData() const { return m_data; } void SetIsTruncated(bool t) { m_is_truncated = t; } diff --git a/lldb/include/lldb/Utility/DataExtractor.h b/lldb/include/lldb/Utility/DataExtractor.h --- a/lldb/include/lldb/Utility/DataExtractor.h +++ b/lldb/include/lldb/Utility/DataExtractor.h @@ -134,7 +134,12 @@ DataExtractor(const DataExtractor &data, lldb::offset_t offset, lldb::offset_t length, uint32_t target_byte_size = 1); - DataExtractor(const DataExtractor &rhs); + /// Copy constructor. + /// + /// The copy constructor is explicit as otherwise it is easy to make + /// unintended modification of a local copy instead of a caller's instance. + /// Also a needless copy of the \a m_data_sp shared pointer is/ expensive. + explicit DataExtractor(const DataExtractor &rhs); /// Assignment operator. /// @@ -149,6 +154,12 @@ /// A const reference to this object. const DataExtractor &operator=(const DataExtractor &rhs); + /// Move constructor and move assignment operators to complete the rule of 5. + /// + /// They would get deleted as we already defined those of rule of 3. + DataExtractor(DataExtractor &&rhs) = default; + DataExtractor &operator=(DataExtractor &&rhs) = default; + /// Destructor /// /// If this object contains a valid shared data reference, the reference @@ -1005,7 +1016,8 @@ uint32_t m_addr_size; ///< The address size to use when extracting addresses. /// The shared pointer to data that can be shared among multiple instances lldb::DataBufferSP m_data_sp; - const uint32_t m_target_byte_size = 1; + /// Making it const would require implementation of move assignment operator. + uint32_t m_target_byte_size = 1; }; } // namespace lldb_private diff --git a/lldb/source/DataFormatters/StringPrinter.cpp b/lldb/source/DataFormatters/StringPrinter.cpp --- a/lldb/source/DataFormatters/StringPrinter.cpp +++ b/lldb/source/DataFormatters/StringPrinter.cpp @@ -475,11 +475,9 @@ return true; } - DataExtractor data(buffer_sp, process_sp->GetByteOrder(), - process_sp->GetAddressByteSize()); - StringPrinter::ReadBufferAndDumpToStreamOptions dump_options(options); - dump_options.SetData(data); + dump_options.SetData(DataExtractor(buffer_sp, process_sp->GetByteOrder(), + process_sp->GetAddressByteSize())); dump_options.SetSourceSize(sourceSize); dump_options.SetIsTruncated(is_truncated); dump_options.SetNeedsZeroTermination(needs_zero_terminator); diff --git a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp --- a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp @@ -168,7 +168,7 @@ stream.Printf("%s ", value.c_str()); StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - options.SetData(data); + options.SetData(std::move(data)); options.SetStream(&stream); options.SetPrefixToken("u8"); options.SetQuote('\''); @@ -194,7 +194,7 @@ stream.Printf("%s ", value.c_str()); StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - options.SetData(data); + options.SetData(std::move(data)); options.SetStream(&stream); options.SetPrefixToken("u"); options.SetQuote('\''); @@ -220,7 +220,7 @@ stream.Printf("%s ", value.c_str()); StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - options.SetData(data); + options.SetData(std::move(data)); options.SetStream(&stream); options.SetPrefixToken("U"); options.SetQuote('\''); @@ -254,7 +254,7 @@ const uint32_t wchar_size = *size; StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - options.SetData(data); + options.SetData(std::move(data)); options.SetStream(&stream); options.SetPrefixToken("L"); options.SetQuote('\''); diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -686,7 +686,7 @@ if (!wchar_t_size) return false; - options.SetData(extractor); + options.SetData(std::move(extractor)); options.SetStream(&stream); options.SetPrefixToken("L"); options.SetQuote('"'); @@ -743,12 +743,14 @@ } } - DataExtractor extractor; - const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size); - if (bytes_read < size) - return false; + { + DataExtractor extractor; + const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size); + if (bytes_read < size) + return false; - options.SetData(extractor); + options.SetData(std::move(extractor)); + } options.SetStream(&stream); if (prefix_token.empty()) options.SetPrefixToken(nullptr); diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h @@ -148,7 +148,7 @@ // Parse thread(s) data structures(prstatus, prpsinfo) from given NOTE segment llvm::Error ParseThreadContextsFromNoteSegment( const elf::ELFProgramHeader &segment_header, - lldb_private::DataExtractor segment_data); + const lldb_private::DataExtractor &segment_data); // Returns number of thread contexts stored in the core file uint32_t GetNumThreadContexts(); diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -519,9 +519,8 @@ size_t note_start = offset; size_t note_size = llvm::alignTo(note.n_descsz, 4); - DataExtractor note_data(segment, note_start, note_size); - result.push_back({note, note_data}); + result.push_back({note, DataExtractor(segment, note_start, note_size)}); offset += note_size; } @@ -897,7 +896,8 @@ /// A note segment consists of one or more NOTE entries, but their types and /// meaning differ depending on the OS. llvm::Error ProcessElfCore::ParseThreadContextsFromNoteSegment( - const elf::ELFProgramHeader &segment_header, DataExtractor segment_data) { + const elf::ELFProgramHeader &segment_header, + const DataExtractor &segment_data) { assert(segment_header.p_type == llvm::ELF::PT_NOTE); auto notes_or_error = parseSegment(segment_data); diff --git a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.cpp b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.cpp --- a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.cpp +++ b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.cpp @@ -34,5 +34,5 @@ uint32_t Type = *TypeOr; auto Iter = llvm::find_if( Notes, [Type](const CoreNote &Note) { return Note.info.n_type == Type; }); - return Iter == Notes.end() ? DataExtractor() : Iter->data; + return Iter == Notes.end() ? DataExtractor() : DataExtractor(Iter->data); } diff --git a/lldb/unittests/DataFormatter/StringPrinterTests.cpp b/lldb/unittests/DataFormatter/StringPrinterTests.cpp --- a/lldb/unittests/DataFormatter/StringPrinterTests.cpp +++ b/lldb/unittests/DataFormatter/StringPrinterTests.cpp @@ -37,9 +37,8 @@ opts.SetEscapeNonPrintables(true); opts.SetIgnoreMaxLength(false); opts.SetEscapeStyle(escape_style); - DataExtractor extractor(input.data(), input.size(), - endian::InlHostByteOrder(), sizeof(void *)); - opts.SetData(extractor); + opts.SetData(DataExtractor(input.data(), input.size(), + endian::InlHostByteOrder(), sizeof(void *))); const bool success = StringPrinter::ReadBufferAndDumpToStream(opts); if (!success) return llvm::None;