From 2bd6481280d8a4607fa9098d37fce941b2043dd8 Mon Sep 17 00:00:00 2001 From: Ric Date: Fri, 11 Aug 2023 00:04:55 -0400 Subject: [PATCH] Fix some build warnings (#6618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fixes a copy constructor issue: In copy constructor ‘boost::shared_ptr::shared_ptr(const boost::shared_ptr&) [with T = RDKit::ROMol]’, inlined from ‘PyObject* RDKit::RunReactant(ChemicalReaction*, T, unsigned int) [with T = boost::python::api::object]’ at /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp:129:14: /usr/include/boost/smart_ptr/shared_ptr.hpp:416:66: warning: ‘*(const boost::shared_ptr*)((char*)& + offsetof(boost::python::extract >,boost::python::extract >::.boost::python::converter::extract_rvalue >::m_data.boost::python::converter::rvalue_from_python_data >::.boost::python::converter::rvalue_from_python_storage >::storage)).boost::shared_ptr::px’ may be used uninitialized [-Wmaybe-uninitialized] 416 | shared_ptr( shared_ptr const & r ) BOOST_SP_NOEXCEPT : px( r.px ), pn( r.pn ) | ~~^~ /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp: In function ‘PyObject* RDKit::RunReactant(ChemicalReaction*, T, unsigned int) [with T = boost::python::api::object]’: /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp:129:30: note: ‘’ declared here 129 | ROMOL_SPTR react = python::extract(reactant); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/boost/smart_ptr/shared_ptr.hpp:17: In copy constructor ‘boost::detail::shared_count::shared_count(const boost::detail::shared_count&)’, inlined from ‘boost::shared_ptr::shared_ptr(const boost::shared_ptr&) [with T = RDKit::ROMol]’ at /usr/include/boost/smart_ptr/shared_ptr.hpp:416:72, inlined from ‘PyObject* RDKit::RunReactant(ChemicalReaction*, T, unsigned int) [with T = boost::python::api::object]’ at /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp:129:14: /usr/include/boost/smart_ptr/detail/shared_count.hpp:438:67: warning: ‘((const boost::detail::shared_count*)((char*)& + offsetof(boost::python::extract >,boost::python::extract >::.boost::python::converter::extract_rvalue >::m_data.boost::python::converter::rvalue_from_python_data >::.boost::python::converter::rvalue_from_python_storage >::storage)))[1].boost::detail::shared_count::pi_’ may be used uninitialized [-Wmaybe-uninitialized] 438 | shared_count(shared_count const & r) BOOST_SP_NOEXCEPT: pi_(r.pi_) | ~~^~~ /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp: In function ‘PyObject* RDKit::RunReactant(ChemicalReaction*, T, unsigned int) [with T = boost::python::api::object]’: /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp:129:30: note: ‘’ declared here 129 | ROMOL_SPTR react = python::extract(reactant); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Fixes a (weird) allocation warning in GCC 12 In file included from /usr/include/c++/12/bits/alloc_traits.h:33, from /usr/include/c++/12/ext/alloc_traits.h:34, from /usr/include/c++/12/bits/basic_string.h:39, from /usr/include/c++/12/string:53, from /usr/include/c++/12/bits/locale_classes.h:40, from /usr/include/c++/12/bits/ios_base.h:41, from /usr/include/c++/12/streambuf:41, from /usr/include/c++/12/bits/streambuf_iterator.h:35, from /usr/include/c++/12/iterator:66, from /usr/include/boost/iterator/iterator_traits.hpp:10, from /usr/include/boost/range/iterator_range_core.hpp:26, from /usr/include/boost/range/iterator_range.hpp:13, from /usr/include/boost/iostreams/traits.hpp:38, from /usr/include/boost/iostreams/detail/call_traits.hpp:15, from /usr/include/boost/iostreams/detail/adapter/device_adapter.hpp:22, from /usr/include/boost/iostreams/tee.hpp:18, from /tmp/rdkit_builder/rdkit/Code/RDGeneral/RDLog.h:17, from /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemTransforms/testChemTransforms.cpp:11: In function ‘void std::_Construct(_Tp*, _Args&& ...) [with _Tp = pair; _Args = {const pair&}]’, inlined from ‘_ForwardIterator std::__do_uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = const pair*; _ForwardIterator = pair*]’ at /usr/include/c++/12/bits/stl_uninitialized.h:120:21, inlined from ‘static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = const std::pair*; _ForwardIterator = std::pair*; bool _TrivialValueTypes = false]’ at /usr/include/c++/12/bits/stl_uninitialized.h:137:32, inlined from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = const pair*; _ForwardIterator = pair*]’ at /usr/include/c++/12/bits/stl_uninitialized.h:185:15, inlined from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = const pair*; _ForwardIterator = pair*; _Tp = pair]’ at /usr/include/c++/12/bits/stl_uninitialized.h:372:37, inlined from ‘void std::vector<_Tp, _Alloc>::_M_range_insert(iterator, _ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = const std::pair*; _Tp = std::pair; _Alloc = std::allocator >]’ at /usr/include/c++/12/bits/vector.tcc:808:38, inlined from ‘std::vector<_Tp, _Alloc>::iterator std::vector<_Tp, _Alloc>::insert(const_iterator, std::initializer_list<_Tp>) [with _Tp = std::pair; _Alloc = std::allocator >]’ at /usr/include/c++/12/bits/stl_vector.h:1409:17, inlined from ‘void testReplaceCoreMatchVectMultipleMappingToCore()’ at /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemTransforms/testChemTransforms.cpp:974:17: /usr/include/c++/12/bits/stl_construct.h:119:7: warning: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ writing 56 bytes into a region of size 48 overflows the destination [-Wstringop-overflow=] 119 | ::new((void*)__p) _Tp(std::forward<_Args>(__args)...); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/x86_64-linux-gnu/c++/12/bits/c++allocator.h:33, from /usr/include/c++/12/bits/allocator.h:46, from /usr/include/c++/12/string:41: In member function ‘_Tp* std::__new_allocator<_Tp>::allocate(size_type, const void*) [with _Tp = std::pair]’, inlined from ‘static _Tp* std::allocator_traits >::allocate(allocator_type&, size_type) [with _Tp = std::pair]’ at /usr/include/c++/12/bits/alloc_traits.h:464:28, inlined from ‘std::_Vector_base<_Tp, _Alloc>::pointer std::_Vector_base<_Tp, _Alloc>::_M_allocate(std::size_t) [with _Tp = std::pair; _Alloc = std::allocator >]’ at /usr/include/c++/12/bits/stl_vector.h:378:33, inlined from ‘void std::vector<_Tp, _Alloc>::_M_range_insert(iterator, _ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = const std::pair*; _Tp = std::pair; _Alloc = std::allocator >]’ at /usr/include/c++/12/bits/vector.tcc:799:40, inlined from ‘std::vector<_Tp, _Alloc>::iterator std::vector<_Tp, _Alloc>::insert(const_iterator, std::initializer_list<_Tp>) [with _Tp = std::pair; _Alloc = std::allocator >]’ at /usr/include/c++/12/bits/stl_vector.h:1409:17, inlined from ‘void testReplaceCoreMatchVectMultipleMappingToCore()’ at /tmp/rdkit_builder/rdkit/Code/GraphMol/ChemTransforms/testChemTransforms.cpp:974:17: /usr/include/c++/12/bits/new_allocator.h:137:55: note: at offset [8, 56] into destination object of size 56 allocated by ‘operator new’ 137 | return static_cast<_Tp*>(_GLIBCXX_OPERATOR_NEW(__n * sizeof(_Tp))); | ^ * finalForce maybe uninitialized in inlined copy constructor * failure is being used uninitialized * MaeWriter::write overrides a method of the base class without being marked 'override' This one is annoying because it happens in an exported header, so it propagates to any code using the header! * otq is never used * fix implicitly declared assignment operator warning * variables in catch statements that are never used * fix type (sign) mismatch warning * drop duplicate export macro --- Code/ForceField/ForceField.cpp | 2 +- Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp | 9 +++------ Code/GraphMol/ChemTransforms/testChemTransforms.cpp | 10 ++++------ Code/GraphMol/Descriptors/PMI.h | 6 +++--- Code/GraphMol/FileParsers/MolWriters.h | 2 +- Code/GraphMol/GeneralizedSubstruct/catch_tests.cpp | 2 -- Code/GraphMol/MolBundle.h | 2 ++ Code/GraphMol/MolStandardize/Tautomer.cpp | 2 +- Code/GraphMol/Wrap/MolOps.cpp | 2 +- Code/RDBoost/python_streambuf.h | 3 +-- Contrib/ConformerParser/test.cpp | 4 ++-- 11 files changed, 19 insertions(+), 25 deletions(-) diff --git a/Code/ForceField/ForceField.cpp b/Code/ForceField/ForceField.cpp index 0133c5f85..945117fa6 100644 --- a/Code/ForceField/ForceField.cpp +++ b/Code/ForceField/ForceField.cpp @@ -278,7 +278,7 @@ int ForceField::minimize(unsigned int snapshotFreq, unsigned int numIters = 0; unsigned int dim = this->d_numPoints * d_dimension; - double finalForce; + double finalForce = 0.0; auto *points = new double[dim]; this->scatter(points); diff --git a/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp b/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp index 7e172c4d7..665f4f634 100644 --- a/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp +++ b/Code/GraphMol/ChemReactions/Wrap/rdChemReactions.cpp @@ -123,10 +123,9 @@ PyObject *RunReactants(ChemicalReaction *self, T reactants, return res; } -template -PyObject *RunReactant(ChemicalReaction *self, T reactant, +PyObject *RunReactant(ChemicalReaction *self, python::object reactant, unsigned int reactionIdx) { - ROMOL_SPTR react = python::extract(reactant); + auto react = python::extract(reactant); std::vector mols; @@ -562,9 +561,7 @@ Sample Usage: "the products as a tuple of tuples. If maxProducts is not zero," " stop the reaction when maxProducts have been generated " "[default=1000]") - .def("RunReactant", - (PyObject * (*)(RDKit::ChemicalReaction *, python::object, unsigned)) - RDKit::RunReactant, + .def("RunReactant", RDKit::RunReactant, "apply the reaction to a single reactant") .def("RunReactantInPlace", RDKit::RunReactantInPlace, (python::arg("self"), python::arg("reactant"), diff --git a/Code/GraphMol/ChemTransforms/testChemTransforms.cpp b/Code/GraphMol/ChemTransforms/testChemTransforms.cpp index 7cb8d8a78..d1da89d93 100644 --- a/Code/GraphMol/ChemTransforms/testChemTransforms.cpp +++ b/Code/GraphMol/ChemTransforms/testChemTransforms.cpp @@ -970,9 +970,7 @@ void testReplaceCoreMatchVectMultipleMappingToCore() { // 3 dummies in core map to single N in molecule core = "C1([*:1])C([*:2])C([*:3])C1"_smiles; mol = "C1C2C3N2C13"_smiles; - mapping.clear(); - mapping.insert(mapping.end(), - {{0, 4}, {1, 3}, {2, 2}, {3, 3}, {4, 1}, {5, 3}, {6, 0}}); + mapping.assign({{0, 4}, {1, 3}, {2, 2}, {3, 3}, {4, 1}, {5, 3}, {6, 0}}); result.reset(replaceCore(*mol.get(), *core.get(), mapping, false)); smi = MolToSmiles(*result.get(), true); TEST_ASSERT("[1*]N([2*])[3*]" == smi); @@ -1072,13 +1070,13 @@ void testMurckoDecomp() { {"OC2C(C)C21C(N)C1C", "C2CC12CC1"}, // Spiro {"C1CC1C(=O)OC", "C1CC1"}, // Carbonyl outside scaffold {"C1CC1C=C", "C1CC1"}, // Double bond outside scaffold - {"C1CC1C=CC1CC1C=CNNCO", "C1CC1C=CC1CC1"}, // Double bond in scaffold + {"C1CC1C=CC1CC1C=CNNCO", "C1CC1C=CC1CC1"}, // Double bond in scaffold {"CC1CC1C(N)C1C(N)C1", "C1CC1CC1CC1"}, {"C1CC1S(=O)C1CC1C=CNNCO", "C1CC1S(=O)C1CC1"}, // S=O group in scaffold {"O=SCNC1CC1S(=O)C1CC1C=CNNCO", - "C1CC1S(=O)C1CC1"}, // S=O group outside scaffold + "C1CC1S(=O)C1CC1"}, // S=O group outside scaffold {"C1CC1S(=O)(=O)C1CC1C=CNNCO", - "C1CC1S(=O)(=O)C1CC1"}, // SO2 group in scaffold + "C1CC1S(=O)(=O)C1CC1"}, // SO2 group in scaffold {"O=S(CNCNC)(=O)CNC1CC1S(=O)(=O)C1CC1C=CNNCO", "C1CC1S(=O)(=O)C1CC1"}, // SO2 group outside scaffold {"C1CC1C=NO", "C1CC1"}, // Hydroxamide diff --git a/Code/GraphMol/Descriptors/PMI.h b/Code/GraphMol/Descriptors/PMI.h index fa1f796b7..121dea7e3 100644 --- a/Code/GraphMol/Descriptors/PMI.h +++ b/Code/GraphMol/Descriptors/PMI.h @@ -37,9 +37,9 @@ RDKIT_DESCRIPTORS_EXPORT double PMI1(const ROMol&, int confId = -1, bool force = false); const std::string PMI1Version = "1.0.0"; //! second principal moment of inertia -RDKIT_DESCRIPTORS_EXPORT RDKIT_DESCRIPTORS_EXPORT double PMI2( - const ROMol&, int confId = -1, bool useAtomicMasses = true, - bool force = false); +RDKIT_DESCRIPTORS_EXPORT double PMI2(const ROMol&, int confId = -1, + bool useAtomicMasses = true, + bool force = false); const std::string PMI2Version = "1.0.0"; //! Third (largest) principal moment of inertia RDKIT_DESCRIPTORS_EXPORT double PMI3(const ROMol&, int confId = -1, diff --git a/Code/GraphMol/FileParsers/MolWriters.h b/Code/GraphMol/FileParsers/MolWriters.h index f268dcf97..004919c06 100644 --- a/Code/GraphMol/FileParsers/MolWriters.h +++ b/Code/GraphMol/FileParsers/MolWriters.h @@ -395,7 +395,7 @@ class RDKIT_FILEPARSERS_EXPORT MaeWriter : public MolWriter { const STR_VECT &propNames = STR_VECT()); //! \brief write a new molecule to the file. - void write(const ROMol &mol, int confId = defaultConfId); + void write(const ROMol &mol, int confId = defaultConfId) override; //! \brief flush the ostream void flush() override; diff --git a/Code/GraphMol/GeneralizedSubstruct/catch_tests.cpp b/Code/GraphMol/GeneralizedSubstruct/catch_tests.cpp index c8d2120fb..cb3ab029f 100644 --- a/Code/GraphMol/GeneralizedSubstruct/catch_tests.cpp +++ b/Code/GraphMol/GeneralizedSubstruct/catch_tests.cpp @@ -84,8 +84,6 @@ TEST_CASE("tautomer basics") { REQUIRE(mol); ExtendedQueryMol xqm = std::unique_ptr(TautomerQuery::fromMol(*mol)); - const TautomerQuery &otq = - *std::get(xqm.xqmol); SECTION("substructure matching and serialization") { ExtendedQueryMol xqm2(xqm.toBinary()); diff --git a/Code/GraphMol/MolBundle.h b/Code/GraphMol/MolBundle.h index 33b40d394..3d3a67633 100644 --- a/Code/GraphMol/MolBundle.h +++ b/Code/GraphMol/MolBundle.h @@ -65,6 +65,8 @@ class MolBundle : public RDProps { MolBundle(const std::string &pkl) { initFromString(pkl); } virtual ~MolBundle() {} + MolBundle &operator=(const MolBundle &other) = default; + //! returns our molecules virtual const std::vector> &getMols() const { return d_mols; diff --git a/Code/GraphMol/MolStandardize/Tautomer.cpp b/Code/GraphMol/MolStandardize/Tautomer.cpp index 60efed110..c3396d51f 100644 --- a/Code/GraphMol/MolStandardize/Tautomer.cpp +++ b/Code/GraphMol/MolStandardize/Tautomer.cpp @@ -443,7 +443,7 @@ TautomerEnumeratorResult TautomerEnumerator::enumerate(const ROMol &mol) const { MolOps::SANITIZE_SETCONJUGATION | MolOps::SANITIZE_SETHYBRIDIZATION | MolOps::SANITIZE_ADJUSTHS); - } catch (const KekulizeException &ke) { + } catch (const KekulizeException &) { continue; } #ifdef VERBOSE_ENUMERATION diff --git a/Code/GraphMol/Wrap/MolOps.cpp b/Code/GraphMol/Wrap/MolOps.cpp index 00a7c2da0..7fc1e54a6 100644 --- a/Code/GraphMol/Wrap/MolOps.cpp +++ b/Code/GraphMol/Wrap/MolOps.cpp @@ -1009,7 +1009,7 @@ void testSetProps(ROMol &mol) { for (auto &bond : mol.bonds()) { _testSetProps(*bond, std::string("bond_") + std::to_string(bond->getIdx())); } - for (int conf_idx = 0; conf_idx < mol.getNumConformers(); ++conf_idx) { + for (unsigned conf_idx = 0; conf_idx < mol.getNumConformers(); ++conf_idx) { _testSetProps(mol.getConformer(conf_idx), "conf_" + std::to_string(conf_idx)); } diff --git a/Code/RDBoost/python_streambuf.h b/Code/RDBoost/python_streambuf.h index 763c1dfc2..93fe37c0c 100644 --- a/Code/RDBoost/python_streambuf.h +++ b/Code/RDBoost/python_streambuf.h @@ -24,7 +24,6 @@ #include #include -//#include #include #include @@ -459,7 +458,7 @@ class streambuf : public std::basic_streambuf { boost::optional seekoff_without_calling_python( off_type off, std::ios_base::seekdir way, std::ios_base::openmode which) { - boost::optional const failure; + boost::optional const failure = off_type(-1); // Buffer range and current position off_type buf_begin, buf_end, buf_cur, upper_bound; diff --git a/Contrib/ConformerParser/test.cpp b/Contrib/ConformerParser/test.cpp index 516aa068a..1a8cdb55e 100644 --- a/Contrib/ConformerParser/test.cpp +++ b/Contrib/ConformerParser/test.cpp @@ -65,7 +65,7 @@ void test1() { bool ok = false; try { readAmberTrajectory(fName, coords, mol->getNumAtoms()); - } catch (ValueErrorException &e) { + } catch (ValueErrorException &) { ok = true; } TEST_ASSERT(ok); @@ -74,7 +74,7 @@ void test1() { ok = false; try { readAmberTrajectory(fName, coords, mol->getNumAtoms()); - } catch (ValueErrorException &e) { + } catch (ValueErrorException &) { // std::cout << e.what() << std::endl; ok = true; }