alrevuelta / cONNXr

Pure C ONNX runtime with zero dependancies for embedded devices
MIT License
193 stars 31 forks source link

Tracing not compatible with C99 #81

Closed alrevuelta closed 3 years ago

alrevuelta commented 3 years ago

Tracing is not compatible with C99. On the other hand, as already discussed in some PR with @nopeslide I think we should also simplify tracing. We have lots of functions and macros, making it complex to use and enforce its correct use.

nopeslide commented 3 years ago

Tracing uses a feature which is NOT C99 so I can't test any changes (any definition of trace level won't compile).

Originally posted by @Coderitter-GmbH in https://github.com/alrevuelta/cONNXr/issues/80#issuecomment-756782633

could you be more specific than that? all files are compiled with -std=c99, no standard errors/warning are thrown. do you refer to some gcc extension?

Coderitter-GmbH commented 3 years ago

I am not an expert in macros, but https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html states, that __VA_OPT__ got introduced with C++2a. If you use an older version of gcc, it wont compile with the -std=c99 flag. With my tool chain for cross compiling to riscv i am stuck at gcc version 7.1.1

Edit: I just read it is indeed a standard of c99, but I use -std=c99 and it wont compile.

../../include/tracing.h:410:30: error: expected ')' before '__VA_OPT__'
 TRACE_SYMBOL_FPRINTF(FD, FMT __VA_OPT__(,) __VA_ARGS__);
                              ^
../../include/tracing.h:558:5: note: in expansion of macro '__PRINT'
     __PRINT(TRACE_SYMBOL_STDOUT, "exiting function %s\n",__FUNCTION__) \
     ^~~~~~~
../../include/tracing.h:64:1: note: in expansion of macro '_TRACE_EXIT'
 _TRACE_EXIT(LEVEL);
 ^~~~~~~~~~~
../../src/utils.c:410:3: note: in expansion of macro 'TRACE_EXIT'
   TRACE_EXIT(1);
   ^~~~~~~~~~
nopeslide commented 3 years ago

I am not an expert in macros, but https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html states, that __VA_OPT__ got introduced with C++2a. If you use an older version of gcc, it wont compile with the -std=c99 flag. With my tool chain for cross compiling to riscv i am stuck at gcc version 7.1.1

Edit: I just read it is indeed a standard of c99, but I use -std=c99 and it wont compile.

There is a historic alternative for commas via the concat operator ##, which has special meaning if used in combination with __VA_ARGS__. Could you try if your cpp/gcc can run this? If so, we can propably switch

#define TEST(X, ...) X,##__VA_ARGS__ 

TEST(1,2,3,4)

expected output would be 1,2,3,4

Coderitter-GmbH commented 3 years ago
#include <stdio.h>
#include <stdlib.h>
#define LOG(msg, ...) printf(msg, ##__VA_ARGS__)
int main(void) {
  LOG("test %d\n", 5);
  LOG("test\n");
  return EXIT_SUCCESS;
}

This works! It looks like older gcc can't handle __VA_OPT__ correctly. But if it is okay to use ## instead __VA_OPT__ I could provide a pull request.