diff --git a/CMakeLists.txt b/CMakeLists.txt index 82ad430ec..9a3a36536 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -380,7 +380,7 @@ if(RDK_USE_BOOST_SERIALIZATION) else() set(Boost_LIBRARIES ${T_LIBS}) endif() - target_compile_definitions(rdkit_base INTERFACE -DRDK_USE_BOOST_SERIALIZATION) + target_compile_definitions(rdkit_base INTERFACE -DRDK_USE_BOOST_SERIALIZATION -DBOOST_SERIALIZATION_DYN_LINK) endif() diff --git a/CTestConfig.cmake b/CTestConfig.cmake index c406207ba..f5bc6e953 100644 --- a/CTestConfig.cmake +++ b/CTestConfig.cmake @@ -1,2 +1,2 @@ # For some reason, MEMORYCHECK_SUPPRESSIONS_FILE is not being caught, so I hardcoded it here -SET(MEMORYCHECK_COMMAND_OPTIONS "--tool=memcheck --time-stamp=yes --num-callers=20 --gen-suppressions=all --leak-check=full --show-reachable=no --trace-children=yes --suppressions=${RDKit_SOURCE_DIR}/Code/cmake/rdkit_valgrind.suppressions") +SET(MEMORYCHECK_COMMAND_OPTIONS "--tool=memcheck --error-exitcode=13 --time-stamp=yes --num-callers=20 --gen-suppressions=all --leak-check=full --show-reachable=no --trace-children=yes --suppressions=${RDKit_SOURCE_DIR}/Code/cmake/rdkit_valgrind.suppressions") diff --git a/Code/DataStructs/testDatastructs.cpp b/Code/DataStructs/testDatastructs.cpp index 97ef4db69..7d502833c 100644 --- a/Code/DataStructs/testDatastructs.cpp +++ b/Code/DataStructs/testDatastructs.cpp @@ -1425,6 +1425,7 @@ void test16BitVectProps() { RDValue newValue; TEST_ASSERT(handler->read(ss, newValue)); TEST_ASSERT(from_rdvalue(newValue) == bv); + newValue.destroy(); } delete handlers[1]; } diff --git a/Code/GraphMol/ChemReactions/DaylightParser.cpp b/Code/GraphMol/ChemReactions/DaylightParser.cpp index d795810bf..15c5ad681 100644 --- a/Code/GraphMol/ChemReactions/DaylightParser.cpp +++ b/Code/GraphMol/ChemReactions/DaylightParser.cpp @@ -108,8 +108,8 @@ ChemicalReaction *RxnSmartsToChemicalReaction( std::vector pos; - for (std::size_t i = 0; i < text.length(); i++) { - if (text[i] == '>' && text[i-1] != '-') { + for (std::size_t i = 0; i < text.length(); ++i) { + if (text[i] == '>' && (i == 0 || text[i - 1] != '-')) { pos.push_back(i); } } diff --git a/Code/GraphMol/ChemReactions/Enumerate/testEnumerate.cpp b/Code/GraphMol/ChemReactions/Enumerate/testEnumerate.cpp index bfa3d1904..84aa840fd 100644 --- a/Code/GraphMol/ChemReactions/Enumerate/testEnumerate.cpp +++ b/Code/GraphMol/ChemReactions/Enumerate/testEnumerate.cpp @@ -130,20 +130,24 @@ void testSamplers() { void testEvenSamplers() { EnumerationTypes::BBS bbs; bbs.resize(3); - boost::uint64_t R1 = 6000; - boost::uint64_t R2 = 500; - boost::uint64_t R3 = 10000; + boost::uint64_t R1 = 600; + boost::uint64_t R2 = 50; + boost::uint64_t R3 = 1000; + + boost::shared_ptr m(SmilesToMol("C=CCN=C=S")); + boost::shared_ptr m2(SmilesToMol("NCc1ncc(Cl)cc1Br")); + boost::shared_ptr m3(SmilesToMol("NCCCc1ncc(Cl)cc1Br")); + for (unsigned long i = 0; i < R1; ++i) { - bbs[0].push_back(boost::shared_ptr(SmilesToMol("C=CCN=C=S"))); + bbs[0].push_back(m); } for (unsigned long i = 0; i < R2; ++i) { - bbs[1].push_back(boost::shared_ptr(SmilesToMol("NCc1ncc(Cl)cc1Br"))); + bbs[1].push_back(m2); } for (unsigned long i = 0; i < R3; ++i) { - bbs[2].push_back( - boost::shared_ptr(SmilesToMol("NCCCc1ncc(Cl)cc1Br"))); + bbs[2].push_back(m3); } ChemicalReaction rxn; diff --git a/Code/GraphMol/ChemReactions/testReaction.cpp b/Code/GraphMol/ChemReactions/testReaction.cpp index 4b195e9d5..1907c61d7 100644 --- a/Code/GraphMol/ChemReactions/testReaction.cpp +++ b/Code/GraphMol/ChemReactions/testReaction.cpp @@ -6949,7 +6949,7 @@ bool check_bond_stereo(const ROMOL_SPTR &mol, unsigned bond_idx, ROMOL_SPTR run_simple_reaction(const std::string &reaction, const ROMOL_SPTR &reactant) { - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 1); @@ -7135,7 +7135,8 @@ void testOtherBondStereo() { { // Reaction changes order of the stereo bond const std::string reaction(R"([C:1]=[C:2]>>[C:1]-[C:2])"); - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{ + RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 1); @@ -7161,7 +7162,8 @@ void testOtherBondStereo() { // (no directed bonds enclosing the double bond) const std::string reaction( R"([C:1]/[C:2]=[C:3]/[Br:4]>>[C:1][C:2]=[C:3]-[Br:4])"); - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{ + RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 1); @@ -7190,7 +7192,8 @@ void testOtherBondStereo() { } { // Reaction with 2 product sets const std::string reaction(R"([C:1]=[C:2]>>[Si:1]=[C:2])"); - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{ + RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 1); @@ -7218,7 +7221,8 @@ void testOtherBondStereo() { TEST_ASSERT(check_bond_stereo(mol2, 2, 0, 4, Bond::BondStereo::STEREOE)); const std::string reaction(R"([C:1]=[C:2][Br:3]>>[C:1]=[C:2].[Br:3])"); - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{ + RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 2); @@ -7245,7 +7249,8 @@ void testOtherBondStereo() { const std::string reaction( R"([Cl:4][C:1]=[C:2][Br:3]>>[C:1]=[C:2].[Br:3].[Cl:4])"); - const auto rxn = RxnSmartsToChemicalReaction(reaction); + std::unique_ptr rxn{ + RxnSmartsToChemicalReaction(reaction)}; TEST_ASSERT(rxn); TEST_ASSERT(rxn->getNumReactantTemplates() == 1); TEST_ASSERT(rxn->getNumProductTemplates() == 3); diff --git a/Code/GraphMol/FileParsers/MaeMolSupplier.cpp b/Code/GraphMol/FileParsers/MaeMolSupplier.cpp index 61ee29be0..1705cc8c1 100644 --- a/Code/GraphMol/FileParsers/MaeMolSupplier.cpp +++ b/Code/GraphMol/FileParsers/MaeMolSupplier.cpp @@ -297,8 +297,6 @@ void addAtoms(const mae::IndexedBlock &atom_block, RWMol &mol) { const auto ys = atom_block.getRealProperty(mae::ATOM_Y_COORD); const auto zs = atom_block.getRealProperty(mae::ATOM_Z_COORD); - std::shared_ptr atomic_charges; - // atomic numbers, and x, y, and z coordinates const auto size = atomic_numbers->size(); auto conf = new RDKit::Conformer(size); @@ -346,6 +344,42 @@ void addBonds(const mae::IndexedBlock &bond_block, RWMol &mol) { mol.addBond(bond, true); } } + +void build_mol(RWMol &mol, mae::Block &structure_block, bool sanitize, + bool removeHs) { + const auto &atom_block = structure_block.getIndexedBlock(mae::ATOM_BLOCK); + addAtoms(*atom_block, mol); + + const auto &bond_block = structure_block.getIndexedBlock(mae::BOND_BLOCK); + addBonds(*bond_block, mol); + + // These properties need to be set last, as stereochemistry is defined here, + // and it requires atoms and bonds to be available. + set_mol_properties(mol, structure_block); + + if (sanitize) { + if (removeHs) { + MolOps::removeHs(mol, false, false); + } else { + MolOps::sanitizeMol(mol); + } + } else { + // we need some properties for the chiral setup + mol.updatePropertyCache(false); + } + + // If there are 3D coordinates, try to read more chiralities from them, but do + // not override the ones that were read from properties + bool replaceExistingTags = false; + if (mol.getNumConformers() && mol.getConformer().is3D()) { + MolOps::assignChiralTypesFrom3D(mol, -1, replaceExistingTags); + } + + // Find more stereo bonds, assign labels, but don't replace the existing ones + MolOps::detectBondStereochemistry(mol, replaceExistingTags); + MolOps::assignStereochemistry(mol, replaceExistingTags); +} + } // namespace MaeMolSupplier::MaeMolSupplier(std::shared_ptr inStream, @@ -415,40 +449,15 @@ ROMol *MaeMolSupplier::next() { throw FileParseException("All structures read from Maestro file"); } - auto *mol = new RWMol(); + auto mol = new RWMol; - const auto atom_block = d_next_struct->getIndexedBlock(mae::ATOM_BLOCK); - addAtoms(*atom_block, *mol); - - const auto bond_block = d_next_struct->getIndexedBlock(mae::BOND_BLOCK); - addBonds(*bond_block, *mol); - - // These properties need to be set last, as stereochemistry is defined here, - // and it requires atoms and bonds to be available. - set_mol_properties(*mol, *d_next_struct); - - if (df_sanitize) { - if (df_removeHs) { - MolOps::removeHs(*mol, false, false); - } else { - MolOps::sanitizeMol(*mol); - } - } else { - // we need some properties for the chiral setup - mol->updatePropertyCache(false); + try { + build_mol(*mol, *d_next_struct, df_sanitize, df_removeHs); + } catch (...) { + delete mol; + throw; } - // If there are 3D coordinates, try to read more chiralities from them, but do - // not override the ones that were read from properties - bool replaceExistingTags = false; - if (mol->getNumConformers() && mol->getConformer().is3D()) { - MolOps::assignChiralTypesFrom3D(*mol, -1, replaceExistingTags); - } - - // Find more stereo bonds, assign labels, but don't replace the existing ones - MolOps::detectBondStereochemistry(*mol, replaceExistingTags); - MolOps::assignStereochemistry(*mol, replaceExistingTags); - try { d_next_struct = d_reader->next(mae::CT_BLOCK); } catch (const mae::read_exception &e) { diff --git a/Code/GraphMol/FileParsers/PDBParser.cpp b/Code/GraphMol/FileParsers/PDBParser.cpp index 8bd01a176..7bbe0a14c 100644 --- a/Code/GraphMol/FileParsers/PDBParser.cpp +++ b/Code/GraphMol/FileParsers/PDBParser.cpp @@ -30,7 +30,12 @@ // flavor & 2 : Read each MODEL into a separate molecule. namespace RDKit { -static Atom *PDBAtomFromSymbol(const char *symb) { +namespace { + +// This is a macro to allow its use for C++ constants +#define BCNAM(A, B, C) (((A) << 16) | ((B) << 8) | (C)) + +Atom *PDBAtomFromSymbol(const char *symb) { PRECONDITION(symb, "bad char ptr"); if (symb[0] == 'D' && !symb[1]) { auto *result = new Atom(1); @@ -45,8 +50,8 @@ static Atom *PDBAtomFromSymbol(const char *symb) { return elemno > 0 ? new Atom(elemno) : (Atom *)nullptr; } -static void PDBAtomLine(RWMol *mol, const char *ptr, unsigned int len, - unsigned int flavor, std::map &amap) { +void PDBAtomLine(RWMol *mol, const char *ptr, unsigned int len, + unsigned int flavor, std::map &amap) { PRECONDITION(mol, "bad mol"); PRECONDITION(ptr, "bad char ptr"); std::string tmp; @@ -304,9 +309,8 @@ static void PDBAtomLine(RWMol *mol, const char *ptr, unsigned int len, info->setTempFactor(bfactor); } -static void PDBBondLine(RWMol *mol, const char *ptr, unsigned int len, - std::map &amap, - std::map &bmap) { +void PDBBondLine(RWMol *mol, const char *ptr, unsigned int len, + std::map &amap, std::map &bmap) { PRECONDITION(mol, "bad mol"); PRECONDITION(ptr, "bad char ptr"); @@ -415,7 +419,7 @@ static void PDBBondLine(RWMol *mol, const char *ptr, unsigned int len, } } -static void PDBTitleLine(RWMol *mol, const char *ptr, unsigned int len) { +void PDBTitleLine(RWMol *mol, const char *ptr, unsigned int len) { PRECONDITION(mol, "bad mol"); PRECONDITION(ptr, "bad char ptr"); std::string title; @@ -435,8 +439,8 @@ static void PDBTitleLine(RWMol *mol, const char *ptr, unsigned int len) { } } -static void PDBConformerLine(RWMol *mol, const char *ptr, unsigned int len, - Conformer *&conf, int &conformer_atmidx) { +void PDBConformerLine(RWMol *mol, const char *ptr, unsigned int len, + Conformer *&conf, int &conformer_atmidx) { PRECONDITION(mol, "bad mol"); PRECONDITION(ptr, "bad char ptr"); @@ -472,13 +476,10 @@ static void PDBConformerLine(RWMol *mol, const char *ptr, unsigned int len, } } -// This is a macro to allow its use for C++ constants -#define BCNAM(A, B, C) (((A) << 16) | ((B) << 8) | (C)) - // This function determines whether a standard atom name in // in a recognized PDB amino acid should be chiral or not. // This is used to avoid chirality on VAL.CG and LEU.CG. -static bool StandardPDBChiralAtom(const char *resnam, const char *atmnam) { +bool StandardPDBChiralAtom(const char *resnam, const char *atmnam) { switch (BCNAM(resnam[0], resnam[1], resnam[2])) { case BCNAM('G', 'L', 'Y'): return false; @@ -510,7 +511,7 @@ static bool StandardPDBChiralAtom(const char *resnam, const char *atmnam) { return false; } -static void StandardPDBResidueChirality(RWMol *mol) { +void StandardPDBResidueChirality(RWMol *mol) { for (ROMol::AtomIterator atomIt = mol->beginAtoms(); atomIt != mol->endAtoms(); ++atomIt) { Atom *atom = *atomIt; @@ -546,12 +547,11 @@ void BasicPDBCleanup(RWMol &mol) { } } -RWMol *PDBBlockToMol(const char *str, bool sanitize, bool removeHs, - unsigned int flavor, bool proximityBonding) { +void parsePdbBlock(RWMol *&mol, const char *str, bool sanitize, bool removeHs, + unsigned int flavor, bool proximityBonding) { PRECONDITION(str, "bad char ptr"); std::map amap; std::map bmap; - RWMol *mol = nullptr; Utils::LocaleSwitcher ls; bool multi_conformer = false; int conformer_atmidx = 0; @@ -639,14 +639,14 @@ RWMol *PDBBlockToMol(const char *str, bool sanitize, bool removeHs, } if (!mol) { - return (RWMol *)nullptr; + return; } if (proximityBonding) { ConnectTheDots(mol, ctdIGNORE_H_H_CONTACTS); } // flavor & 8 doesn't encode double bonds - if (proximityBonding || ((flavor & 8) != 0)) { + if (proximityBonding || flavor & 8) { StandardPDBResidueBondOrders(mol); } @@ -666,6 +666,18 @@ RWMol *PDBBlockToMol(const char *str, bool sanitize, bool removeHs, /* Set tetrahedral chirality from 3D co-ordinates */ MolOps::assignChiralTypesFrom3D(*mol); StandardPDBResidueChirality(mol); +} +} // namespace + +RWMol *PDBBlockToMol(const char *str, bool sanitize, bool removeHs, + unsigned int flavor, bool proximityBonding) { + RWMol *mol = nullptr; + try { + parsePdbBlock(mol, str, sanitize, removeHs, flavor, proximityBonding); + } catch (...) { + delete mol; + throw; + } return mol; } diff --git a/Code/GraphMol/MolDraw2D/test1.cpp b/Code/GraphMol/MolDraw2D/test1.cpp index b8887dbbe..b50bd6a7a 100644 --- a/Code/GraphMol/MolDraw2D/test1.cpp +++ b/Code/GraphMol/MolDraw2D/test1.cpp @@ -1688,6 +1688,7 @@ void test11DrawMolGrid() { } delete m1; delete m2; + delete m3; std::cerr << " Done" << std::endl; } @@ -1746,6 +1747,8 @@ void test12DrawMols() { outs.flush(); } { + delete mols[2]; + delete mols[4]; mols[2] = nullptr; mols[4] = nullptr; MolDraw2DSVG drawer(750, 400, 250, 200); diff --git a/Code/GraphMol/SmilesParse/SmilesParse.cpp b/Code/GraphMol/SmilesParse/SmilesParse.cpp index 53177632f..a16b20fa5 100644 --- a/Code/GraphMol/SmilesParse/SmilesParse.cpp +++ b/Code/GraphMol/SmilesParse/SmilesParse.cpp @@ -368,6 +368,7 @@ RWMol *SmilesToMol(const std::string &smiles, SmilesParseOps::parseCXExtensions(*res, cxPart, pos); } catch (const SmilesParseException &) { if (params.strictCXSMILES) { + delete res; throw; } } diff --git a/Code/GraphMol/SmilesParse/test.cpp b/Code/GraphMol/SmilesParse/test.cpp index 1f8b60c61..f1e4fe56b 100644 --- a/Code/GraphMol/SmilesParse/test.cpp +++ b/Code/GraphMol/SmilesParse/test.cpp @@ -173,6 +173,7 @@ void testFail() { CHECK_INVARIANT(!mol, smi); } else { CHECK_INVARIANT(mol, smi); + delete mol; } i++; } @@ -833,16 +834,19 @@ void testStereochem() { mol = SmilesToMol(smi); smi = MolToSmiles(*mol, 1); TEST_ASSERT(refSmi == smi); + delete mol; smi = "Cl[C@@H](F)/C=C(\\F)"; mol = SmilesToMol(smi); smi = MolToSmiles(*mol, 1); TEST_ASSERT(refSmi == smi); + delete mol; smi = "Cl[C@@H](F)\\C=C(/F)"; mol = SmilesToMol(smi); smi = MolToSmiles(*mol, 1); TEST_ASSERT(refSmi == smi); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -879,6 +883,7 @@ void testIssue127() { // std::cout << refSmi << " : " << tempStr << std::endl; TEST_ASSERT(refSmi == tempStr); delete mol2; + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1327,6 +1332,7 @@ void testIssue159() { TEST_ASSERT(mol->getBondWithIdx(3)->getStereo() == Bond::STEREOE); TEST_ASSERT(mol->getBondWithIdx(5)->getStereo() == Bond::STEREOZ); TEST_ASSERT(mol->getBondWithIdx(8)->getStereo() == Bond::STEREOE); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1343,17 +1349,19 @@ void testIssue175() { mol = SmilesToMol(smi); TEST_ASSERT(mol); TEST_ASSERT(mol->getBondWithIdx(1)->getStereo() == Bond::STEREOE); - delete mol; + smi = "Cl\\C=C1CN/1"; mol = SmilesToMol(smi); TEST_ASSERT(mol); TEST_ASSERT(mol->getBondWithIdx(1)->getStereo() == Bond::STEREOE); + delete mol; smi = "C/1=C/F.F1"; mol = SmilesToMol(smi); TEST_ASSERT(mol); TEST_ASSERT(mol->getBondWithIdx(0)->getStereo() == Bond::STEREOZ); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1417,6 +1425,7 @@ void testIssue180() { TEST_ASSERT(mol->getBondWithIdx(5)->getStereo() == Bond::STEREOZ); smi = MolToSmiles(*mol, 1); TEST_ASSERT(refSmi == smi); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1452,6 +1461,7 @@ void testIssue184() { smi = MolToSmiles(*mol, 1); TEST_ASSERT(refSmi == smi); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1524,6 +1534,7 @@ void testIssue185() { TEST_ASSERT((*bondIt)->getStereo() == Bond::STEREOE); } } + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1562,6 +1573,7 @@ void testIssue191() { smi = MolToSmiles(*mol, 1); // std::cout << "ref: " << refSmi << " -> " << smi << std::endl; TEST_ASSERT(refSmi == smi); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -1766,6 +1778,7 @@ void testBug1670149() { TEST_ASSERT(mol->getAtomWithIdx(1)->getNumImplicitHs() == 2); smi = MolToSmiles(*mol, false, false, -1); TEST_ASSERT(smi == "C1CC[NH2+]C1"); + delete mol; BOOST_LOG(rdInfoLog) << "\tdone" << std::endl; } @@ -2095,6 +2108,7 @@ void testBug1844959() { TEST_ASSERT(label == "S"); smi2 = MolToSmiles(*mol, true); TEST_ASSERT(smi == smi2); + delete mol; // now make sure it works with a reversed chiral tag: smi = "C[C@@]12CNOC2.F1"; @@ -2132,6 +2146,7 @@ void testBug1844959() { TEST_ASSERT(label == "R"); smi2 = MolToSmiles(*mol, true); TEST_ASSERT(smi == smi2); + delete mol; // ^^^^^^^^^^^^^^^^^^^^^^ // end of the set // ---------------------- @@ -2182,6 +2197,7 @@ void testBug1844959() { TEST_ASSERT(label == "R"); smi2 = MolToSmiles(*mol, true); TEST_ASSERT(smi == smi2); + delete mol; // now make sure it works with a reversed chiral tag: smi = "C[C@@]12CNOC2.[H]1"; @@ -3307,6 +3323,7 @@ void testBug253() { std::string csmiles1 = MolToSmiles(*m, true); std::cerr << "--" << csmiles1 << std::endl; TEST_ASSERT(csmiles1 == "C1CCC2(CC1)CCCCC2CCC1CCCC1"); + delete m; } BOOST_LOG(rdInfoLog) << "done" << std::endl; @@ -3329,6 +3346,7 @@ void testBug257() { m = SmilesToMol(csmiles); TEST_ASSERT(m); TEST_ASSERT(m->getBondWithIdx(1)->getBondType() == Bond::UNSPECIFIED); + delete m; } BOOST_LOG(rdInfoLog) << "done" << std::endl; @@ -3992,11 +4010,13 @@ void testSmilesParseParams() { std::string smiles = "CCCC the_name"; ROMol *m = SmilesToMol(smiles); TEST_ASSERT(m); + delete m; { // it's ignored SmilesParserParams params; m = SmilesToMol(smiles, params); TEST_ASSERT(m); TEST_ASSERT(!m->hasProp(common_properties::_Name)); + delete m; } { SmilesParserParams params; diff --git a/Code/GraphMol/Substruct/test1.cpp b/Code/GraphMol/Substruct/test1.cpp index 61452f87e..e7d89ee0f 100644 --- a/Code/GraphMol/Substruct/test1.cpp +++ b/Code/GraphMol/Substruct/test1.cpp @@ -1724,6 +1724,7 @@ void testGithub2570() { std::vector matches; TEST_ASSERT(SubstructMatch(*mol, *query, matches, uniquify, recursionPossible, useChirality)); + delete query; } { const auto mol = R"([C@H](O)(F)Cl)"_smiles; @@ -1740,6 +1741,7 @@ void testGithub2570() { std::vector matches; TEST_ASSERT(SubstructMatch(*mol, *query, matches, uniquify, recursionPossible, useChirality)); + delete query; } { const auto mol = R"([C@](O)(F)(Cl)C)"_smiles; @@ -1805,6 +1807,7 @@ void testEZVsCisTransMatch() { TEST_ASSERT(check.second == SubstructMatch(*mol, *query, match, recursionPossible, useChirality)); + delete query; } // Symmetrize stereoatoms for (const auto &check : checks) { @@ -1825,6 +1828,7 @@ void testEZVsCisTransMatch() { TEST_ASSERT(check.second == SubstructMatch(*mol, *query, match, recursionPossible, useChirality)); + delete query; } // Flip one stereoatom and the label for (const auto &check : checks) { @@ -1848,6 +1852,7 @@ void testEZVsCisTransMatch() { TEST_ASSERT(check.second == SubstructMatch(*mol, *query, match, recursionPossible, useChirality)); + delete query; } } diff --git a/Code/RDGeneral/testDict.cpp b/Code/RDGeneral/testDict.cpp index 8690b4c1c..4514e4827 100644 --- a/Code/RDGeneral/testDict.cpp +++ b/Code/RDGeneral/testDict.cpp @@ -706,6 +706,7 @@ void testCustomProps() { TEST_ASSERT(handler->read(ss, newValue)); TEST_ASSERT(from_rdvalue(newValue).bar == f.bar); TEST_ASSERT(from_rdvalue(newValue).baz == f.baz); + newValue.destroy(); } delete handlers[1]; } diff --git a/Code/boost/serialization/singleton.hpp b/Code/boost/serialization/singleton.hpp new file mode 100644 index 000000000..439942481 --- /dev/null +++ b/Code/boost/serialization/singleton.hpp @@ -0,0 +1,214 @@ +#ifndef BOOST_SERIALIZATION_SINGLETON_HPP +#define BOOST_SERIALIZATION_SINGLETON_HPP + +/////////1/////////2///////// 3/////////4/////////5/////////6/////////7/////////8 +// singleton.hpp +// +// Copyright David Abrahams 2006. Original version +// +// Copyright Robert Ramey 2007. Changes made to permit +// application throughout the serialization library. +// +// Distributed under the Boost +// Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// +// The intention here is to define a template which will convert +// any class into a singleton with the following features: +// +// a) initialized before first use. +// b) thread-safe for const access to the class +// c) non-locking +// +// In order to do this, +// a) Initialize dynamically when used. +// b) Require that all singletons be initialized before main +// is called or any entry point into the shared library is invoked. +// This guarentees no race condition for initialization. +// In debug mode, we assert that no non-const functions are called +// after main is invoked. +// + +// MS compatible compilers support #pragma once +#if defined(_MSC_VER) +# pragma once +#endif + +#include +#include +#include +#include + +#include +#include +#include // must be the last header + +#ifdef BOOST_MSVC +# pragma warning(push) +# pragma warning(disable : 4511 4512) +#endif + +namespace boost { +namespace serialization { + +////////////////////////////////////////////////////////////////////// +// Provides a dynamically-initialized (singleton) instance of T in a +// way that avoids LNK1179 on vc6. See http://tinyurl.com/ljdp8 or +// http://lists.boost.org/Archives/boost/2006/05/105286.php for +// details. +// + +// singletons created by this code are guarenteed to be unique +// within the executable or shared library which creates them. +// This is sufficient and in fact ideal for the serialization library. +// The singleton is created when the module is loaded and destroyed +// when the module is unloaded. + +// This base class has two functions. + +// First it provides a module handle for each singleton indicating +// the executable or shared library in which it was created. This +// turns out to be necessary and sufficient to implement the tables +// used by serialization library. + +// Second, it provides a mechanism to detect when a non-const function +// is called after initialization. + +// make a singleton to lock/unlock all singletons for alteration. +// The intent is that all singletons created/used by this code +// are to be initialized before main is called. A test program +// can lock all the singletons when main is entereed. This any +// attempt to retieve a mutable instances while locked will +// generate a assertion if compiled for debug. + +// note usage of BOOST_DLLEXPORT. These functions are in danger of +// being eliminated by the optimizer when building an application in +// release mode. Usage of the macro is meant to signal the compiler/linker +// to avoid dropping these functions which seem to be unreferenced. +// This usage is not related to autolinking. + +class BOOST_SYMBOL_VISIBLE singleton_module : + public boost::noncopyable +{ +private: + BOOST_DLLEXPORT static bool & get_lock() BOOST_USED { + static bool lock = false; + return lock; + } + +public: + BOOST_DLLEXPORT static void lock(){ + get_lock() = true; + } + BOOST_DLLEXPORT static void unlock(){ + get_lock() = false; + } + BOOST_DLLEXPORT static bool is_locked(){ + return get_lock(); + } +}; + +template +class singleton : public singleton_module +{ +private: + static void cleanup_func() { + delete static_cast (&get_instance()); + } + + // use a wrapper so that types T with protected constructors + // can be used + class singleton_wrapper : public T { + public: + singleton_wrapper () { + #if !defined(BOOST_ALL_DYN_LINK) && !defined(BOOST_SERIALIZATION_DYN_LINK) + /* Static builds: We're in a single module, use atexit() to + * ensure destruction in reverse of construction. + * (In static builds the compiler-generated order may be wrong...) */ + atexit(&cleanup_func); + #endif + } + }; + + /* This wrapper ensures the instance is cleaned up when the + * module is wound down. (The cleanup of the static variable + * in get_instance() may happen at the wrong time.) */ + struct instance_and_cleanup + { + T& x; + + instance_and_cleanup(T& x) : x(x) { + } + ~instance_and_cleanup() { + #if defined(BOOST_ALL_DYN_LINK) || defined(BOOST_SERIALIZATION_DYN_LINK) + /* Shared builds: The ordering through global variables is + * sufficient. + * However, avoid atexit() as it may cause destruction order + * issues here. */ + singleton::cleanup_func(); + #endif + } + }; + static instance_and_cleanup m_instance_and_cleanup; + // include this to provoke instantiation at pre-execution time + static void use(T const *) {} + static T & get_instance() { + // Use a heap-allocated instance to work around static variable + // destruction order issues: this inner singleton_wrapper<> + // instance may be destructed before the singleton<> instance. + // Using a 'dumb' static variable lets us precisely choose the + // time destructor is invoked. + // The destruction itself is handled by m_instance_cleanup (for + // shared builds) or in an atexit() function (static builds). + static singleton_wrapper* t = new singleton_wrapper; + + // refer to instance, causing it to be instantiated (and + // initialized at startup on working compilers) + BOOST_ASSERT(! is_destroyed()); + + // note that the following is absolutely essential. + // commenting out this statement will cause compilers to fail to + // construct the instance at pre-execution time. This would prevent + // our usage/implementation of "locking" and introduce uncertainty into + // the sequence of object initializaition. + use(& m_instance_and_cleanup.x); + return static_cast(*t); + } + static bool & get_is_destroyed(){ + static bool is_destroyed; + return is_destroyed; + } + +public: + BOOST_DLLEXPORT static T & get_mutable_instance(){ + BOOST_ASSERT(! is_locked()); + return get_instance(); + } + BOOST_DLLEXPORT static const T & get_const_instance(){ + return get_instance(); + } + BOOST_DLLEXPORT static bool is_destroyed(){ + return get_is_destroyed(); + } + BOOST_DLLEXPORT singleton(){ + get_is_destroyed() = false; + } + BOOST_DLLEXPORT ~singleton() { + get_is_destroyed() = true; + } +}; + +template +typename singleton< T >::instance_and_cleanup singleton< T >::m_instance_and_cleanup ( + singleton< T >::get_instance()); + +} // namespace serialization +} // namespace boost + +#include // pops abi_suffix.hpp pragmas + +#ifdef BOOST_MSVC +#pragma warning(pop) +#endif + +#endif // BOOST_SERIALIZATION_SINGLETON_HPP diff --git a/Code/cmake/rdkit_valgrind.suppressions b/Code/cmake/rdkit_valgrind.suppressions index 73783ddcb..46d296137 100644 --- a/Code/cmake/rdkit_valgrind.suppressions +++ b/Code/cmake/rdkit_valgrind.suppressions @@ -3,8 +3,8 @@ # inside a conda environment. Used relevant software was: # # - valgrind: v3.14.0 final, built from git sources under the same env. -# - Boost libs: conda package, v1.67.0 (py36_4) -# - Compiler: conda package, gxx_linux-64 7.2.0 (h550dcbe_27) +# - Boost libs: v1.68.0 (py36) +# - Compiler: gxx_linux-64 6.4.0 # # CMake command/options was: # cmake .. \ @@ -27,61 +27,21 @@ { Avalon #1 Memcheck:Cond - fun:SmilesBranch + ... fun:SmilesBranch fun:MOLToSMIExt fun:CanSmilesStep fun:CanSmiles - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi + fun:_ZN11AvalonTools14getCanonSmiles* } { Avalon #2 Memcheck:Cond - fun:SmilesBranch - fun:MOLToSMIExt - fun:CanSmilesStep - fun:CanSmiles - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi -} -{ - Avalon #3 - Memcheck:Cond - fun:SmilesBranch + ... fun:SmilesBranch fun:MOLToSMIExt fun:MOLToSMI - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi -} -{ - Avalon #4 - Memcheck:Cond - fun:SmilesBranch - fun:MOLToSMIExt - fun:MOLToSMI - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi -} -{ - Avalon #5 - Memcheck:Cond - fun:SmilesBranch - fun:SmilesBranch - fun:SmilesBranch - fun:SmilesBranch - fun:MOLToSMIExt - fun:MOLToSMI - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi -} -{ - Avalon #6 - Memcheck:Cond - fun:SmilesBranch - fun:SmilesBranch - fun:SmilesBranch - fun:SmilesBranch - fun:MOLToSMIExt - fun:CanSmilesStep - fun:CanSmiles - fun:_ZN11AvalonTools14getCanonSmilesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbi + fun:_ZN11AvalonTools14getCanonSmiles* } { boost::singleton #1