HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Typetokens in C++11

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
typetokensstackoverflowprogramming

Problem

I have written a typetoken library in C++11 and I want to know if it is indeed typesafe, threadsafe and complete.

The purpose of this library is to give an unique ID to every requested typename during runtime.

The code I used is the following:

/*
 * typetoken.hpp
 *  Created on: 27.11.2014
 *      Author: WorldSEnder
 */

#pragma once
#ifndef TYPETOKEN_HPP_
#define TYPETOKEN_HPP_

#include 

namespace typetoken
{

using token_t = std::size_t;
template token_t getToken(void);

class tokenbase {
private:
    tokenbase(void) = delete;
    static volatile std::atomic counter;

    template
    friend token_t getToken(void);
};

template
struct typetoken : tokenbase {
private:
    typetoken(void) = delete;
    static token_t ID;

    template
    friend token_t getToken(void);
};

template
token_t typetoken::ID = 0;

template
token_t getToken(void) {
    typedef typetoken token;
    if(!token::ID) {
        token::ID = ++token::counter;
    }
    return token::ID;
}
} // namespace typetoken

#endif /* TYPETOKEN_HPP_ */

/*
 * typetoken.cpp
 *  Created on: 27.11.2014
 *      Author: WorldSEnder
 */
#include "typetoken.hpp"

namespace typetoken
{

volatile std::atomic tokenbase::counter = ATOMIC_VAR_INIT(1);
volatile std::atomic impltokenbase::counter = ATOMIC_VAR_INIT(1);

}


You can use this to map a typename to any other object during runtime, for example:

#include 
#include 
#include "typetoken.hpp"

class Mapper {
    std::map mapObj;
public:
    template
    void setFor(float object) {
        mapObj.insert(std::make_pair(typetoken::getToken(), object));
    }
    template
    float getFor(void) {
        return (*mapObj.find(typetoken::getToken())).second;
    }
};

int main(void) {
    Mapper map;
    map.setFor(2.5f);
    map.setFor(3.5f);
    std::cout () () << std::endl;
}
// Outputs: "3.5\n2.5"


I have the following doubts about this implementation:

  • When two threads create a token for the same type at the same time, it could be set simult

Solution

I used your code with the following driver to test it out.

#include 
#include 
#include 
#include "typetoken.h"

#define SHOW(x) std::cout  typetoken::tokenbase::counter{100};

void f(int i) { 
    SHOW(typetoken::getToken());
    SHOW(typetoken::getToken());
    SHOW(typetoken::getToken());
}
void g(int i) { 
    SHOW(typetoken::getToken());
    SHOW(typetoken::getToken());
    SHOW(typetoken::getToken());
}

int main() {
    std::vector v;
    const int threadcount = 4;
    for (int i=0; i < threadcount; ++i) {
        v.push_back(std::move(std::thread{f, i}));
        v.push_back(std::move(std::thread{g, ++i}));
    }
    for (auto &thr : v) 
        thr.join();
}


The output varies from run to run, as we should expect. In some runs float gets assigned a value of 101, and in others 102, but after a value is assigned, it does not change and there is no possibility (that I can detect) that a value will be duplicated.

With that said, there are a few minor things that I think could be improved.

Don't use volatile unless hardware can change the value

The C++ keyword volatile only means that something outside of the program may alter the underlying value during the program's run. That does not appear to be the case here, and so counter should not be declared volatile. As Stroustrup writes:


Do not assume that volatile has special meaning in the memory model. It does not. It is not -- as in some later languages -- a synchronization mechanism.

Delete all constructors that are not desired

You have wisely deleted default constructors for tokenbase and typetoken which will help to prevent casual misuse, but what about more dedicated attempts at misuse? This example is admittedly perverse, but still possible:

typetoken::tokenbase tb(*reinterpret_cast(77));


We now have a tokenbase object although it's almost guaranteed to be malformed. We could delete the automatically generated copy constructor, move constructor, etc. but there's a better way. The way to prevent these kinds of abuse is by adding these two lines to the declaration of tokenbase:

~tokenbase() = delete;
void *operator new(size_t) = delete;


The first makes it impossible to have a local variable of type tokenbase and the latter prevents one from being created on the free store.

Prevent other abuses

If we want to further subvert the use of this mechanism, we could define this:

namespace typetoken{
    template<>
    token_t getToken(void) { return typetoken::counter = 42; }
}


And then use it:

typetoken::getToken();
typetoken::getToken();
typetoken::getToken();
typetoken::getToken();


It has the effect of resetting the counter so that in this case, both int and double will be assigned the value of 43.

Perhaps the users of your class are not as destructively creative, so it may well be reasonable to simply expect that users of the class won't do this, but it can also be actively prevented. Instead of having getToken() be a friend of tokenbase you could omit that friend declaration from within tokenbase and then change typetoken and getToken to these:

template
struct typetoken : tokenbase {
private:
    static token_t ID;
    static token_t getToken(void) {
        return ID ? ID : ID = ++typetoken::counter;
    }
    template
    friend token_t getToken(void);
};

template
token_t getToken(void) {
    return typetoken::getToken();
}


Now the "abusive" function simply won't compile, preventing the problem. The problem is simplified still further if we can accept the elmination of the "syntactic sugar" of the friend function. That is, if we can accept this:

typetoken::typetoken::getToken();


instead of this:

typetoken::getToken();


We can eliminate the friend, and make member function getToken public.

Code Snippets

#include <iostream>
#include <thread>
#include <vector>
#include "typetoken.h"

#define SHOW(x) std::cout << i << " says " # x " = " << x << '\n'
std::atomic<typetoken::token_t> typetoken::tokenbase::counter{100};

void f(int i) { 
    SHOW(typetoken::getToken<float>());
    SHOW(typetoken::getToken<unsigned long>());
    SHOW(typetoken::getToken<double>());
}
void g(int i) { 
    SHOW(typetoken::getToken<unsigned long>());
    SHOW(typetoken::getToken<float>());
    SHOW(typetoken::getToken<double>());
}

int main() {
    std::vector<std::thread> v;
    const int threadcount = 4;
    for (int i=0; i < threadcount; ++i) {
        v.push_back(std::move(std::thread{f, i}));
        v.push_back(std::move(std::thread{g, ++i}));
    }
    for (auto &thr : v) 
        thr.join();
}
typetoken::tokenbase tb(*reinterpret_cast<typetoken::tokenbase *>(77));
~tokenbase() = delete;
void *operator new(size_t) = delete;
namespace typetoken{
    template<>
    token_t getToken<void>(void) { return typetoken<void>::counter = 42; }
}
typetoken::getToken<void>();
typetoken::getToken<double>();
typetoken::getToken<void>();
typetoken::getToken<int>();

Context

StackExchange Code Review Q#71096, answer score: 5

Revisions (0)

No revisions yet.