hercules-team / augeas

A configuration editing tool and API
http://augeas.net/
GNU Lesser General Public License v2.1
486 stars 199 forks source link

Implicit function delcaration error with Apple Clang #817

Closed p-linnane closed 1 year ago

p-linnane commented 1 year ago

Hello 👋 . I'm a maintainer for the Homebrew project. We've noticed the following error with Apple Clang when trying to compile augeas. We are currently working around this by passing -Wno-implicit-function-declaration, but hoping to get an upstream fix. Thanks!

lens.c:979:9: error: call to undeclared function 'dump_lens_tree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        dump_lens_tree(lens);
        ^
1 error generated.
igalic commented 1 year ago

this code is a bit weird. both dump_lens_tree and the debugging function guarding its call are gated by # if ENABLE_DEBUG. but unlike debugging, there's no # else branch declaring a (useless) dump_lens_tree version to appease the compiler.

so, despite the fact that the compiler sees this:

if ((0))
      dump_lens_tree(lens);

it doesn't just delete the code from the AST, it tries to actually analyse it.

A possibly fix would be to declare the two dump functions regardless of whether Debugging is enabled. Ideally we could declare them static, so as to not pollute the exported functions

georgehansper commented 1 year ago

Thanks for pointing this out @igalic

The function dump_lens_tree() is defined within an #if ENABLE_DEBUG ... #endif clause, as is the corresponding function declaration in the header file lens.h

I was wondering why the compiler did not complain that the function didn't exist at all after complaining that it was an "undeclared function".

I expect it is because the of the if(0), which allows the compiler to optimise it out of existance in the code.

The "undeclared function" warning probably happens on a eariler pass of the compiler over the code, before it does the optimisation

PR#818 puts the code inside an #if ENABLE_DEBUG clause.

As an alternative, the lens.h could be updated along the lines of debugging() in internal.h, like this:

#if ENABLE_DEBUG
void dump_lens_tree(struct lens *lens);
void dump_lens(FILE *out, struct lens *lens);
#else
#define dump_lens_tree(lens) (void)0
#endif

I'm thinking that this is closer to the existing solution applied for the function debugging() which is in a similar situation

The net result is that when ENABLE_DEBUG is false, to compiler sees the dummy macros in place of the functions and stops complaining about them being "undeclared functions"

p-linnane commented 1 year ago

Confirming that https://github.com/hercules-team/augeas/pull/818 fixes the issue.

georgehansper commented 1 year ago

Thanks @p-linnane for confirming this

I have updated the Pull Request #818 to use #define dump_lens_tree as I mentioned above, instead of the previous solution

The end-result is the same (the compiler warning is gone), but it "fits" better with the existing code and is slightly more readable

Sorry for any inconvenience.