Crude check for long long for when configure isn't used

[basvodde]

No description provided.

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 9efe82e on basvodde:long-long into bcabc12 on cpputest:master.

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 962535a on basvodde:long-long into bcabc12 on cpputest:master.

[uecasm]

Have you tried just -Wno-long-long? That's what the configure/cmake builds should be using.

[basvodde]

I can turn off more warnings, but it doesn't feel right. And the Watcom error bothers me. Not sure yet how to proceed.

[uecasm]

You shouldn't turn off something generic like -Wc++98-compat-pedantic, but anything long-long-specific should be relatively safe. But the order of warning parameters can also be important -- enabling some of the generic compatibility warnings will turn -Wlong-long back on unless explicitly disabled.

Regarding the Watcom error, it looks like the tests are being built with a different value of CPPUTEST_USE_LONG_LONG than the library was. It's probably something to do with the config.h changes, though I didn't look too closely. (See also my other PR #942.)

[basvodde]

The -Wno-long-long is already on.

[basvodde]

Ah yeah, lemme see if the order helps

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 20715da on basvodde:long-long into bcabc12 on cpputest:master.

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 22d4056 on basvodde:long-long into bcabc12 on cpputest:master.

[basvodde]

@uecasm Did turn off -Wc++98-compat-pedantic as it causes warning when compiling with C++11, which is kinda odd. Now it is off when compiling with C++11, which is what you would want.

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 645171e on basvodde:long-long into bcabc12 on cpputest:master.

[uecasm]

Fair enough -- I missed that this occurred in a C++11 build. It's reasonable to disable C++98 warnings in that case.

[basvodde]

Can't yet figure out the last 2 failures though. Can't reproduce the gcc one, it feels very odd....

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 0ea8e7a on basvodde:long-long into bcabc12 on cpputest:master.

[arstrube]

@basvodde should this be working with Wcl? Or do we need to disable long long manually?

[basvodde]

@arstrube Both the Watcom and the one GCC failures are a complete mystery to me :)

[arstrube]

Well, I can's say anything about the Gcc failure.

As for Wcl, a possible explanation could be that cpputest_long_long is defined differently in the library than it is in the code that's trying to use it?

[basvodde]

It compiled the library itself, so how and why would that be so?

[arstrube]

opt-in / opt-out mixup?

[arstrube]

I use a monolithic compile for Wcl locally, which compiles fine.

[arstrube]

Looking at the code, I can't see any good reason :-/

[arstrube]

Ironically, it seems that Wcl actually does support long long (on a 16 bit platform ;-). I still can't see how this could cause a linker error.

[arstrube]

Perhaps 'monolithic' is a bit misleading. It's still six separate executables, but without the use of libraries

[arstrube]

Another puzzlement is, that none of the tests in the executable that fails to link is supposed to call the long long assertion methods anyway.

[arstrube]

I took the liberty of just deleting a few tests, after which it compiled and ran fine, long long tests inclusive.

[arstrube]

Hmm. I managed to tweak platform.mk to suit my new version of Cygwin, and - everything compiles just fine. So this happens only cross-compiling with Wcl under Linux, but not cross-compiling with Wcl under Windows.

[arstrube]

Ooops. Of course it works - was testing master, not your branch

[basvodde]

Both the watcom and the gcc failures are a big puzzle to me. I can't reproduce the gcc failure locally on my mac and will switch to linux to see if I can do it there. The watcom just seems really weird. I don't know whether it detected long long support or not, I guess I'll need to install Watcom locally :)

[arstrube]

@basvodde okay, I can reproduce it now. As expected, Wcl has LLONG_MAX defined, because it does support longlong.

So how do I sniff out the problem???

[basvodde]

Why would it then only lead to a linker error?

[arstrube]

1. Adding a brute-force -dCPPUTEST_USE_LONG_LONG=1 via Makefile will make the problem go away
2. Once it is gone, we run into the 64k per segment (e.g. source file) limit. That means, we need to split up TestUtestMacro.cpp somehow.

[arstrube]

Why would it then only lead to a linker error

For some reason, CPPUTEST_USE_LONG_LONG is not defined everywhere.

[basvodde]

1. doesn't make sense, when it does define LLONG_MAX and then uses CPPUTEST_USE_LONG_LONG, that should be the same. I think needs more research :)

[arstrube]

How do we know that CppUTestConfig.h is included everywhere?

[arstrube]

Oh, by the way, I put an #error into CppUTestConfig.h which confirmed, that when run with Wcl, LLONG_MAX is indeed present and CPPUTEST_USE_LONG_LONG is therefore defined.

[arstrube]

The culprit has been identified as AllocLetTestFreeTest.cpp.

It does:

#include "CppUTest/StandardCLibrary.h"

extern "C"
{
#include "AllocLetTestFree.h"
}

#include "CppUTest/TestHarness.h"

Ring any bells?

[arstrube]

Moving CppUTest/TestHarness.h BEFORE CppUTest/StandardCLibrary.h makes the problem go away.

[arstrube]

If I add this code into AllocLetTestFreeTest.cpp:

#ifdef CPPUTEST_USE_LONG_LONG
#error CPPUTEST_USE_LONG_LONG is defined!
#else
#error CPPUTEST_USE_LONG_LONG is lacking!!!
#endif // CPPUTEST_USE_LONG_LONG

Then, if Testharness.h comes first, CPPUTEST_USE_LONG_LONG is defined. If StandardCLibrary.h comes first, it is lacking. Go figure.....

[arstrube]

With Gcc, it is defined in either order :-/

[basvodde]

That actually makes sense, thanks!

[basvodde]

Ok, this brought us a little closer to getting the Watcom build to work, but the gcc build is still a puzzle :)

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling 686544a on basvodde:long-long into bcabc12 on cpputest:master.

[arstrube]

Simply doing:

#include "CppUTest/TestHarness.h" // instead of #include "CppUTest/StandardCLibrary.h"

extern "C"
{
#include "AllocLetTestFree.h"
}

like everywhere else would have worked, too, wouldn't it?

[arstrube]

That's a bit of an odd thing to do, the include blocker not being the first line, don't you think?

[basvodde]

It makes sense though, and it fixes the problem. The reason is that StandardCLibrary is included in CppUTestConfig halfway. It must be included at that point. Having the CppUTestConfig before the include guard would prevent StandardCLibrary from being included, so it must be actually outside of the include guide.

That way the problem is served permanently.

[arstrube]

I would prefer #including TestHarness.h as indicated above. That way, you don't need to patch StandardCLibrary.h in this unusual way.

Including StandardCLibrary.h directly anywhere is non-standard and, in my opinion, shouldn't be done. So, even if you wish to keep this patch (for added safety) I vote to include Testharness.h as indicated above.

[uecasm]

Perhaps StandardCLibrary.h should #error if not included from (or at least after) CppUTestConfig.h? Then the test could be changed to include TestHarness.h as @arstrube suggests and we'd still have a clear indication to tell people not to include StandardCLibrary.h directly.

[arstrube]

Yes, that seems much better to me.

[basvodde]

@arstrube The current solution solves the problem and I cannot imagine how it can lead to a problem. All other proposals do not solve the real problem. So, unless there is a real solution, I won't be changing it.

[uecasm]

Clearly you feel differently about it, but I think that "wanting to include StandardCLibrary.h" is the real problem. 😥 (It's an implementation detail, not a public header.)

[basvodde]

Uhm, yeah, do feel different about it. Especially in CppUtest tests, including StandardCLibrary avoids a direct dependency with StandardC library, which is great :)

[uecasm]

Right, but including CppUTestConfig.h does the exact same thing, is more high-level, and would avoid the recursive dependency.

[basvodde]

True, but it feels ugly to include CppUTestConfig when I actually want StandardC :)

[arstrube]

Ummm. Regarding Gcc. The old Makefile seems to need -Wno-long-long (see error text).

[basvodde]

@arstrube It shouldn't need no-long-long at all, don't use that in autotools either, which is why it is so strange.

[uecasm]

Autotools does use -Wno-long-long -- it was in my original patch. See configure.ac.

[arstrube]

The -Wno-long-long for Automake was changed to -Wno-c++11-long-long in this patch for some reason (why?). I don't believe the old Makefile has C++11 enabled, so it would make sense to me that it wants -Wno-long-long. (I was wondering why the Autotools c++11 one doesn't blow up, because as far as I know the Autotools build does many builds without c++11 and only one with. That's what I don't understand...)

[arstrube]

Autotools make tdd throws a lot of warnings on my build with this patch, which go away when I replace -Wno-c++11-long-long by -Wno-long-long.

[arstrube]

@basvodde - what was the reason you changed to -Wno-c++11-long-long?

[coveralls]

Coverage decreased (-0.0001%) to 99.909% when pulling b3470e7 on basvodde:long-long into bcabc12 on cpputest:master.