tau icon indicating copy to clipboard operation
tau copied to clipboard

defaults and changing them for compiler errors

Open matu3ba opened this issue 2 years ago • 8 comments

I prefer developing in a CI and thus I think disabling warnings as in TAU_DISABLE_DEBUG_WARNINGS is very uncool.

Aside, what warnings and errors are ignored should be the one of the first things in the README and documentation.

Ideally, a testing tool also allows simple integration of valgrind or better document how the user should invoke tests. You dont mention in the README, how tests should be run (as convention).

matu3ba avatar Dec 30 '21 18:12 matu3ba

I don't like disabling warnings, but at the time of writing the codebase, I had multiple things going on and didn't have time to nitpick at each warning/error. If you're open to doing this, by all means!

jasmcaus avatar Jan 09 '22 16:01 jasmcaus

I think adding an option to enable all warnings, which means disable the disabling of warnings/errors should be sufficient. I will call this TAU_DONT_DISABLE_WARNINGS or we can discuss naming in the PR.

matu3ba avatar Mar 27 '22 19:03 matu3ba

Waiting for #24 to prevent merge conflicts.

matu3ba avatar Mar 27 '22 19:03 matu3ba

If any warnings need to be fixed, they need to be fixed here on the original repo. If a user that uses Tau to test their project, and accidentally uses this macro to disable certain warnings, then Tau's warnings will appear along with the user's code. I'd really look to minimizing the amount of warnings we have currently (I know there are quite a bit) in the codebase.

jasmcaus avatar Mar 28 '22 04:03 jasmcaus

I'll look what codepaths I can test with a toy project. I have everything except -Wformat and -Wcast-align cleaned up from -Weverything from clang, so this should be nice for testing the test framework.

matu3ba avatar Apr 11 '22 00:04 matu3ba

In my toy project on this branch https://github.com/matu3ba/pbcd/tree/9dc0f8ae9c6463a38230610ede42095e195fb6f9 I get these warnings with clang -std=c17 -Weverything:

./runTest.sh 
In file included from test.c:2:
./helper.c:60:20: warning: cast from 'char *' to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Wcast-align]
  hd->ptr_prefix = (uint32_t*)(hd->src);
                   ^~~~~~~~~~~~~~~~~~~~
./helper.c:69:20: warning: cast from 'char *' to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Wcast-align]
  hd->ptr_prefix = (uint32_t*)(hd->dst);
                   ^~~~~~~~~~~~~~~~~~~~
./helper.c:7:9: warning: padding struct 'struct HelperData' with 4 bytes to align 'src' [-Wpadded]
  char* src;
        ^
In file included from test.c:3:
tau/tau/tau.h:104:32: warning: this function declaration is not a prototype [-Wstrict-prototypes]
typedef void (*tau_testsuite_t)();
                               ^
                                void
tau/tau/tau.h:158:36: warning: this function declaration is not a prototype [-Wstrict-prototypes]
static void __failIfInsideTestSuite();
                                   ^
                                    void
tau/tau/tau.h:158:13: warning: identifier '__failIfInsideTestSuite' is reserved because it starts with '__' [-Wreserved-identifier]
static void __failIfInsideTestSuite();
            ^
tau/tau/tau.h:159:37: warning: this function declaration is not a prototype [-Wstrict-prototypes]
static void __abortIfInsideTestSuite();
                                    ^
                                     void
tau/tau/tau.h:159:13: warning: identifier '__abortIfInsideTestSuite' is reserved because it starts with '__' [-Wreserved-identifier]
static void __abortIfInsideTestSuite();
            ^
tau/tau/tau.h:252:29: warning: cast from function call of type 'clock_t' (aka 'long') to non-matching type 'double' [-Wbad-function-cast]
    return TAU_CAST(double, clock()) * 1000000000 / CLOCKS_PER_SEC; // in nanoseconds
                            ^~~~~~~
tau/tau/misc.h:22:44: note: expanded from macro 'TAU_CAST'
    #define TAU_CAST(type, x)       ((type)x)
                                           ^
test.c:9:1: warning: identifier '_TAU_TEST_FUNC_foo_bar1' is reserved because it starts with '_' followed by a capital letter [-Wreserved-identifier]
TEST(foo, bar1)
^
tau/tau/tau.h:840:17: note: expanded from macro 'TEST'
    static void _TAU_TEST_FUNC_##TESTSUITE##_##TESTNAME(void);                                 \
                ^
<scratch space>:38:1: note: expanded from here
_TAU_TEST_FUNC_foo_bar1
^
test.c:29:3: warning: comparison between pointer and integer ('char *' and 'int') [-Wpointer-integer-compare]
  CHECK_EQ(hd.dst, 0xabc0); // TODO docs: how do we compare pointer addresses?
  ^~~~~~~~~~~~~~~~~~~~~~~~
tau/tau/tau.h:720:70: note: expanded from macro 'CHECK_EQ'
#define CHECK_EQ(actual, expected)      __TAUCMP__(actual, expected, ==, "", CHECK_EQ, TAU_FAIL_IF_INSIDE_TESTSUITE)
                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tau/tau/tau.h:558:26: note: expanded from macro '__TAUCMP__'
            if(!((actual)cond(expected))) {                                                    \
                 ~~~~~~~~^   ~~~~~~~~~~
11 warnings generated.
pbcd.c:31:32: warning: cast from 'char *' to 'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Wcast-align]
    const uint32_t src_len = *((uint32_t*)(src));
                               ^~~~~~~~~~~~~~~~
1 warning generated.

Sidenode: How can I compare pointer addresses, ie to test pointer arithmetic? CHECK_EQ(charptr, 0x123) checks 0x123 as content of a backing string.

matu3ba avatar Apr 11 '22 08:04 matu3ba

The -Wstrict-prototypes warning can be fixed easily by populating empty function declarations func() with void as: func(void). You are welcome to work on this.

As for comparing pointer addresses, I suspect it's not a very common thing + my knowledge in C doesn't expand that much into this, so unless there's a reasonable argument/user traction, I think it's trivial.

jasmcaus avatar Apr 11 '22 13:04 jasmcaus