std::set used as a static templated member variable

Question

I am trying to make something like a Java style Enum, which I'm calling a flag. The requirements are that each flag is static so flags are directly referencable, each flag storing the string of it's name and the whole set iterable and conducive to lookups.

I am using templating so that each set of flags is stored separately (thus saving me from having to explicitly place a set in each child class).

I am convinced this is an initiation problem because the success or failure of running the program depends on the filename of the object file which contains the flag declarations (A.o segfaults but Z.o runs fine.)

The problem seems to be one of static initialization order, this code compiles perfectly fine but when it is run, gdb produces the following:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff751e0fa in std::_Rb_tree_decrement(std::_Rb_tree_node_base*) ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6
(gdb) bt
#0  0x00007ffff751e0fa in std::_Rb_tree_decrement(std::_Rb_tree_node_base*) ()
from /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/libstdc++.so.6
#1  0x0000000000462669 in operator-- ()
at /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/include/g++-v4/bits/stl_tree.h:199
#2  _M_insert_unique ()
at /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/include/g++-v4/bits/stl_tree.h:1179
#3  insert () at /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/include/g++-v4/bits/stl_set.h:411
#4  Flag () at include/../util/include/Flag.hpp:34
#5  ItemFlag () at include/Item.hpp:22
#6  __static_initialization_and_destruction_0 () at Item.cpp:15
#7  global constructors keyed to _ZN3code8ItemFlag5brickE() () at Item.cpp:86
#8  0x000000000046ac62 in ?? ()
#9  0x00007fffffffddc0 in ?? ()
#10 0x000000000046abb0 in ?? ()
#11 0x0000000000692c0a in ?? ()
#12 0x0000000000407693 in _init ()
#13 0x00007ffff7dded08 in ?? () from /usr/lib64/libboost_serialization-1_42.so.1.42.0
#14 0x000000000046abe7 in __libc_csu_init ()
#15 0x00007ffff6cd9b50 in __libc_start_main () from /lib64/libc.so.6
#16 0x0000000000408329 in _start ()

My code is as follows:

template <class FlagType> class Flag
{
public:

    Flag(int ordinal, String name):
    ordinal(ordinal),
    name(name)
    {
        flagSet.insert(this);
    }

    inline bool operator==(const Flag<FlagType>& e) const
    {
                    //edited due to comment
        //if(this->ordinal == e.getOrdinal()) return true;
        //else return false;
                    return (this->ordinal == e.getOrdinal());

    }

    inline bool operator!=(const Flag<FlagType>& e) const
    {
        return !(*this==e);
    }

    static const std::set<const Flag<FlagType>*>& flagValues()
    {
        return flagSet;
    }

    const String& toString() const
    {
        return name;
    }

    const size_t& getOrdinal() const
    {
        return ordinal;
    }

    static int size()
    {
        return flagSet.size();
    }

    static const Flag<FlagType>& valueOf(const String& string)
    {
        typename std::set<const Flag<FlagType>*>::const_iterator i;
        for(i = flagSet.begin(); i != flagSet.end(); i++)
        {
            if((**i).toString().startsWith(string))
            {
                return **i;
            }
        }
        throw NotAFlagException();
    }

protected:

    static std::set<const Flag<FlagType>*> flagSet;

    size_t ordinal;
    String name;
    private:
            //added in response to comment to prevent copy and assignment, not compile tested
            Flag<FlagType>(const Flag<FlagType>&);
            Flag<FlagType>& operator=(const Flag<FlagType>&);
};

template <class FlagType> std::set<const Flag<FlagType>*> Flag<FlagType>::flagSet; //template

Item.hpp

    class ItemFlag: public Flag<ItemFlag>
{
public:

    static const ItemFlag brick;

private:

    ItemFlag(int ordinal, String name):
    Flag<ItemFlag>(ordinal, name){}
};

Item.cpp

const ItemFlag ItemFlag::brick(1, "brick");

My first post, so please let me know if I've got the formatting wrong or have been unspecific. PS. curiously, replacing set with vector results in a working program, as if the set is having trouble with inserting the pointers specifically. To test this I replaced the set with a set of int and tried to insert 0 on class initialization, this also resulted in the same error.

`if (…) return true; else return false;` is an anti-pattern. Use `return …;`! — Konrad Rudolph, May 09 '11 at 13:51

score 5 · Accepted Answer · answered May 09 '11 at 14:07

5

It could very easily be an order of initialization issue. You basically need to use some sort of lazy initialization for the set, e.g.

static std::set<Flag<FlagType> const*>& flagSet()
{
    static std::set<Flag<FlagType> const*> theOneAndOnly;
    return theOneAndOnly;
}

instead of your static variable.

And while I'm at it: this is probably not a good use of templates. A much better solution would be to generate the code from a file with a much simpler format, something along the lines of:

[EnumName]
constant_name_1
constant_name_2

etc. It would probably take no more than about 10 lines of AWK, Perl or Python (according to your tastes) to parse this and output both a C++ header and a C++ source file for it. You then only have to maintain the simple format.

answered May 09 '11 at 14:07

James Kanze

150,581
18
184
329

Aha, the lazy initialization is what I needed, thank you very much. This use of templates did seem extremely convoluted and I guessed it was probably not the best idea. I will certainly look into learning perl and using that instead, though. I would upvote you, but apparently I don't have the reputation to do so, so I hope everyone else upvotes you until I get enough. – user745247 May 09 '11 at 14:21
I’m not convinced that this isn’t a good use of templates. I might have some detail criticisms of the design but in general I think that strongly-typed enums are a prime use-case of CRTP. – Konrad Rudolph May 09 '11 at 14:26
@user745247 If you don't already know one of the scripting languages, don't learn perl:-). I use awk, but that's because I learned it long before either perl or Python existed; if you're starting from scratch, I'd recommend Python (probably, based on what I've heard about it---I don't actually know it). – James Kanze May 09 '11 at 14:30
@Konrad I'm sure you can implement it with templates, but why do things the hard way. In this case, using templates means that you pay all of the costs for them (readability, compile times, etc.), and don't really reap any benefit: you don't need any information internal to the compiler, so there's no point in using an overly complicated compile time technique when very simple pre-compiler techniques are available. – James Kanze May 09 '11 at 14:33
1

@James As if precompilers don’t also incur a cost (maintainance, comprehensiveness, more complex build system, flexibility …). Templates solve this very elegantly. Again, I’ll refer you to CRTP, whose sole purpose is code generation as required in the example at hand. Compile time isn’t relevant here: this can be pushed off into a separate compilation unit (for once) so it’s *less* expensive than any precompiler you’d care to name. Finally, Java implements its enums via CRTP. – Konrad Rudolph May 09 '11 at 14:47
@Konrad But not a great cost. Whereas templates are extremely difficult to read and maintain, as soon as they cease to be trivial. I wouldn't call any solution involving non-trivial templates "elegant". – James Kanze May 09 '11 at 15:44
@James Arguably CRTP is trivial. Or at least, trivial for C++ standards. Either way, it’s a well-studied, well-understood, widely used idiom. That particular train has left the station. – Konrad Rudolph May 09 '11 at 16:03
@Konrad CRTP isn't trivial, but it's fairly simple; I use it myself a lot (e.g. for implementing overloaded operators). But I don't quite see how it applies here: the problem here is to generate `n` distinct objects, given `n` names. Java can only use CRTP because it uses compiler magic to know about these names; in C++, it would require some seriously tricky meta-programming. Unless you've got an idea I haven't thought of (in which case, you'll be bitten by the character limit of a comment). – James Kanze May 09 '11 at 16:40
If this usage is fairly simple, perhaps someone knows of a similar design already done which I could use for reference, as although my design works I am sure it is not the best way of doing it as I am fairly new to c++ – user745247 May 09 '11 at 16:44
@James A good many operators *are* shared for enums so CRTP helps creating them (see OP’s code). Furthermore, I’m not preaching purity. Coupled with macros (BOOST_PP), we can generate enums easily without any compiler magic. Also, let me show you how I beat the character limit: https://gist.github.com/963346/ ;-) – Konrad Rudolph May 09 '11 at 20:42
@Konrad OK. I see what you're getting at. I was thinking of how to create the individual instances for each enum value; you were thinking of the behavior of each enum class. I think we're both right; CRTP is very appropriate for the common behavior of all enumerated types; an external generator is more appropriate for generating the actual class that the client code sees, with its unique instances for each value. – James Kanze May 10 '11 at 07:35

score 0 · Answer 2 · answered May 09 '11 at 13:59

0

If your class's constructor inserts items into the static set then its destructor should remove them. You also probably need a copy constructor and assignment operator. aso, on a point of style, protected data is normally considered A Bad Thing.

answered May 09 '11 at 13:59

I never planned on the static flags being destructed, they are static const members of the class, so the destructor is never called. Unless I should add it in for completeness in case of a later redesign. – user745247 May 09 '11 at 14:02
@user Whether or not you plan on them being destructed, they will be. They will also be copy constructed if you ever use a flag as a non-reference function parameter or return type. – May 09 '11 at 14:06
I'll add a private copy constructor and assigner to prevent further instances being made and always refer to the flags by reference then, that should ensure that the only flags in existence are those I statically assign and everywhere references and pointers to them are used. – user745247 May 09 '11 at 14:10

score 0 · Answer 3 · answered May 09 '11 at 14:06

0

The order of static initialization between different translation units are not guaranteed. From the dump it seems like the ItemFlags are created before the set is constructed. That makes the insert fail.

Changing the name of the file just might affect the order of the file in the linking process. That would explain why a file starting with an A is linked in early.

The only way to make this work properly is to have the set and the ItemFlags defined in the same .cpp file. Then the order is always top-to-bottom. If they are in different files, the linker decides the order (mostly at random).

answered May 09 '11 at 14:06

Bo Persson

90,663
31
146
203

Changing the name of the file, or using a std::vector works, which made me think it was something peculiar about a set, but lazy initialization of the set works regardless. Unfortunately the itemflags are inherited and used throughout the program and having them both in the same file would not be suitable due to the recompilation overhead. – user745247 May 09 '11 at 14:24
@user745247 - Initialization of globals is performed in several phases, the first of which is zeroing all the memory. If that is all the default constructor of std::vector needs, that might work. If it happens to do something that requires a dynamic initialization, like std::set seems to do, you would have the order problem again. There are no guarantees in the standard, so it would be implementation specific, aka luck. – Bo Persson May 09 '11 at 15:00

std::set used as a static templated member variable

3 Answers3