Compile with -Werror

[kraiskil]

This is just a quick "get to know cONNXr"-commit. No functional changes.

Use CFLAGS when compiling connxr & runtest.
Fix compiler warnings: unused variables and print format errors.

[nopeslide]

CI compiles with -Werror, having it always enabled is imho cumbersome.
We are currently fleshing out the base architecture.
Just ask if you have any questions :)

[nopeslide]

CI compiles with -Werror, having it always enabled is imho cumbersome.

Just noticed the -Wall does not stick since we don't set it in the CI script.

[kraiskil]

Ok - the -Werror is a preference I guess :)

One change I made was to compile the connxr & runtest binaries with CFLAGS passed to the compiler. So before 49db5ee, connxr and runtest were not built with -Wall (or -std=c99). This might be why the build passed with -Werror in the CI? Was this intentional?
Then again, I did see warnings on my local build even before 49db5ee (which is why I made these patches to begin with), but that might just be my local gcc settings. Confusing.

I'm not sure how to proceed now.

[nopeslide]

Ok - the -Werror is a preference I guess :)

if you just want to test stuff or play around it gets in the way. so yeah, preferences :D

One change I made was to compile the connxr & runtest binaries with CFLAGS passed to the compiler. So before 49db5ee, connxr and runtest were not built with -Wall (or -std=c99). This might be why the build passed with -Werror in the CI? Was this intentional?
Then again, I did see warnings on my local build even before 49db5ee (which is why I made these patches to begin with), but that might just be my local gcc settings. Confusing.

I stand corrected, CI applies -Werror -std=c99 -Wall -g3 -gdwarf while compiling, but not while linking.
I get no warning when compiling the current master, do you have some options set via env?

CI only sets CFLAGS env to -Werror and runs make all.
the other options are added by the Makefile

[kraiskil]

What I originally saw was:
include/test/models/common_models.h:72:10: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Wformat=] printf("[benchmark][%s] CLOCKS_PER_SEC: %d\n", model_id, CLOCKS_PER_SEC);
And only this. (Ubuntu 16.04 x86_64, no local modifications to env or gcc).
When I then started poking around and passed CFLAGS to the compilation/link step of runtest & connxr, the other warnings that I fixed started showing up too.

So if I understood correctly: now only removing -Werror from Makefile, CI should still both compile & link with -Wall -Werror set.

[nopeslide]

What I originally saw was:
include/test/models/common_models.h:72:10: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Wformat=] printf("[benchmark][%s] CLOCKS_PER_SEC: %d\n", model_id, CLOCKS_PER_SEC);
And only this. (Ubuntu 16.04 x86_64, no local modifications to env or gcc).
When I then started poking around and passed CFLAGS to the compilation/link step of runtest & connxr, the other warnings that I fixed started showing up too.

I think this happend because we left out the $(CFLAGS) when compiling the last file for the binaries. Nice catch :D

So if I understood correctly: now only removing -Werror from Makefile, CI should still both compile & link with -Wall -Werror set.

correct, both are applied now while compiling & linking, I just checked the CI logs

[nopeslide]

Could you squash your last commit 1ad1c6b with your first?

[kraiskil]

Not all of them into one? Squashing just the last to the first will cause the build of that new ref to fail to compile on mac & windows. Someone doing git bisect down the line will curse me :) And the changes are still pretty related.

[nopeslide]

you are completely right :)
on this note, we may have to rebase the whole repo to allow for a clean bisect ;D

[nopeslide]

@kraiskil
and a new issues was born #52
Are you profound enough with github actions to tackle this? I would have to do a lot of reading and overcome my hate for yaml

[kraiskil]

That would be a good policy there in #52. I haven't used github actions yet - but learning them have been on my TODO list. Maybe next rainy day :)

[nopeslide]

Maybe next rainy day :)

just checking the weather :D