Fail to use libpkgconf in C++20 code #154

New Issue

ariadne · 2017-11-06T22:06:53Z

karen-arutyunov commented

2017-11-06 22:06:53 +00:00

(Migrated from github.com)

Hello,

The pkgconf_pkg_ struct that is defined in libpkgconf/libpkgconf.h has the 'requires' member. The problem is that 'requires' is a keyword in C++20, so compiling the C++20 code (for example with clang trunk) that includes libpkgconf/libpkgconf.h header fails.

To fix the issue we have renamed the 'requires' member to 'requires_'.

I have attached the patch in case you find it useful for the issue fix. Note that it also covers main.c file.

Thanks,
Karen
pkgconf.patch.gz

Hello, The pkgconf_pkg_ struct that is defined in libpkgconf/libpkgconf.h has the 'requires' member. The problem is that 'requires' is a keyword in C++20, so compiling the C++20 code (for example with clang trunk) that includes libpkgconf/libpkgconf.h header fails. To fix the issue we have renamed the 'requires' member to 'requires_'. I have attached the patch in case you find it useful for the issue fix. Note that it also covers main.c file. Thanks, Karen [pkgconf.patch.gz](https://github.com/pkgconf/pkgconf/files/1447916/pkgconf.patch.gz)

ariadne commented

2017-11-13 21:29:04 +00:00

pkgconf should be built as C99, not C++.

ariadne commented

2017-11-13 21:29:45 +00:00

Oh. I see what you're saying here.

Bah.

I guess we should just remove the variable name declarations from all headers?

Oh. I see what you're saying here. Bah. I guess we should just remove the variable name declarations from all headers?

karen-arutyunov commented

2017-11-14 10:28:11 +00:00

(Migrated from github.com)

Currently structure variables are the part of the library interface. We, in our build2 project, rely on them quite heavily. Also pkgconf executable, does the same as I can see.

I think that hiding all structure variables will tremendously decrease the library usability, and most likely end up with inventing additional accessor functions or smth.

Currently structure variables are the part of the library interface. We, in our build2 project, rely on them quite heavily. Also pkgconf executable, does the same as I can see. I think that hiding all structure variables will tremendously decrease the library usability, and most likely end up with inventing additional accessor functions or smth.

ariadne commented

2017-11-15 00:43:33 +00:00

I propose we rename the structure members when compiling as C++. That way the C interface remains clean.

karen-arutyunov commented

2017-11-15 10:15:15 +00:00

(Migrated from github.com)

It is not very common to name structure variables differently for C/C++, but seems to work. At least I can't think of how it can break anything.

If you decide to go this way, then it should probably look like this:

#ifdef __cplusplus
  pkgconf_list_t requires_; /* Keyword in C++20. */
#else
  pkgconf_list_t requires;
#endif

It is not very common to name structure variables differently for C/C++, but seems to work. At least I can't think of how it can break anything. If you decide to go this way, then it should probably look like this: ``` #ifdef __cplusplus pkgconf_list_t requires_; /* Keyword in C++20. */ #else pkgconf_list_t requires; #endif ```

ariadne commented

2017-12-08 19:05:21 +00:00

any opinion on requiring the C++ interface to have prefixes on the member names?

karen-arutyunov commented

2017-12-08 21:38:43 +00:00

(Migrated from github.com)

I think that prefixing all members for C++ just because of a single name conflict is an overkill. Also I'm not a fan of member prefixes.

My personal preferences in descending order of attractiveness are:

Rename the 'requires' member for both C and C++, for example, to 'requirements'. No hairiness, but can imagine you don't want to bother existing C clients.
Add trailing underscore for C++, so it becomes 'requires_'. Not too ugly and is somehow consistent with the trailing underscores in the structure names.
Add leading underscore for C++, so it becomes '_requires'.

I think that prefixing all members for C++ just because of a single name conflict is an overkill. Also I'm not a fan of member prefixes. My personal preferences in descending order of attractiveness are: 1. Rename the 'requires' member for both C and C++, for example, to 'requirements'. No hairiness, but can imagine you don't want to bother existing C clients. 2. Add trailing underscore for C++, so it becomes 'requires_'. Not too ugly and is somehow consistent with the trailing underscores in the structure names. 3. Add leading underscore for C++, so it becomes '_requires'.

plicease commented

2017-12-09 14:16:57 +00:00

(Migrated from github.com)

I have a mild preference for renaming to something that doesn't conflict in C++. I am not currently using this field in my Perl bindings, but if I do end up needing this it could be a problem to have different C++ name, because Perl XS code can be compiled using either a C or a C++ compiler (it is up to the whomever compiles Perl). If I have to, I can #ifdef around this of course. There may be other tools that have similar requirements.

I have a mild preference for renaming to something that doesn't conflict in C++. I am not currently using this field in my Perl bindings, but if I do end up needing this it could be a problem to have different C++ name, because Perl XS code can be compiled using either a C or a C++ compiler (it is up to the whomever compiles Perl). If I have to, I can `#ifdef` around this of course. There may be other tools that have similar requirements.

Sign in to join this conversation.