This is an archive of the discontinued LLVM Phabricator instance.

Differential D86055

[libc++] Fix the wrong usage of __STDC_HOSTED__
ClosedPublic

Authored by hyd-dev on Aug 17 2020, 4:05 AM.

Details

Reviewers
__simt__
jfb
ldionne
Group Reviewers
Restricted Project
Commits
rG44cc78da056f: [libc++] Fix incorrect usage of __STDC_HOSTED__
Summary

D56913 introduced the _LIBCPP_FREESTANDING macro and implemented it as:

#ifndef __STDC_HOSTED__
#  define _LIBCPP_FREESTANDING
#endif

However, __STDC_HOSTED__ is defined as 0 in freestanding implementations instead of undefined (according to https://github.com/llvm/llvm-project/blob/cd2139a527f2d829bdde7877c992215f598e927b/clang/lib/Frontend/InitPreprocessor.cpp#L356-L362 and https://en.cppreference.com/w/cpp/preprocessor/replace).
This patch corrects the above as:

#if __STDC_HOSTED__ == 0
#  define _LIBCPP_FREESTANDING
#endif

Diff Detail

Event Timeline

hyd-dev created this revision.Aug 17 2020, 4:05 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 17 2020, 4:05 AM
Herald added subscribers: libcxx-commits, dexonsmith. · View Herald Transcript
hyd-dev requested review of this revision.Aug 17 2020, 4:05 AM
Harbormaster completed remote builds in B68584: Diff 285955.Aug 17 2020, 4:19 AM
hyd-dev updated this revision to Diff 288919.Aug 31 2020, 4:31 AM
hyd-dev edited the summary of this revision. (Show Details)

Add a test (libcxx/test/libcxx/libcpp_freestanding.sh.cpp).

ldionne requested changes to this revision.Sep 1 2020, 7:57 AM

Thanks for the fix. See my concern about the test, though.

libcxx/test/libcxx/libcpp_freestanding.sh.cpp
2

Did you run this test? I don't think we can use FileCheck in the libcxx tests yet, can we?

I had a patch lying somewhere that enabled FileCheck in the libc++ test suite, but I had shelved it when it became a lot more complicated than expected. Perhaps it's now time to revive it?

This revision now requires changes to proceed.Sep 1 2020, 7:57 AM
hyd-dev added inline comments.Sep 2 2020, 1:00 AM
libcxx/test/libcxx/libcpp_freestanding.sh.cpp
2

Did you run this test? I don't think we can use FileCheck in the libcxx tests yet, can we?

Sorry, I have run this test before submitting it and it passed, but that was because I installed FileCheck using the system package manager.

I had a patch lying somewhere that enabled FileCheck in the libc++ test suite, but I had shelved it when it became a lot more complicated than expected. Perhaps it's now time to revive it?

That's not necessary. I'll update the test to use #ifdefs and #errors.

hyd-dev updated this revision to Diff 289366.Sep 2 2020, 1:37 AM

Update the test to use #if defineds and #error.

hyd-dev marked an inline comment as done.Sep 2 2020, 1:39 AM
ldionne accepted this revision.Sep 2 2020, 8:52 AM

Thanks for fixing!

This revision is now accepted and ready to land.Sep 2 2020, 8:52 AM
hyd-dev added a comment.Sep 2 2020, 9:19 AM

@ldionne Thanks! Could you commit it for me (hyd-dev <yd-huang@outlook.com>)?

ldionne added a comment.Sep 2 2020, 9:27 AM

Committed, thanks!

Closed by commit rG44cc78da056f: [libc++] Fix incorrect usage of __STDC_HOSTED__ (authored by hyd-dev, committed by ldionne). · Explain WhySep 2 2020, 9:27 AM
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rG44cc78da056f: [libc++] Fix incorrect usage of __STDC_HOSTED__.

Revision Contents

PathSize
libcxx/
include/
2 lines
test/
libcxx/
21 lines

Diff 289493

libcxx/include/__config

Show All 32 Lines
#endif #endif
#define _LIBCPP_VERSION 12000 #define _LIBCPP_VERSION 12000
#ifndef _LIBCPP_ABI_VERSION #ifndef _LIBCPP_ABI_VERSION
# define _LIBCPP_ABI_VERSION 1 # define _LIBCPP_ABI_VERSION 1
#endif #endif
#ifndef __STDC_HOSTED__ #if __STDC_HOSTED__ == 0
# define _LIBCPP_FREESTANDING # define _LIBCPP_FREESTANDING
#endif #endif
#ifndef _LIBCPP_STD_VER #ifndef _LIBCPP_STD_VER
# if __cplusplus <= 201103L # if __cplusplus <= 201103L
# define _LIBCPP_STD_VER 11 # define _LIBCPP_STD_VER 11
# elif __cplusplus <= 201402L # elif __cplusplus <= 201402L
# define _LIBCPP_STD_VER 14 # define _LIBCPP_STD_VER 14
▲ Show 20 Lines • Show All 1,502 Lines • Show Last 20 Lines

libcxx/test/libcxx/libcpp_freestanding.sh.cpp

  • This file was added.
// -*- C++ -*-
//===----------------------------------------------------------------------===//
ldionneUnsubmitted
Done
ReplyInline Actions

Did you run this test? I don't think we can use FileCheck in the libcxx tests yet, can we?

I had a patch lying somewhere that enabled FileCheck in the libc++ test suite, but I had shelved it when it became a lot more complicated than expected. Perhaps it's now time to revive it?

ldionne: Did you run this test? I don't think we can use `FileCheck` in the libcxx tests yet, can we? I…
hyd-devAuthorUnsubmitted
Done
ReplyInline Actions

Did you run this test? I don't think we can use FileCheck in the libcxx tests yet, can we?

Sorry, I have run this test before submitting it and it passed, but that was because I installed FileCheck using the system package manager.

I had a patch lying somewhere that enabled FileCheck in the libc++ test suite, but I had shelved it when it became a lot more complicated than expected. Perhaps it's now time to revive it?

That's not necessary. I'll update the test to use #ifdefs and #errors.

hyd-dev: > Did you run this test? I don't think we can use FileCheck in the libcxx tests yet, can we?
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// Test that _LIBCPP_FREESTANDING is not defined when -ffreestanding is not passed
// to the compiler but defined when -ffreestanding is passed to the compiler.
// RUN: %{cxx} %{flags} %{compile_flags} -fsyntax-only %s
// RUN: %{cxx} %{flags} %{compile_flags} -fsyntax-only -ffreestanding -DFREESTANDING %s
#include <__config>
#if defined(FREESTANDING) != defined(_LIBCPP_FREESTANDING)
#error _LIBCPP_FREESTANDING should be defined in freestanding mode and not \
defined in non-freestanding mode
#endif