-
-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
defaults and changing them for compiler errors #15
Comments
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! |
I think adding an option to enable all warnings, which means disable the disabling of warnings/errors should be sufficient. |
Waiting for #24 to prevent merge conflicts. |
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. |
I'll look what codepaths I can test with a toy project. I have everything except |
In my toy project on this branch https://github.com/matu3ba/pbcd/tree/9dc0f8ae9c6463a38230610ede42095e195fb6f9 I get these warnings with ./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? |
The 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. |
Casting pointers is a very evil thing to do, because accessing them can be UB: https://stackoverflow.com/questions/4810417/when-is-casting-between-pointer-types-not-undefined-behavior-in-c In short: Pointers are a huge footgun in C standard.
|
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).
The text was updated successfully, but these errors were encountered: