From 0e89b2dbea1c16fc5a82a70cefa888090835a684 Mon Sep 17 00:00:00 2001 From: Greg Landrum Date: Fri, 26 Feb 2021 05:10:02 +0100 Subject: [PATCH] cleanup a bunch of compiler warnings (#3849) * cleanup a bunch of g++ warnings * make it work with clang * remove some additional warnings based on CI builds * fix that version number * stop being verbose when building --- .azure-pipelines/linux_build_java.yml | 2 +- Code/Geometry/point.h | 9 +++++++++ .../GraphMol/ChemTransforms/MolFragmenter.cpp | 2 +- .../DistGeomHelpers/testDgeomHelpers.cpp | 1 + Code/GraphMol/FileParsers/MolSGroupWriting.h | 18 ++++++++--------- Code/GraphMol/FileParsers/testMolSupplier.cpp | 1 + .../Fingerprints/MorganFingerprints.cpp | 3 +++ .../GraphMol/Fingerprints/MorganGenerator.cpp | 2 ++ .../Fingerprints/PatternFingerprints.cpp | 2 +- Code/GraphMol/MolDraw2D/MolDraw2D.cpp | 17 +++++++--------- Code/GraphMol/MolInterchange/Parser.cpp | 9 +++++++++ Code/GraphMol/QueryOps.cpp | 4 ++-- Code/GraphMol/RGroupDecomposition/RGroupGa.h | 6 ++++-- .../RGroupDecomposition/RGroupUtils.cpp | 4 ++-- .../RGroupDecomposition/testRGroupDecomp.cpp | 2 +- Code/GraphMol/Subgraphs/SubgraphUtils.h | 3 +++ .../SubstructLibrarySerialization.h | 4 ++-- Code/RDBoost/python_streambuf.h | 10 +++++----- Code/RDGeneral/BoostStartInclude.h | 3 +++ .../GA/ga/BinaryStringChromosomePolicy.cpp | 20 ++++++++++--------- External/GA/ga/LinkedPopLinearSel.h | 2 ++ External/INCHI-API/inchi.cpp | 8 ++++---- 22 files changed, 82 insertions(+), 50 deletions(-) diff --git a/.azure-pipelines/linux_build_java.yml b/.azure-pipelines/linux_build_java.yml index e121756eb..eb67056ea 100644 --- a/.azure-pipelines/linux_build_java.yml +++ b/.azure-pipelines/linux_build_java.yml @@ -38,7 +38,7 @@ steps: displayName: Configure build (Run CMake) - bash: | cd build - VERBOSE=1 make -j $( $(number_of_cores) ) install + make -j $( $(number_of_cores) ) install displayName: Build - bash: | export RDBASE=`pwd` diff --git a/Code/Geometry/point.h b/Code/Geometry/point.h index c3154aeac..1d9bd35a4 100644 --- a/Code/Geometry/point.h +++ b/Code/Geometry/point.h @@ -41,6 +41,12 @@ class RDKIT_RDGEOMETRYLIB_EXPORT Point { virtual Point *copy() const = 0; }; +// g++ (at least as of v9.3.0) generates some spurious warnings from here. +// disable them +#if !defined(__clang__) and defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif // typedef class Point3D Point; class RDKIT_RDGEOMETRYLIB_EXPORT Point3D : public Point { @@ -501,6 +507,9 @@ class RDKIT_RDGEOMETRYLIB_EXPORT PointND : public Point { return dp_storage.get(); } }; +#if !defined(__clang__) and defined(__GNUC__) +#pragma GCC diagnostic pop +#endif typedef std::vector PointPtrVect; typedef PointPtrVect::iterator PointPtrVect_I; diff --git a/Code/GraphMol/ChemTransforms/MolFragmenter.cpp b/Code/GraphMol/ChemTransforms/MolFragmenter.cpp index 4dcc89125..8c470e517 100644 --- a/Code/GraphMol/ChemTransforms/MolFragmenter.cpp +++ b/Code/GraphMol/ChemTransforms/MolFragmenter.cpp @@ -807,7 +807,7 @@ struct ZipBond { std::vector atoms; bool has_dummy = false; for (auto idx : bond->getStereoAtoms()) { - if (idx == dummy_atom->getIdx()) { + if (static_cast(idx) == dummy_atom->getIdx()) { atoms.push_back(new_atom); has_dummy = true; } else { diff --git a/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp b/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp index 904a2fdca..d9d87179e 100644 --- a/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp +++ b/Code/GraphMol/DistGeomHelpers/testDgeomHelpers.cpp @@ -2319,6 +2319,7 @@ void testGithub3019() { namespace { void throwerror(unsigned int notUsedHere) { + RDUNUSED_PARAM(notUsedHere); throw ValueErrorException("embedder is abortable"); } } // namespace diff --git a/Code/GraphMol/FileParsers/MolSGroupWriting.h b/Code/GraphMol/FileParsers/MolSGroupWriting.h index f9f5a7cef..602f97924 100644 --- a/Code/GraphMol/FileParsers/MolSGroupWriting.h +++ b/Code/GraphMol/FileParsers/MolSGroupWriting.h @@ -1,5 +1,5 @@ // -// Copyright (C) 2002-2018 Greg Landrum and T5 Informatics GmbH +// Copyright (C) 2002-2021 Greg Landrum and T5 Informatics GmbH // // @@ All Rights Reserved @@ // This file is part of the RDKit. @@ -11,6 +11,7 @@ #pragma once #include +#include #include namespace RDKit { @@ -20,21 +21,18 @@ typedef std::unordered_map IDX_TO_SGROUP_MAP; /* ------------------ Inlined Formatters ------------------ */ inline std::string FormatV2000IntField(int value) { - char output[5]; - snprintf(output, 5, " %3d", value); - return std::string(output); + auto fmt = boost::format(" %3d") % value; + return fmt.str(); } inline std::string FormatV2000NumEntriesField(int value) { - char output[4]; - snprintf(output, 4, " %2d", value); - return std::string(output); + auto fmt = boost::format(" %2d") % value; + return fmt.str(); } inline std::string FormatV2000DoubleField(double value) { - char output[11]; - snprintf(output, 11, "%10.4f", value); - return std::string(output); + auto fmt = boost::format("%10.4f") % value; + return fmt.str(); } inline std::string FormatV2000StringField(const std::string &value, diff --git a/Code/GraphMol/FileParsers/testMolSupplier.cpp b/Code/GraphMol/FileParsers/testMolSupplier.cpp index 34240838c..62cf0935c 100644 --- a/Code/GraphMol/FileParsers/testMolSupplier.cpp +++ b/Code/GraphMol/FileParsers/testMolSupplier.cpp @@ -2829,6 +2829,7 @@ void testGitHub3517() { SDMolSupplier sdsup(fname); TEST_ASSERT(!sdsup.atEnd()); size_t l = sdsup.length(); + TEST_ASSERT(l > 0); TEST_ASSERT(!sdsup.atEnd()); } diff --git a/Code/GraphMol/Fingerprints/MorganFingerprints.cpp b/Code/GraphMol/Fingerprints/MorganFingerprints.cpp index dffe3da79..caa9c214a 100644 --- a/Code/GraphMol/Fingerprints/MorganFingerprints.cpp +++ b/Code/GraphMol/Fingerprints/MorganFingerprints.cpp @@ -39,9 +39,12 @@ #include #include +#include #include #include #include +#include + #include #include diff --git a/Code/GraphMol/Fingerprints/MorganGenerator.cpp b/Code/GraphMol/Fingerprints/MorganGenerator.cpp index 45fc320d6..669c531d8 100644 --- a/Code/GraphMol/Fingerprints/MorganGenerator.cpp +++ b/Code/GraphMol/Fingerprints/MorganGenerator.cpp @@ -15,9 +15,11 @@ #include #include +#include #include #include #include +#include #include diff --git a/Code/GraphMol/Fingerprints/PatternFingerprints.cpp b/Code/GraphMol/Fingerprints/PatternFingerprints.cpp index 4b4a720ff..08aeebedc 100644 --- a/Code/GraphMol/Fingerprints/PatternFingerprints.cpp +++ b/Code/GraphMol/Fingerprints/PatternFingerprints.cpp @@ -384,7 +384,7 @@ ExplicitBitVect *PatternFingerprintMol(const MolBundle &bundle, PRECONDITION(!setOnlyBits || setOnlyBits->getNumBits() == fpSize, "bad setOnlyBits size"); ExplicitBitVect *res = nullptr; - for (const auto molp : bundle.getMols()) { + for (const auto &molp : bundle.getMols()) { ExplicitBitVect molfp(fpSize); updatePatternFingerprint(*molp, molfp, fpSize, nullptr, setOnlyBits, tautomericFingerprint); diff --git a/Code/GraphMol/MolDraw2D/MolDraw2D.cpp b/Code/GraphMol/MolDraw2D/MolDraw2D.cpp index 879904186..755529e91 100644 --- a/Code/GraphMol/MolDraw2D/MolDraw2D.cpp +++ b/Code/GraphMol/MolDraw2D/MolDraw2D.cpp @@ -1913,6 +1913,7 @@ void MolDraw2D::calcLabelEllipse(int atom_idx, // **************************************************************************** StringRect MolDraw2D::calcAnnotationPosition(const ROMol &mol, const std::string ¬e) { + RDUNUSED_PARAM(mol); StringRect note_rect; if (note.empty()) { note_rect.width_ = -1.0; // so we know it's not valid. @@ -2477,7 +2478,7 @@ void MolDraw2D::extractLinkNodes(const ROMol &mol) { const double lengthFrac = 0.333; Point2D labelPt{-1000, -1000}; Point2D labelPerp{0, 0}; - for (const auto bAts : node.bondAtoms) { + for (const auto &bAts : node.bondAtoms) { // unlike brackets, we know how these point Point2D startLoc = at_cds_[activeMolIdx_][bAts.first]; Point2D endLoc = at_cds_[activeMolIdx_][bAts.second]; @@ -3190,6 +3191,9 @@ void drawQueryBond(MolDraw2D &d2d, const Bond &bond, bool highlight_bond, const Point2D &at1_cds, const Point2D &at2_cds, const std::vector &at_cds, const DrawColour &col1, const DrawColour &col2, double double_bond_offset) { + RDUNUSED_PARAM(col1); + RDUNUSED_PARAM(col2); + PRECONDITION(bond.hasQuery(), "no query"); const auto qry = bond.getQuery(); if (!d2d.drawOptions().splitBonds) { @@ -3343,13 +3347,12 @@ void MolDraw2D::drawBond( PRECONDITION(activeMolIdx_ >= 0, "bad mol idx"); RDUNUSED_PARAM(highlight_atoms); RDUNUSED_PARAM(highlight_atom_map); + RDUNUSED_PARAM(mol); - if (at1_idx != bond->getBeginAtomIdx()) { + if (static_cast(at1_idx) != bond->getBeginAtomIdx()) { std::swap(at1_idx, at2_idx); } - const Atom *at1 = mol.getAtomWithIdx(at1_idx); - const Atom *at2 = mol.getAtomWithIdx(at2_idx); Point2D at1_cds = at_cds_[activeMolIdx_][at1_idx]; Point2D at2_cds = at_cds_[activeMolIdx_][at2_idx]; @@ -3395,19 +3398,13 @@ void MolDraw2D::drawBond( } } - Bond::BondType bt = bond->getBondType(); bool isComplex = false; if (bond->hasQuery()) { std::string descr = bond->getQuery()->getDescription(); if (bond->getQuery()->getNegation() || descr != "BondOrder") { isComplex = true; - } - if (isComplex) { drawQueryBond(*this, *bond, highlight_bond, at1_cds, at2_cds, at_cds_[activeMolIdx_], col1, col2, double_bond_offset); - } else { - bt = static_cast( - static_cast(bond->getQuery())->getVal()); } } diff --git a/Code/GraphMol/MolInterchange/Parser.cpp b/Code/GraphMol/MolInterchange/Parser.cpp index b488b79d6..eb8ea1462 100644 --- a/Code/GraphMol/MolInterchange/Parser.cpp +++ b/Code/GraphMol/MolInterchange/Parser.cpp @@ -23,11 +23,20 @@ #include #include +// g++ (at least as of v9.3.0) generates some spurious warnings from here. +// disable them +#if !defined(__clang__) and defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wclass-memaccess" +#endif #include #include #include #include #include +#if !defined(__clang__) and defined(__GNUC__) +#pragma GCC diagnostic pop +#endif namespace rj = rapidjson; diff --git a/Code/GraphMol/QueryOps.cpp b/Code/GraphMol/QueryOps.cpp index 36b8756c8..9214d20e2 100644 --- a/Code/GraphMol/QueryOps.cpp +++ b/Code/GraphMol/QueryOps.cpp @@ -695,7 +695,7 @@ bool isAtomListQuery(const Atom *a) { return false; } if (a->getQuery()->getDescription() == "AtomOr") { - for (const auto child : boost::make_iterator_range( + for (const auto &child : boost::make_iterator_range( a->getQuery()->beginChildren(), a->getQuery()->endChildren())) { if (!_atomListQueryHelper(child)) { return false; @@ -713,7 +713,7 @@ void getAtomListQueryVals(const Atom::QUERYATOM_QUERY *q, auto descr = q->getDescription(); PRECONDITION(descr == "AtomOr", "bad query"); if (descr == "AtomOr") { - for (const auto child : + for (const auto &child : boost::make_iterator_range(q->beginChildren(), q->endChildren())) { auto descr = child->getDescription(); if (child->getNegation() || diff --git a/Code/GraphMol/RGroupDecomposition/RGroupGa.h b/Code/GraphMol/RGroupDecomposition/RGroupGa.h index 0f70f6f57..5e70cb3f3 100755 --- a/Code/GraphMol/RGroupDecomposition/RGroupGa.h +++ b/Code/GraphMol/RGroupDecomposition/RGroupGa.h @@ -91,9 +91,11 @@ struct GaResult { vector> permutations; GaResult(const double score, const vector>& permutations) - : score(score), permutations(permutations) {} + : score(score), permutations(permutations){}; + GaResult(const GaResult& other) + : score(other.score), permutations(other.permutations){}; - GaResult() {} + GaResult(){}; // Copy constructor required by MSVC for future GaResult& operator=(const GaResult& other); diff --git a/Code/GraphMol/RGroupDecomposition/RGroupUtils.cpp b/Code/GraphMol/RGroupDecomposition/RGroupUtils.cpp index 239e68667..e4664aa22 100644 --- a/Code/GraphMol/RGroupDecomposition/RGroupUtils.cpp +++ b/Code/GraphMol/RGroupDecomposition/RGroupUtils.cpp @@ -146,7 +146,7 @@ std::string toJSON(const RGroupRow &rgr, const std::string &prefix) { std::string toJSON(const RGroupRows &rows, const std::string &prefix) { std::string res = prefix + "[\n"; auto rowPrefix = prefix + " "; - for (const auto row : rows) { + for (const auto &row : rows) { res += toJSON(row, rowPrefix) + ",\n"; } res.erase(res.end() - 2, res.end()); @@ -168,7 +168,7 @@ std::string toJSON(const RGroupColumn &rgr, const std::string &prefix) { std::string toJSON(const RGroupColumns &cols, const std::string &prefix) { std::string res = prefix + "[\n"; auto colPrefix = prefix + " "; - for (const auto col : cols) { + for (const auto &col : cols) { auto fmt = boost::format{" \"%1%\": %2%"} % (col.first) % (toJSON(col.second, colPrefix)); res += prefix + fmt.str() + ",\n"; diff --git a/Code/GraphMol/RGroupDecomposition/testRGroupDecomp.cpp b/Code/GraphMol/RGroupDecomposition/testRGroupDecomp.cpp index 831a2e4ca..33f6c51f2 100644 --- a/Code/GraphMol/RGroupDecomposition/testRGroupDecomp.cpp +++ b/Code/GraphMol/RGroupDecomposition/testRGroupDecomp.cpp @@ -1999,7 +1999,7 @@ void testNoAlignmentAndSymmetry() { size_t i = 0; for (const auto &smi : smilesData) { ROMOL_SPTR mol(static_cast(SmilesToMol(smi))); - TEST_ASSERT(decomp.add(*mol) == i++); + TEST_ASSERT(decomp.add(*mol) == static_cast(i++)); } TEST_ASSERT(decomp.process()); auto rows = decomp.getRGroupsAsRows(); diff --git a/Code/GraphMol/Subgraphs/SubgraphUtils.h b/Code/GraphMol/Subgraphs/SubgraphUtils.h index bd5129524..eee3bd6f1 100644 --- a/Code/GraphMol/Subgraphs/SubgraphUtils.h +++ b/Code/GraphMol/Subgraphs/SubgraphUtils.h @@ -12,7 +12,10 @@ #define _RD_SUBGRAPHUTILS_H_ #include "Subgraphs.h" +#include #include +#include + #include namespace RDKit { diff --git a/Code/GraphMol/SubstructLibrary/SubstructLibrarySerialization.h b/Code/GraphMol/SubstructLibrary/SubstructLibrarySerialization.h index 35b0b571d..3a39ae971 100644 --- a/Code/GraphMol/SubstructLibrary/SubstructLibrarySerialization.h +++ b/Code/GraphMol/SubstructLibrary/SubstructLibrarySerialization.h @@ -151,9 +151,9 @@ void serialize(Archive &ar, RDKit::PatternHolder &pattern_holder, pattern_holder.getNumBits() != RDKit::PatternHolder::defaultNumBits()) { ar &pattern_holder.getNumBits(); } else if (Archive::is_loading::value) { - try { + try { ar &pattern_holder.getNumBits(); - } catch (boost::archive::archive_exception) { + } catch (boost::archive::archive_exception &) { pattern_holder.getNumBits() = RDKit::PatternHolder::defaultNumBits(); } } diff --git a/Code/RDBoost/python_streambuf.h b/Code/RDBoost/python_streambuf.h index 8ba031a09..fdc5cb819 100644 --- a/Code/RDBoost/python_streambuf.h +++ b/Code/RDBoost/python_streambuf.h @@ -282,19 +282,19 @@ class streambuf : public std::basic_streambuf { off_type n_written = (off_type)(farthest_pptr - pbase()); off_type orig_n_written = n_written; const unsigned int STD_ASCII = 0x7F; - if (df_isTextMode && c > STD_ASCII) { + if (df_isTextMode && static_cast(c) > STD_ASCII) { // we're somewhere in the middle of a utf8 block. If we // only write part of it we'll end up with an exception, // so push everything that could be utf8 into the next block - while (n_written > 0 && - static_cast(write_buffer[n_written - 1]) > STD_ASCII) { + while (n_written > 0 && static_cast( + write_buffer[n_written - 1]) > STD_ASCII) { --n_written; } } bp::str chunk(pbase(), pbase() + n_written); py_write(chunk); - if ((!df_isTextMode || c <= STD_ASCII) && + if ((!df_isTextMode || static_cast(c) <= STD_ASCII) && !traits_type::eq_int_type(c, traits_type::eof())) { py_write(traits_type::to_char_type(c)); n_written++; @@ -305,7 +305,7 @@ class streambuf : public std::basic_streambuf { farthest_pptr = pptr(); if (n_written) { pos_of_write_buffer_end_in_py_file += n_written; - if (df_isTextMode && c > STD_ASCII && + if (df_isTextMode && static_cast(c) > STD_ASCII && !traits_type::eq_int_type(c, traits_type::eof())) { size_t n_to_copy = orig_n_written - n_written; diff --git a/Code/RDGeneral/BoostStartInclude.h b/Code/RDGeneral/BoostStartInclude.h index 44b869d46..a24947343 100644 --- a/Code/RDGeneral/BoostStartInclude.h +++ b/Code/RDGeneral/BoostStartInclude.h @@ -67,6 +67,9 @@ #if (__GNUC__ > 4 || __GNUC_MINOR__ > 7) #pragma GCC diagnostic ignored "-Wunused-local-typedefs" #endif +#if (__GNUC__ > 8) +#pragma GCC diagnostic ignored "-Wdeprecated-copy" +#endif #elif defined(__HP_cc) || defined(__HP_aCC) /* Hewlett-Packard C/aC++. ---------------------------------- */ diff --git a/External/GA/ga/BinaryStringChromosomePolicy.cpp b/External/GA/ga/BinaryStringChromosomePolicy.cpp index 6a4074027..473668fb9 100755 --- a/External/GA/ga/BinaryStringChromosomePolicy.cpp +++ b/External/GA/ga/BinaryStringChromosomePolicy.cpp @@ -12,18 +12,20 @@ namespace GapeGa { -BinaryStringChromosomePolicy::BinaryStringChromosomePolicy(GarethUtil::RandomUtil & rng_) : rng(rng_){ -} +BinaryStringChromosomePolicy::BinaryStringChromosomePolicy( + GarethUtil::RandomUtil& rng_) + : rng(rng_) {} -BinaryStringChromosomePolicy::~BinaryStringChromosomePolicy() { -} +BinaryStringChromosomePolicy::~BinaryStringChromosomePolicy() {} bool BinaryStringChromosomePolicy::mutate(int pos, bool currentValue) const { - return ! currentValue; -} - -bool BinaryStringChromosomePolicy::initialize(int pos) const { - return rng.randomBoolean(); + (void)pos; // not used + return !currentValue; } +bool BinaryStringChromosomePolicy::initialize(int pos) const { + (void)pos; // not used + return rng.randomBoolean(); } + +} // namespace GapeGa diff --git a/External/GA/ga/LinkedPopLinearSel.h b/External/GA/ga/LinkedPopLinearSel.h index 1a03a60d0..6c938bc75 100644 --- a/External/GA/ga/LinkedPopLinearSel.h +++ b/External/GA/ga/LinkedPopLinearSel.h @@ -141,6 +141,8 @@ LinkedPopLinearSel::LinkedPopLinearSel( assert(abs(totalScaledFitness - predictTotalScaledFitness) < 1.0e-5); double predictEndFitness = SELECT_START + (popsize - 1.0) * scaledFitnessStep; + (void)predictEndFitness; // suppress warnings when building with + // assertions disabled #ifdef INCLUDE_REPORTER REPORT(Reporter::TRACE) << "endFitness " << endFitness << " predictEndFitness " << predictEndFitness; diff --git a/External/INCHI-API/inchi.cpp b/External/INCHI-API/inchi.cpp index 1ffeb79cf..bd9291b61 100644 --- a/External/INCHI-API/inchi.cpp +++ b/External/INCHI-API/inchi.cpp @@ -93,11 +93,11 @@ bool assignBondDirs(RWMol& mol, INT_PAIR_VECT& zBondPairs, INT_PAIR_VECT& eBondPairs) { // bonds to assign std::set pending; - for (const auto pair : zBondPairs) { + for (const auto& pair : zBondPairs) { pending.insert(pair.first); pending.insert(pair.second); } - for (const auto pair : eBondPairs) { + for (const auto& pair : eBondPairs) { pending.insert(pair.first); pending.insert(pair.second); } @@ -138,7 +138,7 @@ bool assignBondDirs(RWMol& mol, INT_PAIR_VECT& zBondPairs, for (int _ = 0; _ < 2; _++) { INT_PAIR_VECT* _rules = _ == 0 ? &zBondPairs : &eBondPairs; Bond::BondDir _dir = _ == 0 ? dir : otherDir; - for (const auto pair : *_rules) { + for (const auto& pair : *_rules) { int other = -1; if (pair.first == curBondIdx) { other = pair.second; @@ -1872,7 +1872,7 @@ std::string MolToInchi(const ROMol& mol, ExtraInchiReturnValues& rv, // std::sort(neighbors.begin(), neighbors.end()); unsigned char nid = 0; // std::cerr<<" at: "<getIdx(); - for (const auto p : neighbors) { + for (const auto& p : neighbors) { stereo0D.neighbor[nid++] = p.second; // std::cerr<<" "<