Fix makefs bootstrap on macOS after D25563
ClosedPublic
Actions

Authored by arichardson on Aug 3 2020, 2:40 PM.

Details

Reviewers

delphij
emaste

Commits

rS364765: Fix makefs bootstrap on macOS after D25563

Summary

The macOS assert.h header does not define static_assert when compiling in
C99 mode. To fix this compile with -std=c11.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33138
Build 30497: arc lint + arc unit

Event Timeline

arichardson requested review of this revision.Aug 3 2020, 2:40 PM

arichardson created this revision.

Harbormaster completed remote builds in B32718: Diff 75305.Aug 3 2020, 2:40 PM

arichardson added a child revision: D16767: Add a tools/build/make.py script that bootstraps bmake and then runs the build.Aug 7 2020, 3:30 PM

I'm not sure but if I'm reading it right:

#ifndef __cplusplus
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
#define static_assert _Static_assert
#endif /* __STDC_VERSION__ */
#endif /* !__cplusplus */

It's basically saying, if we are not compiling for C++ and if -std=c11 was used, static_assert is defined as an alias of _Static_assert? Will saying CSTD=c11 fix this on macOS?

In D25928#578302, @delphij wrote:
I'm not sure but if I'm reading it right:
#ifndef __cplusplus
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
#define static_assert _Static_assert
#endif /* __STDC_VERSION__ */
#endif /* !__cplusplus */
It's basically saying, if we are not compiling for C++ and if -std=c11 was used, static_assert is defined as an alias of _Static_assert? Will saying CSTD=c11 fix this on macOS?

Yeah using C11 should also fix it since C11 requires assert.h to define static_assert.

I'd slightly prefer compiling as C11 and spelling it the sensible (static_assert) way, but either way is acceptable to me.

This revision is now accepted and ready to land.Aug 21 2020, 11:40 PM

Use CSTD instead

This revision now requires review to proceed.Aug 24 2020, 10:05 AM

Harbormaster completed remote builds in B33138: Diff 76146.Aug 24 2020, 10:05 AM

arichardson edited the summary of this revision. (Show Details)Aug 24 2020, 10:05 AM

Also add CSTD to makefs

Harbormaster completed remote builds in B33139: Diff 76147.Aug 24 2020, 10:13 AM

As an aside, we should put CSTD in style.Makefile(5). Looking at a few other examples this order seems to be common -- WARNS, CSTD, CFLAGS

This revision is now accepted and ready to land.Aug 24 2020, 12:45 PM

In D25928#581280, @emaste wrote:

As an aside, we should put CSTD in style.Makefile(5). Looking at a few other examples this order seems to be common -- WARNS, CSTD, CFLAGS

I searched for CSTD and it seems most uses are after WARNS, so I did the same here. I wasn't aware of style.Makefile(5) but if you'd like I can add CSTD between warns and CFLAGS?

In D25928#581283, @arichardson wrote:

I searched for CSTD and it seems most uses are after WARNS, so I did the same here. I wasn't aware of style.Makefile(5) but if you'd like I can add CSTD between warns and CFLAGS?

Sure, I think it'd be fine to make that change to style.Makefile.

Closed by commit rS364765: Fix makefs bootstrap on macOS after D25563 (authored by arichardson). · Explain WhyAug 25 2020, 1:30 PM

This revision was automatically updated to reflect the committed changes.

arichardson added a commit: rS364765: Fix makefs bootstrap on macOS after D25563.

Herald added a subscriber: imp. · View Herald TranscriptAug 25 2020, 1:30 PM

arichardson mentioned this in rS364766: style.Makefile: list CSTD between WARNS and CFLAGS.Aug 25 2020, 1:30 PM