This is an archive of the discontinued LLVM Phabricator instance.

Differential D117935

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

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

Details

Reviewers
cebowleratibm
hubert.reinterpretcast
Commits
rG27ee91162dd3: [AIX][clang] include_next through clang provided float.h
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
Herald added a subscriber: mstorsjo. · View Herald TranscriptJan 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)
Harbormaster completed remote builds in B144947: Diff 402127.Jan 21 2022, 5:27 PM
hubert.reinterpretcast updated this revision to Diff 402156.Jan 21 2022, 6:47 PM
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
hubert.reinterpretcast added a comment.Jan 21 2022, 7:09 PM

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.

Harbormaster completed remote builds in B144966: Diff 402156.Jan 21 2022, 7:29 PM
hubert.reinterpretcast added inline comments.Jan 21 2022, 7:34 PM
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

Harbormaster completed remote builds in B145367: Diff 402720.Jan 26 2022, 12:38 PM
hubert.reinterpretcast accepted this revision.Jan 26 2022, 8:02 PM

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
Closed by commit rG27ee91162dd3: [AIX][clang] include_next through clang provided float.h (authored by daltenty). · Explain Why
This revision was automatically updated to reflect the committed changes.
daltenty added a commit: rG27ee91162dd3: [AIX][clang] include_next through clang provided float.h.
Harbormaster completed remote builds in B146328: Diff 404083.Jan 28 2022, 11:27 AM

Revision Contents

PathSize
clang/
lib/
Headers/
21 lines
test/
Headers/
Inputs/
include/
2 lines
6 lines

Diff 402720

clang/lib/Headers/float.h

Context not available.
* additional definitions provided for Windows. * additional definitions provided for Windows.
* For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx * For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx
* *
* Also fall back on Darwin to allow additional definitions and * Also fall back on Darwin and AIX to allow additional definitions and
* implementation-defined values. * implementation-defined values.
*/ */
#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \ #if (defined(__APPLE__) || defined(__MINGW32__) || defined(_MSC_VER) || \
defined(_AIX)) && \
hubert.reinterpretcastUnsubmitted
Done
ReplyInline Actions
* implementation-defined values.
*/
- #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) || \
- defined(_AIX) && __STDC_HOSTED__ && __has_include_next(<float.h>)
+ #if (defined(__APPLE__) || defined(__MINGW32__) || defined(_MSC_VER) || \
+ defined(_AIX)) && \
+ __STDC_HOSTED__ && __has_include_next(<float.h>)
/* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level

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

hubert.reinterpretcast: Fix the binding of the `&&` and remove the preexisting excess parentheses.
__STDC_HOSTED__ && __has_include_next(<float.h>) __STDC_HOSTED__ && __has_include_next(<float.h>)
/* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level /* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level
Context not available.
# undef FLT_MANT_DIG # undef FLT_MANT_DIG
# undef DBL_MANT_DIG # undef DBL_MANT_DIG
# undef LDBL_MANT_DIG # undef LDBL_MANT_DIG
# if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L # if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || \
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-#  if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) ||              \
-      __cplusplus >= 201103L ||                                                \
-      (__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
+#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) ||                \
+    __cplusplus >= 201103L ||                                                  \
+    (__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
Lint: Pre-merge checks: clang-format: please reformat the code ``` -# if __STDC_VERSION__ >= 199901L || !defined…
hubert.reinterpretcastUnsubmitted
Done
ReplyInline Actions
# undef LDBL_MANT_DIG
- #if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || \
- __cplusplus >= 201103L || defined(_AIX) && defined(_ALL_SOURCE)
+ # if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || \
+ __cplusplus >= 201103L || defined(_AIX) && defined(_ALL_SOURCE)
# undef DECIMAL_DIG

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

hubert.reinterpretcast: Do not remove scoping indentation; see below where reformatting actively causes confusion.
__cplusplus >= 201103L || \
(__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
# undef DECIMAL_DIG # undef DECIMAL_DIG
# endif # endif
# undef FLT_DIG # undef FLT_DIG
Context not available.
# undef FLT_MIN # undef FLT_MIN
# undef DBL_MIN # undef DBL_MIN
# undef LDBL_MIN # undef LDBL_MIN
hubert.reinterpretcastUnsubmitted
Done
ReplyInline Actions
# undef LDBL_MIN
- #if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || \
- __cplusplus >= 201703L || defined(_AIX) && defined(_ALL_SOURCE)
+ # if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || \
+ __cplusplus >= 201703L || defined(_AIX) && defined(_ALL_SOURCE)
# undef FLT_TRUE_MIN

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

hubert.reinterpretcast: Change in indentation makes it look like (an casual reading) that the corresponding `#endif` is…
# if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L # if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || \
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-#  if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) ||              \
-      __cplusplus >= 201703L ||                                                \
-      (__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
+#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) ||                \
+    __cplusplus >= 201703L ||                                                  \
+    (__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
Lint: Pre-merge checks: clang-format: please reformat the code ``` -# if __STDC_VERSION__ >= 201112L || !defined…
__cplusplus >= 201703L || \
(__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
# undef FLT_TRUE_MIN # undef FLT_TRUE_MIN
# undef DBL_TRUE_MIN # undef DBL_TRUE_MIN
# undef LDBL_TRUE_MIN # undef LDBL_TRUE_MIN
Context not available.
#define DBL_MANT_DIG __DBL_MANT_DIG__ #define DBL_MANT_DIG __DBL_MANT_DIG__
#define LDBL_MANT_DIG __LDBL_MANT_DIG__ #define LDBL_MANT_DIG __LDBL_MANT_DIG__
hubert.reinterpretcastUnsubmitted
Done
ReplyInline Actions
#define LDBL_MANT_DIG __LDBL_MANT_DIG__
#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || \
- __cplusplus >= 201103L || defined(_AIX) && defined(_ALL_SOURCE)
+ __cplusplus >= 201103L || \
+ (__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
# define DECIMAL_DIG __DECIMAL_DIG__

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.

hubert.reinterpretcast: I understand why `_ALL_SOURCE` matters when `__STDC_HOSTED__` does not evaluate to 0, but the…
#if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || __cplusplus >= 201103L #if __STDC_VERSION__ >= 199901L || !defined(__STRICT_ANSI__) || \
__cplusplus >= 201103L || \
(__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
# define DECIMAL_DIG __DECIMAL_DIG__ # define DECIMAL_DIG __DECIMAL_DIG__
#endif #endif
hubert.reinterpretcastUnsubmitted
Done
ReplyInline Actions

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

hubert.reinterpretcast: Same comment as above. Applies also to similar changes in the rest of the file.
Context not available.
#define DBL_MIN __DBL_MIN__ #define DBL_MIN __DBL_MIN__
#define LDBL_MIN __LDBL_MIN__ #define LDBL_MIN __LDBL_MIN__
#if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || __cplusplus >= 201703L #if __STDC_VERSION__ >= 201112L || !defined(__STRICT_ANSI__) || \
__cplusplus >= 201703L || \
(__STDC_HOSTED__ && defined(_AIX) && defined(_ALL_SOURCE))
# define FLT_TRUE_MIN __FLT_DENORM_MIN__ # define FLT_TRUE_MIN __FLT_DENORM_MIN__
# define DBL_TRUE_MIN __DBL_DENORM_MIN__ # define DBL_TRUE_MIN __DBL_DENORM_MIN__
# define LDBL_TRUE_MIN __LDBL_DENORM_MIN__ # define LDBL_TRUE_MIN __LDBL_DENORM_MIN__
Context not available.

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

  • This file was added.
#pragma once
#define FLOAT_LOCAL_DEF 1
hubert.reinterpretcastUnsubmitted
Not Done
ReplyInline Actions
#pragma once
- #define FLOAT_LOCAL_DEF 1
+ #define ERSATZ_SYSTEM_FLOAT_H 1

Minor nit: Suggest to use more explicit name.

hubert.reinterpretcast: Minor nit: Suggest to use more explicit name.

clang/test/Headers/float-aix.c

  • This file was added.
// RUN: %clang_cc1 -triple powerpc-ibm-aix -fsyntax-only -verify -internal-isystem %S/Inputs/include %s
// expected-no-diagnostics
#include <float.h>
_Static_assert(FLOAT_LOCAL_DEF, "");
hubert.reinterpretcastUnsubmitted
Not Done
ReplyInline Actions
#include <float.h>
- _Static_assert(FLOAT_LOCAL_DEF, "");
+ _Static_assert(ERSATZ_SYSTEM_FLOAT_H, "");
hubert.reinterpretcast: