Hi Jason,

Correct. You'd probably better use std::unique_ptr unless you are planning to reference the original pointer from multiple std::shared_ptr's.

Cheers,
p.


On 11/01/2018 14:42, Jason Biggs wrote:

On Thu, Jan 11, 2018 at 8:03 AM, Paolo Tosco <paolo.to...@unito.it <mailto:paolo.to...@unito.it>> wrote:

    Hi Jason,

    I believe the problem here is that if you are building outside
    CMake the WIN32 preprocessor macro is not defined (_WIN32 is). So,
    when ROMol.h is parsed, the ROMol class definition includes a
    "private" directive that should not be there, hence the error. To
    fix the issue, you need to add to your cl command line a /DWIN32.


Thank you!  This did the trick.  I should learn to use CMake instead of compiling everything in Mathematica (when all you have is a hammer, everything looks like a nail)

    As a side note, I am not sure you really want to do:

            RDKit::ROMol mol = *(RDKit::SmilesToMol(smi));
            std::string res = RDKit::MolToSmiles(mol);

    In fact, this makes a copy of the molecule that the pointer
    returned by SmilesToMol() points to, while leaking the memory
    pointed to.
    If you are not interested in making a copy of the returned
    molecule, you should probably rather do:

        RDKit::RWMol *mol = RDKit::SmilesToMol(smi);
        std::string res = RDKit::MolToSmiles(*mol);
        delete mol;


You are definitely correct here, I was trying to make the simplest failing example and the example without the copy compiled fine - declaring a pointer to an ROMol is fine but declaring an actual object triggered the error.

I'm very new to c++, will keep in mind to always delete my pointers.   If I understand correctly, if I always wrap my ROMol objects in a std::shared_ptr (or the boost equivalent), then the memory will not be leaked.  Is that right?

Best,
Jason



    If you actually meant to make a copy, you'd better do:

        RDKit::RWMol *mol = RDKit::SmilesToMol(smi);
        RDKit::ROMol molCopy(*mol);
        delete mol;
        std::string res = RDKit::MolToSmiles(molCopy);

    Cheers,
    p.


    On 11/01/2018 02:33, Jason Biggs wrote:
    I'm trying to use the rdkit as a library in another project, and
    am having trouble getting it to build on windows.  I can get the
    code to compile on mac and linux, but it fails for windows, both
    32-big and 64-bit varieties.  I don't know how specific this is
    to the rdkit, but I have zero experience compiling with visual
    studio (and very little C++ coding background) and I am very
    confused here.  The following is just a toy example showing the
    minimum necessary for me to get the error.  I get the same error
    using the full code.


    If I create a test class using this code, I can compile it just
    fine in windows:


        #include <GraphMol/GraphMol.h>

        #include <GraphMol/SmilesParse/SmilesParse.h>

        #include <GraphMol/SmilesParse/SmilesWrite.h>


        classtestClass{

        testClass();

        ~testClass();

        std::string testFunc() {

        std::string smi = "CCC";

        RDKit::ROMol mol = *(RDKit::SmilesToMol(smi));

        std::string res = RDKit::MolToSmiles(mol);

        returnres;

        };

        };


        testClass::testClass() {

        }


        testClass::~testClass() {

        }



    But if I move the definition of the testFunc() function outside
    of the class declaration (which is the normal case, where the
    definitions are in separate files), like this


        #include <GraphMol/GraphMol.h>

        #include <GraphMol/SmilesParse/SmilesParse.h>

        #include <GraphMol/SmilesParse/SmilesWrite.h>



        classtestClass{

        testClass();

        ~testClass();

        std::string testFunc();

        };


        testClass::testClass() {

        }


        testClass::~testClass() {

        }


        std::string testClass::testFunc() {

        std::string smi = "CCC";

        RDKit::ROMol mol = *(RDKit::SmilesToMol(smi));

        std::string res = RDKit::MolToSmiles(mol);

        returnres;

        };

    then I get the following linker errors:


    error LNK2019: unresolved external symbol "private: virtual void
    __thiscall RDKit::ROMol::destroy(void)"
    (?destroy@ROMol@RDKit@@EAEXXZ) referenced in function "public:
    virtual __thiscall RDKit::ROMol::~ROMol(void)"
    (??1ROMol@RDKit@@UAE@XZ)
    failing.obj : error LNK2019: unresolved external symbol "private:
    void __thiscall RDKit::ROMol::initFromOther(class RDKit::ROMol
    const &,bool,int)" (?initFromOther@ROMol@RDKit@@AAEXABV12@_NH@Z)
    referenced in function "public: __thiscall
    RDKit::ROMol::ROMol(class RDKit::ROMol const &,bool,int)"
    (??0ROMol@RDKit@@QAE@ABV01@_NH@Z)
    
C:\Users\IEUser\Documents\rdkitlink_windows_compile_issue\Working-ie11win7-3268-3284-12\RDKitLink.dll
    : fatal error LNK1120: 2 unresolved externals


    I can't understand why moving the definition of testFunc() causes
    this error.

    Any help would be most appreciated. Clearly I can use the
    workaround of making all definitions for testClass in the header
    file, but I would rather not do that.

    Thank you,
    Jason



    
------------------------------------------------------------------------------
    Check out the vibrant tech community on one of the world's most
    engaging tech sites, Slashdot.org!http://sdm.link/slashdot


    _______________________________________________
    Rdkit-discuss mailing list
    Rdkit-discuss@lists.sourceforge.net
    <mailto:Rdkit-discuss@lists.sourceforge.net>
    https://lists.sourceforge.net/lists/listinfo/rdkit-discuss
    <https://lists.sourceforge.net/lists/listinfo/rdkit-discuss>



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Rdkit-discuss mailing list
Rdkit-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rdkit-discuss

Reply via email to