Skip to content

Fix: Miscellaneous Clang warnings #781

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.

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
748 changes: 429 additions & 319 deletions src/applications/bmqstoragetool/m_bmqstoragetool_cslprinter.cpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,14 @@ static void test1_humanReadableShortResultTest()
// Prepare expected output
bmqu::MemOutStream expectedStream(bmqtst::TestHelperUtil::allocator());
{
bslim::Printer printer(&expectedStream, 0, -1);
printer.start();
printer.printAttribute("recordType", header.recordType());
printer.printAttribute("electorTerm", header.electorTerm());
printer.printAttribute("sequenceNumber", header.sequenceNumber());
printer.printAttribute("timestamp", header.timestamp());
printer.end();
bslim::Printer expectedPrinter(&expectedStream, 0, -1);
expectedPrinter.start();
expectedPrinter.printAttribute("recordType", header.recordType());
expectedPrinter.printAttribute("electorTerm", header.electorTerm());
expectedPrinter.printAttribute("sequenceNumber",
header.sequenceNumber());
expectedPrinter.printAttribute("timestamp", header.timestamp());
expectedPrinter.end();
expectedStream << recordId << '\n';
}

Expand Down Expand Up @@ -134,10 +135,12 @@ static void test2_humanReadableDetailResultTest()
bmqu::MemOutStream recordStream(bmqtst::TestHelperUtil::allocator());
record.print(recordStream, 2, 2);

CslRecordPrinter<bmqu::AlignedPrinter> printer(
CslRecordPrinter<bmqu::AlignedPrinter> expectedPrinter(
expectedStream,
bmqtst::TestHelperUtil::allocator());
printer.printRecordDetails(recordStream.str(), header, recordId);
expectedPrinter.printRecordDetails(recordStream.str(),
header,
recordId);
}

BMQTST_ASSERT_EQ(expectedStream.str(), resultStream.str());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ bool FileManagerImpl::CslFileHandler::resetIterator(

if (d_cslFromBegin) {
// Move iterator to the first record.
const int rc = d_iter_p->next();
rc = d_iter_p->next();
if (rc != 0) {
errorDescription << "CSL file either empty or corrupted: rc="
<< rc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ inline FileManagerImpl::FileHandler<ITER>::FileHandler(
}

template <typename ITER>
inline FileManagerImpl::FileHandler<ITER>::~FileHandler()
inline FileManagerImpl::FileHandler<ITER>::FileHandler::~FileHandler()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this syntax doesn't compile on C++03, I believe I've been burned by this before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll give this a shot in C++03 then; if it doesn't compile, we'll have to turn that warning off.

I see in the git log that we also tried:

inline FileManagerImpl::FileHandler<ITER>::~FileHandler<ITER>()

which also doesn't compile in C++03.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We shouldn't need this to compile in C++03: the storage tool is written in C++11, and won't compile in C++03. But our check for this only makes sure that we're not building on Solaris: building all in C++03 on Linux will fail with some C++11-isms in the storage tool. I'll fix the check to make sure CMAKE_CXX_STANDARD is C++11 or better.

But either way, clang doesn't complain about this line in C++03.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we solve this issue completely by placing the nested class declaration to the outer scope?

{
d_iter.clear();
if (d_mfd.isValid()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ Filters::Filters(const bsl::vector<bsl::string>& queueKeys,
}
}
if (!queueUris.empty()) {
mqbu::StorageKey key;
bsl::vector<bsl::string>::const_iterator uriIt = queueUris.cbegin();
for (; uriIt != queueUris.cend(); ++uriIt) {
bsl::optional<mqbu::StorageKey> key = queueMap.findKeyByUri(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ namespace BloombergLP::m_bmqstoragetool {
// failures.

/// Print the specified 'details' to the specified 'stream'
void PrintTo(const QueueDetails& details, bsl::ostream* stream)
inline void PrintTo(const QueueDetails& details, bsl::ostream* stream)
{
*stream << "\trecordsNumber : " << details.d_recordsNumber
<< "\n\tmessageRecordsNumber : " << details.d_messageRecordsNumber
Expand All @@ -324,7 +324,7 @@ void PrintTo(const QueueDetails& details, bsl::ostream* stream)
}

/// Print the specified 'detailsMap' to the specified 'stream'
void PrintTo(const QueueDetailsMap& detailsMap, std::ostream* stream)
inline void PrintTo(const QueueDetailsMap& detailsMap, std::ostream* stream)
{
for (const auto& d : detailsMap) {
*stream << d.first << " :\n";
Expand Down
Loading
Loading