Skip to content

Remove special handling of functions defined in system headers. #431

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

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

mattmccutchen-cci
Copy link
Member

The liberal itypes change (#356) added code to flag them as not having a
body even though they did. This may have been a useful mitigation for
certain problems with unwritable files at the time, but it risks causing
confusion and violating 3C invariants in the future. From now on, we
want to handle all unwritable files (including system headers) via the
new canWrite constraints approach.

It appears that removing this code will immediately fix one problem with
cast insertion that is currently blocking the benchmark tests (#423), so
we'll go ahead and make this change even though we don't fully
understand the cast insertion problem or how to appropriately
regression-test it.

The liberal itypes change (#356) added code to flag them as not having a
body even though they did. This may have been a useful mitigation for
certain problems with unwritable files at the time, but it risks causing
confusion and violating 3C invariants in the future. From now on, we
want to handle all unwritable files (including system headers) via the
new canWrite constraints approach.

It appears that removing this code will immediately fix one problem with
cast insertion that is currently blocking the benchmark tests (#423), so
we'll go ahead and make this change even though we don't fully
understand the cast insertion problem or how to appropriately
regression-test it.
Copy link
Collaborator

@john-h-kastner john-h-kastner left a comment

Choose a reason for hiding this comment

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

This looks good. It fixes that benchmarks test that I expected it to.

The problem that the isInSysytemHeader checked fixed was that atoi instdlib.h was not constrained to WILD because it had a body, and we were not doing correct base dir checks. With the base dir fix, this change should be fine.

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