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

cmake: define HAVE_VARIADIC_MACROS_C99 #3452

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@bagder
Copy link
Member

bagder commented Jan 10, 2019

This should ideally check what variadic macros that actually
work. This is mostlu to improve the current situation where no define at
all is set for this.

Fixes #3142

cmake: define HAVE_VARIADIC_MACROS_C99
This should ideally *check* what variadic macros that actually
work. This is mostlu to improve the current situation where no define at
all is set for this.

Fixes #3142

@bagder bagder added the cmake label Jan 10, 2019

@bagder bagder requested a review from snikulov Jan 10, 2019

@gvanem

This comment has been minimized.

Copy link
Member

gvanem commented Jan 10, 2019

So can the the IMHO horrid DEBUGF() macro be changed in the same way?

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Jan 10, 2019

It probably could, yes. The only downside with using that approach is that it isn't C89 compatible. But since debug-enabled builds aren't the default I think its fine. In general C89 strictness is growing less and less important too.

@snikulov

This comment has been minimized.

Copy link
Member

snikulov commented Jan 10, 2019

@bagder l'll check it soon.

@snikulov

This comment has been minimized.

Copy link
Member

snikulov commented Jan 10, 2019

@bagder BTW, we could use CMAKE_C_COMPILE_FEATURES to check what feature is available.

CMAKE_C_COMPILE_FEATURES="c_std_90;c_function_prototypes;c_std_99;c_restrict;c_variadic_macros;c_std_11;c_static_assert"

But to use this, we should increase requirements to cmake v3.1 minimum.

What do you think?

@bradking is it possible to check for C compiler features on cmake less than 3.1?

@bradking

This comment has been minimized.

Copy link
Contributor

bradking commented Jan 10, 2019

Anything can be checked with an explicit try_compile, e.g. via CheckCSourceCompiles.

CMAKE_C_COMPILE_FEATURES is not meant for project code to check. It is an implementation detail of the target_compile_features command.

@snikulov

This comment has been minimized.

Copy link
Member

snikulov commented Jan 10, 2019

@bradking But is it a good idea to make this check, if it is already performed?
Or CMAKE_C_COMPILE_FEATURES does not check anything?

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Jan 10, 2019

we could use CMAKE_C_COMPILE_FEATURES to check what feature is available

The bug (#3142) has been around since October which made me finally get my act together and do something. This is my way to make the situation slightly better than before - as I don't have the knowledge on how to make the correct and full fix.

I would certainly welcome a more complete fix but I'm not the one to it.

@snikulov

This comment has been minimized.

Copy link
Member

snikulov commented Jan 11, 2019

I've proposed #3459 for this. Could you please review?

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Jan 11, 2019

Let #3459 replace this.

@bagder bagder closed this Jan 11, 2019

@bagder bagder deleted the bagder/cmake-variadic-macros branch Jan 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment