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.

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

Fix dead cxxstring_abi code paths #616

Merged
merged 5 commits into from
Jan 16, 2020
Merged

Conversation

staticfloat
Copy link
Member

We were accidentally stripping out ABI information from our platform
objects while generating compiler wrappers, resulting in cxxstring_abi
settings being ignored. In order to ensure that this does not happen
again, we've added some more tests ensuring that when we ask to build a
cxx11 library, we actually get what we asked for.

Also, the cppfilt() function wasn't working at all because I forgot to seek(input, 0) before sending it into the run_interactive().

@staticfloat staticfloat force-pushed the sf/cxxstringabi_testing branch from 7f33280 to 55773f3 Compare January 15, 2020 23:48
@staticfloat staticfloat requested a review from giordano January 15, 2020 23:49
@staticfloat staticfloat force-pushed the sf/cxxstringabi_testing branch from 55773f3 to f4670fe Compare January 15, 2020 23:58
We were accidentally stripping out ABI information from our platform
objects while generating compiler wrappers, resulting in `cxxstring_abi`
settings being ignored.  In order to ensure that this does not happen
again, we've added some more tests ensuring that when we ask to build a
`cxx11` library, we actually get what we asked for.
@staticfloat staticfloat force-pushed the sf/cxxstringabi_testing branch from f4670fe to caa443d Compare January 16, 2020 00:04
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

unpack(tarball_path, testdir)
prefix = Prefix(testdir)

# Ensure that the library detects as the correct cxxstring_abi:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add also a unit test on cppfilt that checks it returns what is supposed to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants