This is an archive of the discontinued LLVM Phabricator instance.

[AIX][clang] include_next through clang provided float.h
ClosedPublic

Authored by daltenty on Jan 21 2022, 3:51 PM.

Details

Summary

AIX provides additional definitions in the system libc float.h that we
would like to be available to users, so we need to include_next through,
similar to what is done on some other platforms.

We also adjust the guards for some definitions which are restricted
based on language level to also be provide with the _ALL_SOURCE feature
test macro on AIX, similar to what is done by the platform float.h
header, so we don't run into cases where we don't provide the compiler
macro but still have a different definition from the system.

Diff Detail

Event Timeline

daltenty created this revision.Jan 21 2022, 3:51 PM
daltenty requested review of this revision.Jan 21 2022, 3:51 PM
daltenty edited the summary of this revision. (Show Details)
hubert.reinterpretcast edited the summary of this revision. (Show Details)

Refresh with full context

Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2022, 6:47 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

More comments may follow, but didn't want to delay the first one.

clang/lib/Headers/float.h
20–21

Fix the binding of the && and remove the preexisting excess parentheses.

clang/lib/Headers/float.h
40–41

Do not remove scoping indentation; see below where reformatting actively causes confusion.

68–69

Change in indentation makes it look like (an casual reading) that the corresponding #endif is the one on line 80.

92–93

I understand why _ALL_SOURCE matters when __STDC_HOSTED__ does not evaluate to 0, but the general logic of the file is that platform extensions are not present for freestanding. The interfaces injected via _ALL_SOURCE are platform extensions.

129–130

Same comment as above. Applies also to similar changes in the rest of the file.

clang/test/Headers/float.c
7 ↗(On Diff #402156)

The changes to the test are for functionality that I do not believe should be present (see earlier comments).

daltenty updated this revision to Diff 402720.Jan 24 2022, 5:19 PM
daltenty marked 6 inline comments as done.

Address review comments

LGTM with minor comment.

clang/test/Headers/Inputs/include/float.h
2

Minor nit: Suggest to use more explicit name.

clang/test/Headers/float-aix.c
6
This revision is now accepted and ready to land.Jan 26 2022, 8:02 PM
daltenty updated this revision to Diff 404083.Jan 28 2022, 10:26 AM

Use more unique macro name

This revision was landed with ongoing or failed builds.Jan 28 2022, 10:27 AM
This revision was automatically updated to reflect the committed changes.