Colliding MockingContext ids when used in multiple translation units because of __COUNTER__

[otrempe]

MyInterface.h

#pragma once

class MyInterface
{
public:
    virtual ~MyInterface() = default;

    virtual std::string        MyStringMethod() const = 0;
    virtual std::vector< int > MyVectorMethod() const = 0;
};

MyMock.h

#pragma once

#include <fakeit.hpp>

#include "MyInterface.h"

class MyMock : public fakeit::Mock< MyInterface >
{
public:
    void MyStringMethodStub();
    void MyVectorMethodStub();
};

MyMock.cpp

#include "MyMock.h"

void MyMock::MyStringMethodStub()
{
    fakeit::When( Method( *this, MyStringMethod ) ).AlwaysReturn< std::string >( "MyString" ); // __COUNTER__ == 10
}

void MyMock::MyVectorMethodStub()
{
    fakeit::When( Method( *this, MyVectorMethod ) ).AlwaysReturn< std::vector< int > >( { 1, 2, 3 } ); // __COUNTER__ == 11
}

MyMethodStubs.h

#pragma once

class MyMock;

void MyVectorMethodStub( MyMock& myMock_ );
void MyStringMethodStub( MyMock& myMock_ );

MyMethodStubs.cpp

#include "MyMethodStubs.h"

#include <fakeit.hpp>

#include "MyMock.h"

void MyVectorMethodStub( MyMock& myMock_ )
{
    fakeit::When( Method( myMock_, MyVectorMethod ) ).AlwaysReturn< std::vector< int > >( { 4, 5, 6 } ); // __COUNTER__ == 10
}

void MyStringMethodStub( MyMock& myMock_ )
{
    fakeit::When( Method( myMock_, MyStringMethod ) ).AlwaysReturn< std::string >( "OtherString" ); // __COUNTER__ == 11
}

MyTest.cpp

#include "MyMethodStubs.h"
#include "MyMock.h"

BOOST_AUTO_TEST_CASE( CollidingIdsTest )
{
    MyMock myMock;

    MyVectorMethodStub( myMock ); // __COUNTER__ == 10 in MyMethodStubs.cpp

    myMock.MyStringMethodStub(); // __COUNTER__ == 10 in MyMock.cpp. Override previous stub with wrong id.

    auto myVector = myMock.get().MyVectorMethod(); // Calls MyStringMethod under the hood, leading to a crash
}

I am using Visual Studio 2017.

The above code snippet is a simplified demonstration of the problem. It is a lot more difficult to pinpoint in a real test setup.

I cannot propose a solution since I don't have a deep understanding of fakeit internal wizardry.

This is a major show stopper to better architecture a testing setup, especially considering the bloating of obj files by fakeit symbols rapidly breaking the maximum number of sections allowed, requiring to either split the source files in multiple translation units, or using the /bigobj switch.

[malcolmdavey]

Hadn't seen that one before. I guess I haven't been making stubs implemented in another file before.

A quick fix to fakeit could be using the filename, line number with the counter .. not sure how much work. Would need to make the id a string instead of an int.

Note sure of proper fix, or a solution to avoid the whole problem in the first place.

[otrempe]

Combining counter, line number and filename would still produce multiple entries for the same stub in the invocationHandlerCollection. I am not sure if there are any pitfalls doing this for properly handling different flavors of stubbing (Return, AlwaysReturn, ...) and verification (AtLeast, Exactly, ...)

[FranckRJ]

String literals cannot be used as non type template parameter, there are workarounds but only in C++20 as far as I know. We could create a hash from filename + __COUNTER__, or use addresses of uniquely created variables (albeit I don't think this one is a good idea).

Combining counter, line number and filename would still produce multiple entries for the same stub in the invocationHandlerCollection.

I haven't looked too deep into this part of the code but I don't understand what you mean. When the method stub is called it is currently always called with a different ID (if everything is done in the same file), even for two stubs of the same method, it's the intended behavior from what I can see. And the ID of a specific stub don't change because it is only set once (in the Method macro and its alternatives).

I there something I don't understand ? Could you detail a bit more what kind of issues an ID generated from the filename would cause ?

[otrempe]

Actually, you are most likely right that it's not a problem. Multiple calls to the same stub do indeed already generate multiple ids. I didn't dive into the implementation, but I was left with the wrong idea that a specific stub should have one and only one unique id.

I don't have the knowledge to determine if generating an ID from the filename could lead to other issues. If it's a possible fix, it should definitely be explored as a solution.

[otrempe]

I am about to submit a pull request for this one. I got working code locally.

The id is hashed from combining __FILE__ and __COUNTER__. The hashing has to be constexpr. std::hash is not constexpr. I just copied VS implementation and made a constexpr version of it.

I need to get the length of the string literal to compute its hash. I used std::char_traits<char>::length which is constexpr, but only since C++17.

What are the requirements regarding the support of different versions of the standard?

[FranckRJ]

Every features should work on C++11 and beyond, but you can have some optimizations or compile-time checks that are only on newer versions. For example for the ReturnAndSet method the validity of the argument type is checked at compile time on C++17 whereas it is only checked at run-time in C++11.

[FranckRJ]

If you don't want to manually write a loop, something like that also work:

template <int Size>
constexpr int hash(const char (&filename)[Size], int counter)
{
    return Size + counter;
}

int main()
{
    hash(__FILE__, __COUNTER__);
    return 0;
}

https://godbolt.org/z/Tvc7GWjKz