undefined behaviour in integer_storage and double_storage

[wojdyr]

After reading your blog post about sajson optimizations (it was a really interesting read) I started browsing the code. I noticed type-punning through a union:

    union integer_storage {
        enum {
            word_length = 1
        };

        static void store(size_t* location, int value) {
            integer_storage is;
            is.i = value;
            *location = is.u;
        }

        int i;
        size_t u;
    };

From what I know from reading discussions on SO this is an undefined behavior in C++ (although a common practice). Just to let you know.

(it's not a big deal; I think I'll start using this library soon anyway)

[chadaustin]

Wow, it seems you're right. For some reason I thought this was made okay in C++11.

I believe I've read MSVC has said they will continue to support type punning through unions in VC++.

Also, Owen Shepherd here ( https://news.ycombinator.com/item?id=9129404 ) says it's implementation-defined (and okay), not UB... but is he talking about C or C++ there?

I played around with godbolt and found in-use compilers that don't do the constant-sized memcpy load and store optimization, even though on newer compilers it generates even better code than the union implementation... I'm a bit nervous about changing it, even though it shouldn't matter that much. What do you think?

[wojdyr]

yes, I suppose it's safe to leave it as is

[chadaustin]

Went ahead and fixed it anyway :) https://github.com/chadaustin/sajson/commit/fd7ad76cbaa131ad77a0c08deb3eec06695daa03

[wojdyr]

Great!

My warning options may be too pedantic :-)

../third_party/sajson.h:316:6: warning: extra ‘;’ [-Wpedantic]
     };
      ^
../third_party/sajson.h:335:6: warning: extra ‘;’ [-Wpedantic]
     };

[chadaustin]

:) https://github.com/chadaustin/sajson/commit/124950ba4c82f5d42461e7acfbff0eecd44ad3ba