Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix issue with C-code-compiled-as-C++ from 59e148d #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix issue with C-code-compiled-as-C++ from 59e148d #70

wants to merge 1 commit into from

Conversation

jnpkrn
Copy link

@jnpkrn jnpkrn commented Jan 8, 2018

Said commit was meant to fix [Issue#64], but rather introduced new
problem -- when "decltype(nullptr)" was observed (happens on transitive
includes starting with standard C library headers, at least with newer
stdlibc++ as this is how includes are treated by gcc when it's passed
"-x c++-header"), the C-only-detection logic would short-circuit (or
give up) early, leaving C++ compatibility measures off, and in turn,
choking later on when valid C identifiers such as "new" are used
somewhere in the headers of the C project at hand.

This rather simplistic solution masks, item-wise, "decltype(nullptr)"
expression consisting of otherwise C++ compatibility mangling eligible
words, and restores them right after there's no danger this would happen
to them, because they need to be preserved (empirically tested).

[Issue#64] #64

@jnpkrn
Fix issue with C-code-compiled-as-C++ from 59e148d
6406868 
Said commit was meant to fix [Issue#64], but rather introduced new
problem -- when "decltype(nullptr)" was observed (happens on transitive
includes starting with standard C library headers, at least with newer
stdlibc++ as this is how includes are treated by gcc when it's passed
"-x c++-header"), the C-only-detection logic would short-circuit (or
give up) early, leaving C++ compatibility measures off, and in turn,
choking later on when valid C identifiers such as "new" are used
somewhere in the headers of the C project at hand.

This rather simplistic solution masks, item-wise, "decltype(nullptr)"
expression consisting of otherwise C++ compatibility mangling eligible
words, and restores them right after there's no danger this would happen
to them, because they need to be preserved (empirically tested).

[Issue#64] lvc#64

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
@jnpkrn jnpkrn mentioned this pull request Jan 8, 2018
Open
jnpkrn added a commit to jnpkrn/libqb that referenced this pull request Jan 10, 2018
@jnpkrn
maint: check abi: fix two issues with abi-compliance-checker/libstdc++
4faa64a 
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
   no longer a default for this tool (used to be vice versa),
   plus current version will stop choking on C vs. C++ (our C code with
   C++ compatibility wrapping being viewed from C++ perspective for the
   purpose of dumping the declared symbols, which somewhat conflicts
   with internal masking of the C++ keywords being used as valid C
   identifiers [yet some instances must not be masked here, see
   lvc/abi-compliance-checker#64) only
  if _also_ something like this is applied:
   lvc/abi-compliance-checker#70

2. since 20246f5, libqb.so no longer poses a symlink to the actual
   version-qualified shared library, but rather a standalone linker
   script, which confuses ABICC, so blacklist that file for the scanning
   purposes explicitly, together with referring to the library through
   it's basic version qualification (which alone, sadly, is not
   sufficient as ABICC proceeds to scan whole containing directory
   despite particular file is specified)

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
@jnpkrn jnpkrn mentioned this pull request Jan 10, 2018
Closed
jnpkrn added a commit to jnpkrn/libqb that referenced this pull request Jan 10, 2018
@jnpkrn
Low: qblist.h: fix incompatibility with C++ & check it regularly
8f2e62e 
Primary trigger was the fact that "./check abi" (via
build-aux/generate-docs) choked on that because abi-compliance-checker
(ABICC) uses g++ to list symbols in header files playing hide-and-see
regarding C identifiers otherwise conflicting with C++ keywords,
which is moreover triggered only with -cxx-incompatible switch
since ABICC 2.0 (and even then the respective code needs some further
tweaking: lvc/abi-compliance-checker#70),
and this switch wasn't historically applied.

Anyway, this C++ perspective there was likely unprecedented even
though libqb no doubt targets such compatiblity.  To avoid its
breakage within the public headers' scope in the future, arrange
for checking it regularly alongside the "auto_check_header" tests
(cf. make -C tests check-headers).

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
jnpkrn added a commit to jnpkrn/libqb that referenced this pull request Jan 10, 2018
@jnpkrn
maint: check abi: fix two issues with abi-compliance-checker/libstdc++
ec53fb3 
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
   no longer a default for this tool (used to be vice versa),
   plus current version will stop choking on C vs. C++ (our C code with
   C++ compatibility wrapping being viewed from C++ perspective for the
   purpose of dumping the declared symbols, which somewhat conflicts
   with internal masking of the C++ keywords being used as valid C
   identifiers [yet some instances must not be masked here, see
   lvc/abi-compliance-checker#64) only
  if _also_ something like this is applied:
   lvc/abi-compliance-checker#70

2. since 20246f5, libqb.so no longer poses a symlink to the actual
   version-qualified shared library, but rather a standalone linker
   script, which confuses ABICC, so blacklist that file for the scanning
   purposes explicitly, together with referring to the library through
   it's basic version qualification (which alone, sadly, is not
   sufficient as ABICC proceeds to scan whole containing directory
   despite particular file is specified)

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
jnpkrn added a commit to jnpkrn/libqb that referenced this pull request Jan 11, 2018
@jnpkrn
maint: check abi: fix two issues with abi-compliance-checker/libstdc++
e0bc8b1 
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
   no longer a default for this tool (used to be vice versa),
   plus current version will stop choking on C vs. C++ (our C code with
   C++ compatibility wrapping being viewed from C++ perspective for the
   purpose of dumping the declared symbols, which somewhat conflicts
   with internal masking of the C++ keywords being used as valid C
   identifiers [yet some instances must not be masked here, see
   lvc/abi-compliance-checker#64) only
  if _also_ something like this is applied:
   lvc/abi-compliance-checker#70

2. since 20246f5, libqb.so no longer poses a symlink to the actual
   version-qualified shared library, but rather a standalone linker
   script, which confuses ABICC, so blacklist that file for the scanning
   purposes explicitly, together with referring to the library through
   it's basic version qualification (which alone, sadly, is not
   sufficient as ABICC proceeds to scan whole containing directory
   despite particular file is specified)

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
jnpkrn added a commit to jnpkrn/libqb that referenced this pull request Jan 11, 2018
@jnpkrn
Low: qblist.h: fix incompatibility with C++ & check it regularly
3107df1 
Primary trigger was the fact that "./check abi" (via
build-aux/generate-docs) choked on that because abi-compliance-checker
(ABICC) uses g++ to list symbols in header files playing hide-and-see
regarding C identifiers otherwise conflicting with C++ keywords,
which is moreover triggered only with -cxx-incompatible switch
since ABICC 2.0 (and even then the respective code needs some further
tweaking: lvc/abi-compliance-checker#70),
and this switch wasn't historically applied.

Anyway, this C++ perspective there was likely unprecedented even
though libqb no doubt targets such compatiblity.  To avoid its
breakage within the public headers' scope in the future, arrange
for checking it regularly alongside the "auto_check_header" tests
(cf. make -C tests check-headers).

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
chrissie-c added a commit to ClusterLabs/libqb that referenced this pull request Sep 25, 2018
@chrissie-c
UPDATED: doc (ABI comparison) and various other fixes (#324)
d0ec0a6 
* doc: qbarray.h: fix garbled Doxygen markup

* build: follow-up for and fine-tuning of a rushed 6d62b64 commit
  (It made a service as-was, but being afforded more time, this would
  have accompanied that commit right away, for better understanding,
  brevity and uniformity.)

* build: prune superfluous Makefile declarations within tests directory
  There was a significant redundancy wrt. build flags and EXTRA_DIST
  assignment (the latter become redundant as of f6e4042 at latest)
  spread all over the place (vivat copy&paste).  Also, in one instance,
  CPPFLAGS (used) was confused with CFLAGS (meant).

* maint: check abi: fix two issues with abi-compliance-checker/libstdc++
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
   no longer a default for this tool (used to be vice versa),
   plus current version will stop choking on C vs. C++ (our C code with
   C++ compatibility wrapping being viewed from C++ perspective for the
   purpose of dumping the declared symbols, which somewhat conflicts
   with internal masking of the C++ keywords being used as valid C
   identifiers [yet some instances must not be masked here, see
   lvc/abi-compliance-checker#64) only
  if _also_ something like this is applied:
   lvc/abi-compliance-checker#70
2. since 20246f5, libqb.so no longer poses a symlink to the actual
   version-qualified shared library, but rather a standalone linker
   script, which confuses ABICC, so blacklist that file for the scanning
   purposes explicitly, together with referring to the library through
   it's basic version qualification (which alone, sadly, is not
   sufficient as ABICC proceeds to scan whole containing directory
   despite particular file is specified)

* maint: check abi: switch to abi-dumper for creating "ABI dumps"
Beside avoiding issues with abi-compliance-checker in the role of ABI
dumps producer (see the preceding commit), it also seems to generate
more accurate picture (maybe because it expressly requires compiling
with debugging information requested).

* Low: qblist.h: fix incompatibility with C++ & check it regularly

* tests: check_list.c: start zeroing in on the gaps in tests' coverage

* tests: print_ver: make preprocessor emit "note" rather than warning

IIRC, Chrissie asked about this around inclusion of the test at
hand, and it seemed there was no way but to emit a warning to get
something output at all.  Now it turns wrong, and moreover, we
make the code not fixed on GCC specific pragmas, with a bit of
luck, "#pragma message" approach is adopted more widely by compilers.

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>

* Replace ck_assert_uint_eq() with ck_assert_int_eq()
it's not available in check 0.9

* Proper check for C++ compiler (from Fabio)
* add (c) to copyright dates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@jnpkrn