[Minor Issue][Warnings] Clang-3.5

[valvesoftware]

[Minor Issue][Warnings] Clang-3.5

[mikesart]

Ah, this is cool. Thanks for running vogl through this.

So key.get_ptr() should return a "const char *" pointer:

vogl_json.inl:
    inline bool add_key_and_parsed_value(const dynamic_string &key, const char *pValueToParse)
    {
        return add_key_and_parsed_value(key.get_ptr(), pValueToParse);
    }

Which should call the first function here:

vogl_json.h:
        bool add_key_and_parsed_value(const char *pKey, const char *pValueToParse);
        bool add_key_and_parsed_value(const dynamic_string &key, const char *pValueToParse);

And that function which takes the "const char *" is defined in vogl_json.cpp.

Seems like it is calling the dynamic_string(const char *p) ctor and then recursing though? Hm.

[kingtaurus]

I believe that this warning might be spurious.

(a) key.get_ptr() calls the following function dynamic_string::get_ptr_priv();; This function is declared inline which returns char*.

So, if the compiler inlines (both functions) and discards the return qualification of const (which shouldn't be allowed) - I believe that the compiler would than have to deal overload resolution matching both functions.

(1) Few other points: a large number of the VOGL_ASSERT involving checking size could be replaced with static asserts (C++11 or boost::static_assert).

(2) src/voglcore/vogl_math.h

• A large number of the functions are already defined in <algorithm>
• A number of the rounding functions could be changed to <cmath> calls (although the new calls available in C++11 are nicer). In addition there are overflow issues being avoided.
• The constants declared here could be replaced with constants in <cmath> (M_E, M_PI, ...)
• vogl_math.cpp is_prime could be replaced with quick lookup using sieve of Eratosthenes (constant lookup) or just create a prime list less than sqrt(max_int); This should reduce the required number of tests to being log(N).

(3) A number of the old style casts can probably replaced directly by static_cast<type>(...) or by type(...) if the appropriate constructor exists.

[mikesart]

I just removed the dynamic_string version of add_key_and_parsed_value. Neither function is used right now and that will kill this warning. I'll gnaw my little toes off before using boost, but switching some of the VOGL_ASSERT stuff over to static_asserts and cleaning up some of the other math stuff, etc. is a good suggestion at some point I think. I'll look at it after these higher priority tasks unless someone else gets to it first... Thanks Gregory.

[kingtaurus]

When I get some time, I'll give a bit of work towards getting a few of these small issues cleaned up.

I've always found most boost libraries to be fairly innocuous (although there are some meta-programming libraries are a little difficult to debug). If you are opposed to using boost libraries or C++11, I could probably use the static check features associated with loki.

I will keep compiling with gcc (4.8.2 as ships with ubuntu) and clang (3.5 from the llvm repos).

[mikesart]

One thing I have on my todo list is to check whether we're going to run into issues pulling the C++ runtime libraries into the processes. If this is potentially a problem, we'll need to look at statically linking with them. If you or anyone has any experience in this area, I'd love to hear it.

The problems I've run into with boost have been compile times and it can get pretty complex. At times the compiler error messages were inscrutable also. Granted I haven't used it for the past 3+ years, so my experiences may be out of date. I think we're all fine with C++11 though. Also more than willing to have a discussion about boost and be proven wrong with some examples, etc. of course.

Thanks much.