GNOME Bugzilla – Bug 793444
A Code attribute for #defines
Last modified: 2018-02-21 06:44:06 UTC
We should introduce a CCode attribute that allows adding #define to the generated C code. This could improve several of our low-level bindings, such as the non-posix file control constants (O_DIRECT, O_NOATIME, O_PATH, O_TMPFILE) in linux.vapi, or the signal handler (sighandler_t) in posix.vapi – both of which only become visible, once _GNU_SOURCE is defined (see also bug 761975). Would something like a "cdefine" CCode attribute be useful? Such as: [CCode (cheader_filename = "fcntl.h", cdefine=_GNU_SOURCE)] public const int O_DIRECT;
A very good idea. I think the CCode detail should be "feature_test_macro". This is a commonly used phrase and makes the definition and usage searchable from search engines for anyone who isn't sure what a feature test macro is. Example documentation from GNU's libc and the Linux man page using the phrase: http://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html http://man7.org/linux/man-pages/man7/feature_test_macros.7.html So it would be: [CCode (cheader_filename = "fcntl.h", feature_test_macro="_GNU_SOURCE")] public const int O_DIRECT; "feature_test_macro" is more precise than cdefine. The macro needs to be placed before the #include directives and not have a value.
Those are simple defines not macros or the like. Of course this attribute would only be used if valac is suppose to invoke "cc" by itself. I would call it [CCode (cflags = "-D_GNU_SOURCE")]
(In reply to Rico Tzschichholz from comment #2) > Those are simple defines not macros or the like. They are, however, commonly referred to as feature test macros as the links in my comment show. Looking at the Linux man page some of them can have values, e.g. _POSIX_C_SOURCE >= 200112L > Of course this attribute would only be used if valac is suppose to invoke > "cc" by itself. There are two ways of doing this, one is invoking them with the -D option to the C compiler, the other is to have them in the C source code. The GNU libc manual recommends "You could also use the ‘-D’ option to GCC, but it’s better if you make the source files indicate their own meaning in a self-contained way." These feature test macros are to be placed in the C code itself and before any #include directives. The GNU libc manual advises "You should define these macros by using ‘#define’ preprocessor directives at the top of your source code files. These directives must come before any #include of a system header file. It is best to make them the very first thing in the file, preceded only by comments." > I would call it [CCode (cflags = "-D_GNU_SOURCE")] To my mind the purpose of this C Code detail is to avoid passing compiler flags. This saves the programmer from digging them out of the documentation. Documentation that is often hard to find a definitive answer for those unfamiliar with the subject.
While my initial idea was more along the lines of Al, making the C files compile without inducing more burden on the build system, I can understand that this might be too invasive. Having `cflags` instead would be a compromise I could live with, at least for a) the documentation aspect, since we could then make sure they find their way into valadoc, and b) work in the case of 'valac' or 'vala', where _we_ are actually the buildsystem. I'm fine with either way, although still slightly more in favor of my Al's proposal. Whose call is it to finally decide on such an issue?
Hmm, ok, so this actually needs to go in the generated c-source then.
I'll try to come up with a patch.
Created attachment 368653 [details] [review] Implement feature test macro CCode attributes
Created attachment 368654 [details] [review] linux.vapi: Add feature test macros
I have picked up your patches which look good. Additionally I would like to a have testcase which would be conditionally executed by the build-sys on linux.
(In reply to Rico Tzschichholz from comment #9) > Additionally I would like to a have testcase which would be conditionally > executed by the build-sys on linux. This could be done cross-platform with a C header file containing the test and appropriate VAPI file and Vala source file. The test symbol would only be available to the compiler Vala binary if the feature test macro was present. In the longer term it would be easier to just have the Vala source implement [CCode (cheader_filename="test.h")] so the cheader_filename detail is respected for .vala as well as .vapi files. At present I don't think this would work in a .vala file: [CCode (cheader_filename = "test.h", feature_test_macro = "_VALA_TEST")] public extern const int TEST_VALUE; void main () { assert (TEST_VALUE == 42); }
Created attachment 368675 [details] [review] Fix bookkeeping Please add this patch on top, it fixes an oversight in the bookkeeping. I agree with Al with regards to a more elegant way for testing such feature, but will submit a "classical" test asap.
Created attachment 368676 [details] [review] Add Linux-specific test for feature macros Were you thinking about something like that? (Note: untested, since I don't have access to a Linux machine atm.)
Created attachment 368682 [details] [review] tests: Add linux-specific test for feature test macros
Comment on attachment 368682 [details] [review] tests: Add linux-specific test for feature test macros Looking good, thanks!
commit fcda327ebfc946de971ce7fd9c2715b6d28b518b Author: Dr. Michael Lauer <mickey@vanille-media.de> Date: Tue Feb 20 16:47:34 2018 +0100 codegen: Add support for feature test macros commit 80dcd7dd205a977c2570d5966cf1993416278d85 Author: Dr. Michael Lauer <mickey@vanille-media.de> Date: Tue Feb 20 16:51:11 2018 +0100 linux: Add feature_test_macros to non-POSIX file control constants and dup3 commit f67582423a132e6c79b0b87c024bd6a8e370f2ad (HEAD -> master, origin/staging, origin/master, origin/HEAD, staging) Author: Rico Tzschichholz <ricotz@ubuntu.com> Date: Tue Feb 20 22:50:24 2018 +0100 tests: Add linux-specific test for feature test macros