Undefined reference to member function and variable

Question

I have the following header file MappingSingleton.h:

#include <vector>
#include <string>
#include <fstream>
#include <iostream>
#include<iterator>
#include <sstream>
#include <thread>
#include <mutex>

#include "SOPExpr.h"
class MappingSingleton {
private:
    static MappingSingleton& mapping_singleton;
    MappingSingleton();
public:
    static unsigned int wireID;
    static bool is_init;
    std::vector<SOPExpr> m_hex_to_SOP_4;
    std::vector<int> m_hex_to_line_number_4;
    std::vector<SOPExpr> m_hex_to_SOP_3;
    std::vector<int> m_hex_to_line_number_3;
    std::vector<SOPExpr> m_hex_to_SOP_2;
    std::vector<int> m_hex_to_line_number_2;
    void loadLUTNHexToStringMapping(std::vector<SOPExpr>& m_hex_to_SOP, std::vector<int> &m_hex_to_line_number, short int LUT_size);
    std::string getQTPrims(std::string& LUT_init_string);
    static MappingSingleton getInstance();
private:
    static void initSingleton();
    static std::once_flag init_instance_flag;
    int getExprIndex(std::string &key, short lut_size);
    int wire_id;

};

And the following function in MappingSingleton.cpp:

MappingSingleton MappingSingleton::getInstance() {
    std::call_once(MappingSingleton::init_instance_flag,&MappingSingleton::initSingleton);
    return mapping_singleton;
}

The compiler gives the following error:

/grid/cva/p4_02/hisham/gcc/ua/Framework/Roman/src/MappingSingleton.cpp:37: undefined reference to `MappingSingleton::initSingleton()'
/grid/cva/p4_02/hisham/gcc/ua/Framework/Roman/src/MappingSingleton.cpp:37: undefined reference to `MappingSingleton::init_instance_flag'
/grid/cva/p4_02/hisham/gcc/ua/Framework/Roman/src/MappingSingleton.cpp:38: undefined reference to `MappingSingleton::mapping_singleton'

I don't understand why seeing as I have clearly defined those variables/functions in the header. I have tried reading many questions about this error, but none of the proposed solutions resolve the error. My main function just initializes Mapping Singleton.

MappingSingleton.cpp has the implementations:

#include "MappingSingleton.h"

#include "HelperFunction.h"
#include <regex>
#include <cmath>
#include "WireNameReplacement.h"

bool MappingSingleton::is_init=false; //this is static in the header file
std::once_flag init_instance_flag;


MappingSingleton::MappingSingleton() {
    wire_id=0; //used for avoiding duplicate wires.
    //Only one mapping generator at a time. Singleton design pattern.
        //Load mappings for the three LUTS
    loadLUTNHexToStringMapping(m_hex_to_SOP_4,m_hex_to_line_number_4,4);
    loadLUTNHexToStringMapping(m_hex_to_SOP_3,m_hex_to_line_number_3,3);
    loadLUTNHexToStringMapping(m_hex_to_SOP_2,m_hex_to_line_number_2,2);
}

 MappingSingleton MappingSingleton::getInstance() {
    std::call_once(MappingSingleton::init_instance_flag,&MappingSingleton::initSingleton);
    return mapping_singleton;
}

void MappingSingleton::loadLUTNHexToStringMapping(std::vector<SOPExpr>& m_hex_to_SOP, std::vector<int> &m_hex_to_line_number, short int LUT_size){
    //Read in Vivado Hex key of each SOP for LUT 4
    std::ifstream iFS("TextAndPythonFiles/LUT" +std::to_string(LUT_size)+"/lutListHex");
    std::string temp_str;
    m_hex_to_line_number.resize(pow(2,pow(2,LUT_size))); // 2^(2^4) for 4-LUT
    int tempInt=0;
    std::stringstream ss;
    while (std::getline(iFS, temp_str, '\n')) {
        m_hex_to_line_number[hexToInteger(temp_str)]=tempInt;
        ++tempInt;
    }
    //Read In the SOP Strings from a file. Reserve vector space first though to avoid reallocating vector. 
    //m_hex_to_SOP.resize(pow(2,pow(2,LUT_size)));
    std::ifstream iFS2("TextAndPythonFiles/LUT"+std::to_string(LUT_size)+"/combinedTrimmedFinal.vg");
    int tempInt2=0;
    std::string current_SOP;
    while (std::getline(iFS2, temp_str, '\n')) {
        if(temp_str.empty()) //Blank line implies new module
        {
            if(!current_SOP.empty()) current_SOP.pop_back(); //Erase last new line. Not necessary.
            SOPExpr expr(current_SOP);
            m_hex_to_SOP.push_back(expr);
            current_SOP.clear();
            continue;
        }
        current_SOP.append(temp_str);
        current_SOP.push_back('\n');
        tempInt2++;
    }



    //   createHexVec(line_num_to_SOP,m_hex_to_line_number); Reviewers please ignore
    std::string test="Q_OAI222 g514(.A0 (n_5), .A1 (n_0), .B0 (n_4), .B1 (n_1), .C0 (n_2), .C1 (n_3), .Z (n_6));\n Q_INV g517(.A (i0), .Z (n_5));Q_INV g518(.A (i2), .Z (n_4));";
    //std::cout<<findOutputWire(test);
}
std::string MappingSingleton::getQTPrims(std::string& vivado_LUT_string)
{
    //find LUT Size
    std::smatch match;
    std::regex re("LUT([0-9])");
    regex_search(vivado_LUT_string, match, re);
    int LUT_size=std::stoi(match.str(1));

    std::string key= extractHexKey(vivado_LUT_string);
    int expr_index;
    std::string qt_prims;
    std::deque<std::string> d;
    switch(LUT_size){
        case 2: expr_index= getExprIndex(key,2);
                qt_prims= m_hex_to_SOP_4[expr_index].getGateLevelNetList();
                d=findWireNames(vivado_LUT_string);
                qt_prims=replaceWireNames(d,vivado_LUT_string,qt_prims,2);
                replaceAllInternalWires(qt_prims,wire_id);
                return qt_prims;
        case 3: expr_index= getExprIndex(key,3);
                qt_prims= m_hex_to_SOP_4[expr_index].getGateLevelNetList();
                d=findWireNames(vivado_LUT_string);
                qt_prims= replaceWireNames(d,vivado_LUT_string,qt_prims,3);
                replaceAllInternalWires(qt_prims,wire_id);
                return qt_prims;
        case 4: expr_index= getExprIndex(key,4);
                qt_prims= m_hex_to_SOP_4[expr_index].getGateLevelNetList();
                d=findWireNames(vivado_LUT_string);
                qt_prims=replaceWireNames(d,vivado_LUT_string,qt_prims,4);
                replaceAllInternalWires(qt_prims,wire_id);
                return qt_prims;
        default:
            throw std::invalid_argument("Invalid LUT Init Val");
    }
    return "Invalid LUT Init Val";
}

int MappingSingleton::getExprIndex(std::string& key, short int LUT_size){

    switch(LUT_size){
        case 2: return m_hex_to_line_number_2[hexToInteger(key)];
        case 3: return m_hex_to_line_number_3[hexToInteger(key)];
        case 4: return m_hex_to_line_number_4[hexToInteger(key)];
        default:
            throw std::invalid_argument("Invalid LUT Init Val");
    }
}

Can you [edit your question](https://stackoverflow.com/posts/57151692/edit) and post it here? — , Jul 22 '19 at 18:22
How exactly do you compile that code? We can't help you if you don't provide enough information. See [mcve]. — HolyBlackCat, Jul 22 '19 at 18:24
This singleton looks weird. Your static member is a reference type and your `getInstance()` function returns by value. Usually it's the opposite. Returning a (non-local) object by value means copying, which breaks the meaning of the singleton pattern. Alternatively, you can have a static `MappingSingleton` instance inside of `getInstance()` to which a reference is returned, and you can do all the work of initialization inside the constructor, which means you can throw out the `std::once_flag` and `MappingSingleton::::mapping_singleton` and your code would be cleaner and achieve the same thing. — alter_igel, Jul 22 '19 at 18:50
See this question for a better singleton implementation: https://stackoverflow.com/questions/1008019/c-singleton-design-pattern — alter_igel, Jul 22 '19 at 18:56
@alterigel I've seen the Meyers Singelton and it certainly looks shorter... but I feel that call_once is more explicit that taking advantage of a certain property of static variables to make your code thread safe. In any case your comment is very helpful. — Hisham Hijjawi, Jul 22 '19 at 19:05

score 5 · Answer 1 · answered Jul 22 '19 at 18:31

5

So, first of all, this is wrong:

std::once_flag init_instance_flag;

Should be this instead:

std::once_flag MappingSingleton::init_instance_flag; // maybe an init value here too

Secondly, even though you declared MappingSingleton::mapping_singleton and MappingSingleton::initSingleton(), you don't have a definition for them. It will ultimately look something like this:

// the actual definition of the variable
MappingSingleton& MappingSingleton::mapping_singleton;

// the actual implementation of the function
void MappingSingleton::initSingleton() {
   // some implementation...
}

That's missing from your code, and is what is probably causing your error.

answered Jul 22 '19 at 18:31

2

One problem the OP is going to have it that their singleton object is actually reference. Since it's not possible to bind a reference after it has been defined it would be better implemented as a pointer. – john Jul 22 '19 at 18:33
I'm not sure what you mean, but I'm sure it's a fair critique. I'm sure an example of what you mean will be a good addition to your answer. – Jul 22 '19 at 18:37
@john by bind a reference do you mean assign a reference? An example of why I should use a pointer would be helpful in your answer. – Hisham Hijjawi Jul 22 '19 at 18:48
@user3586940 It's impossible to assign to a reference. If you try to assign to a reference all that happens is that you assign to the object being referred to. I'll update my answer. – john Jul 22 '19 at 18:48

john · Accepted Answer · 2019-07-22T18:56:58.567

So this

std::once_flag init_instance_flag;

should be this

std::once_flag MappingSingleton::init_instance_flag;

and the other two items initSingleton() and mapping_singleton and have not been defined anywhere I can see

What you have in your header file are declarations not definitions. You need a definition as well.

I'd also change mapping_singleton to be a pointer. Hard to see any benefit in it being a reference given the limitations on references.

UPDATE

Presumably you have made mapping_singleton a reference so that you can create only when it is required. Something like this

MappingSingleton& MappingSingleton::mapping_singleton;

void MappingSingleton::initSingleton()
{
    mapping_singleton = *new MappingSingleton();
}

but this doesn't work since you cannot assign to a reference. A reference is bound when it is initialised and can't be rebound at a later time. The assignment above copies the newly created MappingSingleton to the MappingSingleton object that the reference is already bound to (which doesn't exist).

Instead the following works

MappingSingleton* MappingSingleton::mapping_singleton = nullptr;

void MappingSingleton::initSingleton()
{
    mapping_singleton = new MappingSingleton();
}

MappingSingleton& MappingSingleton::getInstance() {
    std::call_once(MappingSingleton::init_instance_flag,&MappingSingleton::initSingleton);
    return *mapping_singleton;
}

Notice I changed the return type of getInstance to be a reference. It's not illegal to have a non-reference there but it would mean that your singleton object would be copied when you call getInstance which rather defeats the purpose of having a singleton.

I'm glad you addressed the weird reference member and returning by value, but I don't think making `mapping_singleton` a pointer is the best fix. This solution has a memory leak that is difficult to fix. On the other hand, if you implement the singleton [this way](https://stackoverflow.com/questions/1008019/c-singleton-design-pattern), and initialize everything in the constructor, then you can throw out all of `mapping_singleton`, `call_once`, `init_singleton_flag`, `initSingleton` and `is_init`, while achieving the exact same thing. — alter_igel, Jul 22 '19 at 19:04
@alterigel Yeah that looks good. I presumed the OP had good reason for all the baggage around his implementation, so I didn't mess with it. — john, Jul 22 '19 at 19:10

Undefined reference to member function and variable

2 Answers2