[PATCH] D144889: [C2x] Support in freestanding

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

Let's talk about these use cases.

> On the one hand, I think many users will want a libc that targets their 
> particular freestanding environment so that they get the best performance 
> characteristics (and other considerations) for their target.

I think this fits kernel driver environments, GPUs, and other environments 
where the vendor is heavily involved in compiler development.  The vendor is 
likely to provide extensions to their freestanding implementation as well (like 
math.h functions for GPUs).

> On the other hand, I think a not-insignificant number of users are interested 
> in freestanding environments for one-off/fun/experimental projects where ease 
> of access is more important than performance characteristics -- think: users 
> who are playing around with an arduino, etc.

This is also an important use case.  I don't think it rules out a "stock" 
freestanding implementation though.  A toolchain could reasonably ship one 
freestanding static library for each ISA-subset / target-triple they want to 
support, and it could work regardless of the target operating system.  `memcpy` 
has the same assembly whether it's on OpenRTOS or threadx or MyHomeGrownOS, so 
long as all are on the same processor with the same calling conventions.  A 
toolchain could also try the header-only approach and lean on the optimizer to 
turn `strcpy` into reasonable assembly.

I will definitely agree on the big point with this use case though.  I don't 
want to force every OS vendor to also need to be a compiler / libc vendor, so 
long as they are willing to settle for the freestanding subset of the language. 
 I also don't want to force them to rewrite `strcpy` and friends.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144889/new/

https://reviews.llvm.org/D144889

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144889: [C2x] Support in freestanding

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

> 1. A normal operating system target with a "hosted" libc.
> 2. A baremetal target with an "embedded" libc; basically a stripped down libc 
> which only includes stuff that doesn't require an operating system.  What 
> exactly this includes varies; may include some form of "malloc", some "POSIX" 
> APIs, semihosting, etc.
> 3. A baremetal target with no libc, using custom implementations for 
> everything.  This is what we conventionally referred to as "freestanding", 
> and what -ffreestanding implements.  The user provides memcpy/memmove/memset, 
> and uses the compiler-provided "builtins" lib if neceessary, but nothing we 
> would traditionally call "libc".

In my committee work, I'm targeting option 2.  I think that's what should be 
considered when talking about freestanding in a standards context.

Option 3 is valuable and should continue to exist, but it is non-conforming 
pre-C23 and post-C23.  The standards don't have any provision for requiring the 
user to implement mem* functions that the implementation chooses to invoke 
without the user ever spelling that function call (perhaps because they did 
`char buf[256] = {0}` though).  Post-C23, it doesn't provide enough.  The C 
library authors will likely need this mode to author the C library itself 
though.  I'll suggest calling this mode something like "nodefaultlibs".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144889/new/

https://reviews.llvm.org/D144889

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144889: [C2x] Support in freestanding

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

> Okay, so this is potentially ignorance on my part. I was under the impression 
> that folks using freestanding mode did not want any library to be linked in. 
> Are there freestanding libc implementations out there that the user would 
> link in instead that we defer to (same as we defer hosted C standard library 
> interfaces to the CRT)? Basically, do we need to do nothing in Clang to 
> support N2524?

Oh, and I forgot GPUs.  GPUs do like having everything be header-only, but in 
my opinion, that's still a requirement / request for C library authors rather 
than the compiler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144889/new/

https://reviews.llvm.org/D144889

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144889: [C2x] Support in freestanding

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

> Okay, so this is potentially ignorance on my part. I was under the impression 
> that folks using freestanding mode did not want any library to be linked in. 
> Are there freestanding libc implementations out there that the user would 
> link in instead that we defer to (same as we defer hosted C standard library 
> interfaces to the CRT)? Basically, do we need to do nothing in Clang to 
> support N2524?

Well, people want all sorts of things :)

(Gross generalizations coming) The compiler writer view of freestanding that 
I've heard in the past is that the compiler writer views freestanding as the 
necessary parts of the library needed to support the language.  This is mostly 
accurate when describing freestanding prior to C++23 and C23.  Most of that is 
either header-only, in libc++abi, or in compiler-rt.  This makes testing the 
compiler a bit easier, as you don't need a full stdlib.  That's not a design 
goal of freestanding post C/C++23 though.  It's fine for Clang to provide half 
of a stdlib for internal testing or integration with other C libraries, so long 
as the final toolchain has everything.

One challenge for some customers will be that they like to build with 
`-nodefaultlibs`, but that's not a deal breaker here, it just means that they 
would need to manually put their C library on the link line.  Alternatively, 
they could start passing other linker flags, like `-nostartfiles` instead, but 
that will be use case dependent.  These kinds of hoops have historically been 
needed because a customer may use a "stock" ARM Clang, but not want to use the 
libc that came with that toolchain, so the big linker flags would come out to 
give the user control.

I don't think that Clang-the-compiler needs to do anything to support the new 
C/C++ freestanding features.  The main caveat being that you still need to be 
able to ensure the provided memcpy gets used instead of a codegenned (possibly 
vectorized) memcpy, but I think -ffreestanding already forces the compiler to 
generate calls and not use in-compiler codegen.

> FWIW, WG14 is having a second round of CD ballot comments. If you'd like, I'm 
> happy to file a comment asking to remove strtok from the required set for 
> freestanding conformance.

That would be much appreciated.  I can try to get my INCITS / ISO status 
squared away so that I can represent that issue at the appropriate meeting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144889/new/

https://reviews.llvm.org/D144889

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144889: [C2x] Support in freestanding

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

A freestanding implementation doesn't necessarily mean that everything is 
header-only.  It's fine to require linking against a (freestanding) C runtime 
library.  All header-only is fine too though, if you want to make that work.

Architecturally, I don't feel it is required that Clang be the location of all 
the freestanding headers for C.  I think that's fine for the C library in the 
final toolchain to own those headers.  I'm not super familiar with what 
interface contracts you have between the clang-provided C headers and the 
C-library provided C headers though.

> I'm not sure it makes sense to provide a "freestanding" strtok; it requires 
> global state.

I agree with this, but the C committee felt otherwise.  C++26 freestanding is 
most likely including strtok too, to stay consistent with C (the freestanding C 
library paper is through LWG and awaiting a C++26 plenary vote).  It shouldn't 
be hard to implement in an OS-independent way, but it will be a low quality 
implementation with cross-thread races.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144889/new/

https://reviews.llvm.org/D144889

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

add a comment, and it will be fine in my eyes.  You'll need signoff from the 
code owner though.




Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+

tekknolagi wrote:
> tekknolagi wrote:
> > bcraig wrote:
> > > Looking at this again... what new cases does this catch?  FakeIntSandwich 
> > > was caught before (it is identical to 'PaddedA', and AnotherIntSandwich 
> > > generated no warning before.  So what is different?
> > > 
> > > EBO1 and EBO2 are cases above that would be nice to catch, but aren't 
> > > being caught right now.
> > Ah, you're right. I'll just keep the one in `padding_inherit`.
> @bcraig I updated the description of the diff to be more informative about 
> the particular cases this change catches.
ok, got it.  Both the old and new will catch the exact same class / struct 
declarations, but the new checker will catch more array cases.



Comment at: test/Analysis/padding_inherit.cpp:2-4
+
+// A class that has no fields and one base class should visit that base class
+// instead.

tekknolagi wrote:
> bcraig wrote:
> > Evil test case to add...
> > 
> > ```
> > struct Empty {};
> > struct DoubleEmpty : Empty {
> > Empty e;
> > };
> > ```
> > 
> > I don't think that should warn, as you can't reduce padding by rearranging 
> > element.
> > 
> > I guess your new code shouldn't catch that at all anyway, as you are 
> > testing in the other direction when field-less inherits from field-ed.
> > 
> I'll add that test case -- certainly something to consider when somebody 
> tackles the harder field/inheritance problem.
The comment needs some extra exposition.  AllowedPad is set to 20, yet this 
gets flagged anyway.  You should mention that this only warns because of the 
later declaration of `AnotherIntSandwich ais[100];`


Repository:
  rC Clang

https://reviews.llvm.org/D53206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: test/Analysis/padding_cpp.cpp:204-212
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+

Looking at this again... what new cases does this catch?  FakeIntSandwich was 
caught before (it is identical to 'PaddedA', and AnotherIntSandwich generated 
no warning before.  So what is different?

EBO1 and EBO2 are cases above that would be nice to catch, but aren't being 
caught right now.


Repository:
  rC Clang

https://reviews.llvm.org/D53206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

You should probably add whoever the current code owner of that static analyzer 
to this review.  I have very little involvement with Clang these days, so a 
"LGTM" from me doesn't carry much weight.




Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:155-157
+  if ((CXXRD->field_empty() && CXXRD->getNumBases() != 1) ||
+  CXXRD->getNumBases() != 0)
 return true;

1. correctness: If CXXRD->getNumBases() returns 1, this will return true, 
regardless of the emptiness.

2. style: split this into two if statements.  Everything else in this chunk of 
code _could_ be represented as one massive or-ed together conditional, but 
instead it uses multiple, back-to-back ifs.  This may not make sense after 
fixing the correctness bug.



Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:167-171
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+

I think this should just be an if, and not an else if.



Comment at: test/Analysis/padding_inherit.cpp:2-4
+
+// A class that has no fields and one base class should visit that base class
+// instead.

Evil test case to add...

```
struct Empty {};
struct DoubleEmpty : Empty {
Empty e;
};
```

I don't think that should warn, as you can't reduce padding by rearranging 
element.

I guess your new code shouldn't catch that at all anyway, as you are testing in 
the other direction when field-less inherits from field-ed.



Repository:
  rC Clang

https://reviews.llvm.org/D53206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

For those that would prefer random device to not exist if it isn't 
cryptographically secure, please write a wg21 paper.  I will gladly review such 
a paper, and if you need a presenter, then I will present it if I am attending. 
 I won't be at Rapperswil, but I will be at San Diego.

Stated differently, I agree with both Marshall and the other commenters on this 
page.  I don't want busted random_devices to build, AND I want a standards 
compliant implementation.

To some degree, this overlaps with http://wg21.link/P0829 .  That (my) paper 
does not require random_device to exist in a freestanding environment.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

In https://reviews.llvm.org/D32411#1056381, @EricWF wrote:

> I spoke to STL on the MSVC team a while ago, and he stated that if we 
> produced a paper describing why we need `#include_next` and the rational 
> behind it, and they would pass that on to the front-end team. They didn't 
> guarantee that they would implement it, but that seems like a good first step.


You had mentioned as much to me a year or so back in IRC.  
docs/DesignDocs/IncludeNextEmulation.rst is intended to be that document, but I 
can't find any emails where I actually sent that off.

However, there is also my earlier comment...

> Talked with mclow and some Microsoft devs in Albuquerque. I'm not expecting 
> include_next to show up in MSVC.


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 126275.
bcraig added a comment.

Rebased


https://reviews.llvm.org/D32411

Files:
  CMakeLists.txt
  docs/DesignDocs/IncludeNextEmulation.rst
  include/__config
  include/__config_site.in
  include/complex.h
  include/cstddef
  include/ctype.h
  include/errno.h
  include/float.h
  include/inttypes.h
  include/limits.h
  include/locale.h
  include/math.h
  include/setjmp.h
  include/stdbool.h
  include/stddef.h
  include/stdint.h
  include/stdio.h
  include/stdlib.h
  include/string.h
  include/wchar.h
  include/wctype.h
  utils/libcxx/test/config.py

Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -670,6 +670,8 @@
 self.cxx.compile_flags += [define]
 if m == '_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS':
 continue
+if m.startswith('_LIBCPP_INCLUDE_NEXT'):
+continue
 if m == '_LIBCPP_ABI_VERSION':
 self.config.available_features.add('libcpp-abi-version-v%s'
 % feature_macros[m])
Index: include/wctype.h
===
--- include/wctype.h
+++ include/wctype.h
@@ -51,7 +51,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wctype.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/wchar.h
===
--- include/wchar.h
+++ include/wchar.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wchar.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_WCHAR_H)
 #define _LIBCPP_WCHAR_H
@@ -116,7 +120,11 @@
 #define __CORRECT_ISO_CPP_WCHAR_H_PROTO
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wchar.h)
+#else
 #include_next 
+#endif
 
 // Determine whether we have const-correct overloads for wcschr and friends.
 #if defined(_WCHAR_H_CPLUSPLUS_98_CONFORMANCE_)
Index: include/string.h
===
--- include/string.h
+++ include/string.h
@@ -58,7 +58,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(string.h)
+#else
 #include_next 
+#endif
 
 // MSVCRT, GNU libc and its derivates may already have the correct prototype in
 // . This macro can be defined by users if their C library provides
Index: include/stdlib.h
===
--- include/stdlib.h
+++ include/stdlib.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDLIB_H)
 #define _LIBCPP_STDLIB_H
@@ -91,7 +95,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdio.h
===
--- include/stdio.h
+++ include/stdio.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdio.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDIO_H)
 #define _LIBCPP_STDIO_H
@@ -105,7 +109,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdio.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdint.h
===
--- include/stdint.h
+++ include/stdint.h
@@ -116,6 +116,10 @@
 #   define __STDC_CONSTANT_MACROS
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_VC(stdint.h)
+#else
 #include_next 
+#endif
 
 #endif  // _LIBCPP_STDINT_H
Index: include/stddef.h
===
--- include/stddef.h
+++ include/stddef.h
@@ -15,7 +15,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stddef.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDDEF_H)
 #define _LIBCPP_STDDEF_H
@@ -43,7 +47,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stddef.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdbool.h
===
--- include/stdbool.h
+++ include/stdbool.h
@@ -26,7 +26,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_VC(stdbool.h)
+#else
 #include_next 

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

Talked with mclow and some Microsoft devs in Albuquerque.  I'm not expecting 
include_next to show up in MSVC.  Sometime in the next month I hope to rebase 
this change.


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

In https://reviews.llvm.org/D37677#866743, @Quuxplusone wrote:

> This current patch just swaps out std::mutex for a std::mutex-alike class 
> that claims to be faster for uncontested accesses. Definitely safer than my 
> interpretation. :) If this patch actually helps, then I would offer that the 
> class could be provided as a reusable class `std::__spin_lock` in the  
> header instead of being hidden inside `__assoc_shared_state`.


I think the bar for accepting this should be significantly faster, and not just 
a little faster.  Spinlocks don't behave as well as mutexes in abnormal 
conditions.  Spinlocks are more likely to cause priority inversion.  They are 
more likely to cause throughput issues when there is a lot of contention, as 
the spinlock'd thread will consume a full time slice before relinquishing a 
cpu.  On Windows, CRITICAL_SECTION and SRWLOCK become electrified 
 during 
process termination to avoid indefinite hangs.  We shouldn't give all of that 
up for a minor perf gain.  We might give it up for a large perf gain though.


https://reviews.llvm.org/D37677



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

Is there a benchmark where this demonstrates some performance improvement?  I 
fear that the switch to condition_variable_any will swamp any performance gains 
from the switch to a spin lock.

Also, the spin lock is being held during allocating operations (the exception 
throws and at_thread_exit code).  That's a little long to be holding a spin 
lock.


https://reviews.llvm.org/D37677



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

LGTM.  You should probably get one other person to approve though.  I'm hoping 
that @EricWF or @mclow.lists will take a look


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

The test headers should not be in the production include folder.  They should 
probably be in the benchmark folder.

If possible, follow the style and conventions of the existing tests.  When you 
can't follow the style and convention of the existing tests, try to make it 
obvious (or leave a comment) as to why the new test is special.

For example, one way to follow the existing conventions would be to have a test 
that looks like this...

BENCHMARK_CAPTURE(BM_Sort, **qsort_worst_uint32**, 
**getQSortKiller**)->Arg(TestNumInputs);

That test would be called qsort_worst_uint32.  You would need to author the 
sequence generator so that it had a signature like this...
template  std::vector getQSortKiller(size_t N)

On an encouraging note, I don't see anything wrong with the production code.  
I'm optimistic about getting this in once we iron out the test issues.


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34170: [libcxx] Moving compiler specific test infrastructure to compiler.py

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D34170



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

If you want the performance improvements in the BM_sort_std_worst_quick cases 
preserved, you really need to port the benchmark from Aditya's repo into the 
libcxx benchmark code base.  Otherwise, the next person that comes along to 
improve std::sort performance may very well wreck the performance gains you 
achieved.


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

I like this change in general.  Dinkumware has been using introsort for 10+ 
years, so I'm a bit surprised that libc++ wasn't already.




Comment at: include/algorithm:4208
+
+  // Threshold(or depth limit) for introsort is taken to be 2*log2(size)
+  typedef typename iterator_traits<_RandomAccessIterator>::difference_type 
difference_type;

This comment says basically the same thing as the code.  The comment would be 
more useful if it said why 2*log2(size) is used.


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

Those are interesting (and useful) results... but they don't look like they 
came from the same algorithms.bench.cpp that I'm looking at...
https://github.com/llvm-mirror/libcxx/blob/master/benchmarks/algorithms.bench.cpp

That being said, the benchmark there only does 1k elements at a time, and 
doesn't have the worst case for quick sort like yours does.  Adding to the 
upstream algorithms.bench.cpp seems valuable.


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

alternatively, you could report the comparison of the old code vs. the new code 
with an existing benchmark, like benchmarks/algorithms.bench.cpp


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

This patch needs benchmarks that demonstrate the performance changes.


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D18174: Fix libcxx build on musl

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: include/__mutex_base:51
 #ifndef _LIBCPP_CXX03_LANG
-constexpr mutex() = default;
+#ifdef __GLIBC__
+constexpr

smeenai wrote:
> EricWF wrote:
> > Limiting `constexpr` to GLIBC implementations only is incorrect; you want 
> > to exclude MUSL.
> > 
> > Also MUSL is wrong for not allowing `pthread_mutex_t mut = 
> > PTHREAD_MUTEX_INITIALIZER` to be a constant expression, and MUSL should fix 
> > that.
> I've used `__builtin_constant_p` to wok around a similarly non-conforming 
> `PTHREAD_MUTEX_INITIALIZER` (from pthread-win32, which defines it to be 
> `(void *)-1`): see the last part of 
> https://stackoverflow.com/a/10376574/382079. It's a terrible hack, but it 
> works.
musl's pthread_mutex_t has volatile members if I recall correctly.  It's hard 
to validate that statement without building musl, because the header that 
defines pthread_mutex_t is code generated.


https://reviews.llvm.org/D18174



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34170: [libcxx] Moving compiler specific test infrastructure to compiler.py

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D34170



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35174: [libc++] Fix unrepresentable enum for clang-cl unstable ABI

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 105845.
bcraig edited the summary of this revision.
bcraig added a comment.

Switched to static consts


https://reviews.llvm.org/D35174

Files:
  include/string


Index: include/string
===
--- include/string
+++ include/string
@@ -676,11 +676,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -706,11 +706,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?


Index: include/string
===
--- include/string
+++ include/string
@@ -676,11 +676,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -706,11 +706,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35174: [libc++] Fix unrepresentable enum for clang-cl unstable ABI

[Ben Craig via Phabricator via cfe-commits]

bcraig created this revision.

When using LIBCXX_ABI_UNSTABLE=YES, clang-cl gave the following warning:

P:\llvm_master\src\llvm\projects\libcxx\include\string(683,51):
warning: enumerator value is not representable in the underlying type
'int' [-Wmicrosoft-enum-value]

Fixed by providing a sufficiently large representation, so long as C++11
strong enums are available.


https://reviews.llvm.org/D35174

Files:
  include/__config
  include/string


Index: include/string
===
--- include/string
+++ include/string
@@ -680,7 +680,7 @@
 enum {__long_mask  = 0x1ul};
 #else  // _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+enum _LIBCPP_UNDERLYING_ENUM_TYPE(size_type) {__long_mask  = 
~(size_type(~0) >> 1)};
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -707,7 +707,7 @@
 
 #if _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+enum _LIBCPP_UNDERLYING_ENUM_TYPE(size_type) {__long_mask  = 
~(size_type(~0) >> 1)};
 #else  // _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x01};
 enum {__long_mask  = 0x1ul};
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -849,9 +849,11 @@
 _LIBCPP_ALWAYS_INLINE explicit x(int __v) : __v_(static_cast<__lx>(__v)) 
{} \
 _LIBCPP_ALWAYS_INLINE operator int() const {return __v_;} \
 };
+#define _LIBCPP_UNDERLYING_ENUM_TYPE(t)
 #else  // _LIBCPP_HAS_NO_STRONG_ENUMS
 #define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_ENUM_VIS x
 #define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
+#define _LIBCPP_UNDERLYING_ENUM_TYPE(t) : t
 #endif  // _LIBCPP_HAS_NO_STRONG_ENUMS
 
 #ifdef _LIBCPP_DEBUG


Index: include/string
===
--- include/string
+++ include/string
@@ -680,7 +680,7 @@
 enum {__long_mask  = 0x1ul};
 #else  // _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+enum _LIBCPP_UNDERLYING_ENUM_TYPE(size_type) {__long_mask  = ~(size_type(~0) >> 1)};
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -707,7 +707,7 @@
 
 #if _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+enum _LIBCPP_UNDERLYING_ENUM_TYPE(size_type) {__long_mask  = ~(size_type(~0) >> 1)};
 #else  // _LIBCPP_BIG_ENDIAN
 enum {__short_mask = 0x01};
 enum {__long_mask  = 0x1ul};
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -849,9 +849,11 @@
 _LIBCPP_ALWAYS_INLINE explicit x(int __v) : __v_(static_cast<__lx>(__v)) {} \
 _LIBCPP_ALWAYS_INLINE operator int() const {return __v_;} \
 };
+#define _LIBCPP_UNDERLYING_ENUM_TYPE(t)
 #else  // _LIBCPP_HAS_NO_STRONG_ENUMS
 #define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_ENUM_VIS x
 #define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
+#define _LIBCPP_UNDERLYING_ENUM_TYPE(t) : t
 #endif  // _LIBCPP_HAS_NO_STRONG_ENUMS
 
 #ifdef _LIBCPP_DEBUG
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34170: [libcxx] Moving compiler specific test infrastructure to compiler.py

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 105268.
bcraig added a comment.

Rebased.
Separating out logging into it's own class.
Also tweaked the output slightly so that the language dialect under test shows 
up as the first line of output.


https://reviews.llvm.org/D34170

Files:
  test/lit.cfg
  utils/libcxx/compiler.py
  utils/libcxx/test/config.py
  utils/libcxx/test/format.py
  utils/libcxx/test/target_info.py
  utils/libcxx/util.py

Index: utils/libcxx/util.py
===
--- utils/libcxx/util.py
+++ utils/libcxx/util.py
@@ -16,7 +16,50 @@
 import sys
 import tempfile
 import threading
-
+import inspect
+
+class Logger:
+def __init__(self):
+self._notes = []
+self._flushing = False
+
+def _write_message(self, kind, message):
+# Get the file/line where this message was generated.
+f = inspect.currentframe()
+# Step out of _write_message, and then out of wrapper.
+f = f.f_back.f_back
+file,line,_,_,_ = inspect.getframeinfo(f)
+location = '%s:%d' % (file, line)
+
+sys.stderr.write('%s: %s: %s\n' % (location, kind, message))
+
+def note(self, message):
+sys.stderr.write('%s\n' % (message))
+
+def deferred_note(self, message):
+if self._flushing:
+self._write_message('note', message)
+else:
+self._notes.append(message)
+
+def flush_notes(self):
+for note in self._notes:
+self._write_message('note', note)
+_flushing = True
+
+def warning(self, message):
+self._write_message('warning', message)
+self.numWarnings += 1
+
+def error(self, message):
+self._write_message('error', message)
+self.numErrors += 1
+
+def fatal(self, message):
+self._write_message('fatal', message)
+sys.exit(2)
+
+lit_logger = Logger()
 
 # FIXME: Most of these functions are cribbed from LIT
 def to_bytes(str):
Index: utils/libcxx/test/target_info.py
===
--- utils/libcxx/test/target_info.py
+++ utils/libcxx/test/target_info.py
@@ -14,6 +14,7 @@
 import re
 import subprocess
 import sys
+from libcxx.util import lit_logger
 
 class DefaultTargetInfo(object):
 def __init__(self, full_config):
@@ -23,7 +24,7 @@
 return sys.platform.lower().strip()
 
 def add_locale_features(self, features):
-self.full_config.lit_config.warning(
+lit_logger.warning(
 "No locales entry for target_system: %s" % self.platform())
 
 def add_cxx_compile_flags(self, flags): pass
@@ -46,7 +47,7 @@
 locale.setlocale(locale.LC_ALL, default_locale)
 
 
-def add_common_locales(features, lit_config, is_windows=False):
+def add_common_locales(features, is_windows=False):
 # A list of locales needed by the test-suite.
 # The list uses the canonical name for the locale used in the test-suite
 # TODO: On Linux ISO8859 *may* needs to hyphenated.
@@ -63,7 +64,7 @@
 if test_locale(loc_name):
 features.add('locale.{0}'.format(loc_id))
 else:
-lit_config.warning('The locale {0} is not supported by '
+lit_logger.warning('The locale {0} is not supported by '
'your platform. Some tests will be '
'unsupported.'.format(loc_name))
 
@@ -92,7 +93,7 @@
 pass
 
 if not out:
-self.full_config.lit_config.fatal(
+lit_logger.fatal(
 "cannot infer sdk version with: %r" % cmd)
 
 return re.sub(r'.*/[^0-9]+([0-9.]+)\.sdk', r'\1', out)
@@ -119,7 +120,7 @@
 return (True, name, version)
 
 def add_locale_features(self, features):
-add_common_locales(features, self.full_config.lit_config)
+add_common_locales(features)
 
 def add_cxx_compile_flags(self, flags):
 if self.full_config.use_deployment:
@@ -134,7 +135,7 @@
 res = -1
 if res == 0 and out:
 sdk_path = out
-self.full_config.lit_config.note('using SDKROOT: %r' % sdk_path)
+lit_logger.deferred_note('using SDKROOT: %r' % sdk_path)
 flags += ["-isysroot", sdk_path]
 
 def add_cxx_link_flags(self, flags):
@@ -179,7 +180,7 @@
 super(FreeBSDLocalTI, self).__init__(full_config)
 
 def add_locale_features(self, features):
-add_common_locales(features, self.full_config.lit_config)
+add_common_locales(features)
 
 def add_cxx_link_flags(self, flags):
 flags += ['-lc', '-lm', '-lpthread', '-lgcc_s', '-lcxxrt']
@@ -203,7 +204,7 @@
 return ver # Permitted to be None.
 
 def add_locale_features(self, features):
-add_common_locales(features, self.full_config.lit_config)
+add_common_locales(features)
 # Some linux distributions have different locale data than others.
 # Insert

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

_LIBCPP_MS_CRT seems fine too.


https://reviews.llvm.org/D34588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

LGTM, but you should probably get approval from somebody a bit more senior with 
the project.




Comment at: include/__config:234-235
+// a MS compatibility version is specified.
 #  ifndef __MINGW32__
-#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library
+#ifdef _MSC_VER
+#  define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library

bruno wrote:
> bcraig wrote:
> > majnemer wrote:
> > > compnerd wrote:
> > > > smeenai wrote:
> > > > > You can combine this into just
> > > > > 
> > > > > ```
> > > > > #  if defined(_MSC_VER) && !defined(__MINGW32__)
> > > > > ```
> > > > > 
> > > > > I don't know if `__MINGW32__` and `_MSC_VER` will ever be compiled 
> > > > > simultaneously. (clang never defines `_MSC_VER` for its MinGW 
> > > > > triples, for example.)
> > > > What if MinGW is built with clang/c2 and MSVC extensions?  I think that 
> > > > the two could be defined together.  What about cygwin and clang/c2?  I 
> > > > guess we can ignore that since cygwin is not under active development.
> > > > 
> > > > I think this really goes back to my idea for an additional flag to 
> > > > indicate the C library in use.  We can interpret it from the 
> > > > canonicalized triple that LLVM/clang use.
> > > clang/c2 is dead.
> > At some point, I would like to see (or will need to introduce) a flag for 
> > which Windows C library is in use (so I'm agreeing with / echoing 
> > @compnerd).  What all options are there right now?  There's the Visual 
> > Studio C-runtime (multiple versions), there's msvcrt (used by the OS and 
> > mingw), there's the ancient crtdll that we shouldn't ever support, and 
> > there's the kernel C runtime (I'm probably the only person that cares about 
> > that).
> > 
> > I will note that I don't like the name of the macro here.  _LIBCPP_MSVCRT 
> > implies that msvcrt.dll is being used, when it isn't.  I don't think that 
> > this patch needs to fix that naming though.
> Any suggestion on a new name instead of `_LIBCPP_MSVCRT` for a future patch?
_LIBCPP_MS_UCRT? (UCRT for universal C-runtime).  That's more accurate than 
MSVCRT, thought it still isn't entirely accurate, as the UCRT is only a subset 
of the full CRT.

Other brainstormed names...
_LIBCPP_MS_VISUAL_STUDIO_CRT
_LIBCPP_MSVS_CRT
_LIBCPP_MSVC_CRT
_LIBCPP_THE_REAL_MSVC_CRT



https://reviews.llvm.org/D34588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: include/__config:234-235
+// a MS compatibility version is specified.
 #  ifndef __MINGW32__
-#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library
+#ifdef _MSC_VER
+#  define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library

majnemer wrote:
> compnerd wrote:
> > smeenai wrote:
> > > You can combine this into just
> > > 
> > > ```
> > > #  if defined(_MSC_VER) && !defined(__MINGW32__)
> > > ```
> > > 
> > > I don't know if `__MINGW32__` and `_MSC_VER` will ever be compiled 
> > > simultaneously. (clang never defines `_MSC_VER` for its MinGW triples, 
> > > for example.)
> > What if MinGW is built with clang/c2 and MSVC extensions?  I think that the 
> > two could be defined together.  What about cygwin and clang/c2?  I guess we 
> > can ignore that since cygwin is not under active development.
> > 
> > I think this really goes back to my idea for an additional flag to indicate 
> > the C library in use.  We can interpret it from the canonicalized triple 
> > that LLVM/clang use.
> clang/c2 is dead.
At some point, I would like to see (or will need to introduce) a flag for which 
Windows C library is in use (so I'm agreeing with / echoing @compnerd).  What 
all options are there right now?  There's the Visual Studio C-runtime (multiple 
versions), there's msvcrt (used by the OS and mingw), there's the ancient 
crtdll that we shouldn't ever support, and there's the kernel C runtime (I'm 
probably the only person that cares about that).

I will note that I don't like the name of the macro here.  _LIBCPP_MSVCRT 
implies that msvcrt.dll is being used, when it isn't.  I don't think that this 
patch needs to fix that naming though.


https://reviews.llvm.org/D34588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

ping


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20596: [libcxx] Refactor locale switching, creation, and destruction

[Ben Craig via Phabricator via cfe-commits]

bcraig abandoned this revision.
bcraig added a comment.

This is very stale at this point, and isn't blocking anything.  Closing.


https://reviews.llvm.org/D20596



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

[Ben Craig via Phabricator via cfe-commits]

bcraig accepted this revision.
bcraig added a comment.

LGTM.
@waltl : Do you need me to submit the changes, or will you handle that?


https://reviews.llvm.org/D32146



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34170: [libcxx] Moving compiler specific test infrastructure to compiler.py

[Ben Craig via Phabricator via cfe-commits]

bcraig created this revision.

format.py and config.py were routinely reaching into the innards of 
compiler.py, then setting variables in a very gcc / clang-centric way.  Now, 
all the compiler specific code has been moved to compiler.py.  These makes it 
possible to (in theory) build with MSVC instead, by switching to a different 
CXXCompilerInterface implementation.

This refactor did not attempt to make large, simplifying changes.  It is mainly 
moving code to the right place so that later work can consolidate and clean as 
needed.

As a proof of concept, I was able to switch out CXXCompilerInterface 
implementations and run the libc++ tests with MSVC 2017 and the MSVC 2017 STL, 
with 3486/5472 passing (I didn't dive into the failure cases).  The MSVC test 
infrastructure port is not included in this review.


https://reviews.llvm.org/D34170

Files:
  utils/libcxx/compiler.py
  utils/libcxx/test/config.py
  utils/libcxx/test/format.py

Index: utils/libcxx/test/format.py
===
--- utils/libcxx/test/format.py
+++ utils/libcxx/test/format.py
@@ -129,25 +129,11 @@
 extra_modules_defines = self._get_parser('MODULES_DEFINES:',
  parsers).getValue()
 if '-fmodules' in test.config.available_features:
-test_cxx.compile_flags += [('-D%s' % mdef.strip()) for
-   mdef in extra_modules_defines]
-test_cxx.addWarningFlagIfSupported('-Wno-macro-redefined')
-# FIXME: libc++ debug tests #define _LIBCPP_ASSERT to override it
-# If we see this we need to build the test against uniquely built
-# modules.
-if is_libcxx_test:
-with open(test.getSourcePath(), 'r') as f:
-contents = f.read()
-if '#define _LIBCPP_ASSERT' in contents:
-test_cxx.useModules(False)
+test_cxx.add_extra_module_defines(extra_modules_defines,
+  test.getSourcePath())
 
 if is_objcxx_test:
-test_cxx.source_lang = 'objective-c++'
-if is_objcxx_arc_test:
-test_cxx.compile_flags += ['-fobjc-arc']
-else:
-test_cxx.compile_flags += ['-fno-objc-arc']
-test_cxx.link_flags += ['-framework', 'Foundation']
+test_cxx.use_objcxx(is_objcxx_arc_test=is_objcxx_arc_test)
 
 # Dispatch the test based on its suffix.
 if is_sh_test:
@@ -231,17 +217,7 @@
'expected-error', 'expected-no-diagnostics']
 use_verify = self.use_verify_for_fail and \
  any([tag in contents for tag in verify_tags])
-# FIXME(EricWF): GCC 5 does not evaluate static assertions that
-# are dependant on a template parameter when '-fsyntax-only' is passed.
-# This is fixed in GCC 6. However for now we only pass "-fsyntax-only"
-# when using Clang.
-if test_cxx.type != 'gcc':
-test_cxx.flags += ['-fsyntax-only']
-if use_verify:
-test_cxx.useVerify()
-test_cxx.useWarnings()
-if '-Wuser-defined-warnings' in test_cxx.warning_flags:
-test_cxx.warning_flags += ['-Wno-error=user-defined-warnings']
+test_cxx.configure_for_fail_test(use_verify=use_verify)
 
 cmd, out, err, rc = test_cxx.compile(source_path, out=os.devnull)
 expected_rc = 0 if use_verify else 1
Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -13,12 +13,8 @@
 import pkgutil
 import pipes
 import re
-import shlex
-import shutil
 import sys
 
-from libcxx.compiler import CXXCompiler
-from libcxx.test.target_info import make_target_info
 from libcxx.test.executor import *
 from libcxx.test.tracing import *
 import libcxx.util
@@ -57,7 +53,6 @@
 self.config = config
 self.is_windows = platform.system() == 'Windows'
 self.cxx = None
-self.cxx_is_clang_cl = None
 self.cxx_stdlib_under_test = None
 self.project_obj_root = None
 self.libcxx_src_root = None
@@ -127,37 +122,28 @@
 self.configure_src_root()
 self.configure_obj_root()
 self.configure_cxx_stdlib_under_test()
-self.configure_cxx_library_root()
+self.cxx.configure_cxx_library_root(self)
 self.configure_use_clang_verify()
-self.configure_use_thread_safety()
+self.cxx.configure_use_thread_safety(self)
 self.configure_execute_external()
-self.configure_ccache()
-self.configure_compile_flags()
+self.cxx.configure_ccache(self)
+self.cxx.configure_compile_flags(self)
 self.configure_filesystem_compile_flags()
-self.configure_link_flags()
+self.cxx.c

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 100650.
bcraig edited the summary of this revision.

https://reviews.llvm.org/D32411

Files:
  CMakeLists.txt
  docs/DesignDocs/IncludeNextEmulation.rst
  include/__config
  include/__config_site.in
  include/complex.h
  include/cstddef
  include/ctype.h
  include/errno.h
  include/float.h
  include/inttypes.h
  include/limits.h
  include/locale.h
  include/math.h
  include/setjmp.h
  include/stdbool.h
  include/stddef.h
  include/stdint.h
  include/stdio.h
  include/stdlib.h
  include/string.h
  include/wchar.h
  include/wctype.h
  utils/libcxx/test/config.py

Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -632,6 +632,8 @@
 for m in feature_macros:
 if m == '_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS':
 continue
+if m.startswith('_LIBCPP_INCLUDE_NEXT'):
+continue
 if m == '_LIBCPP_ABI_VERSION':
 self.config.available_features.add('libcpp-abi-version-v%s'
 % feature_macros[m])
Index: include/wctype.h
===
--- include/wctype.h
+++ include/wctype.h
@@ -51,7 +51,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wctype.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/wchar.h
===
--- include/wchar.h
+++ include/wchar.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wchar.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_WCHAR_H)
 #define _LIBCPP_WCHAR_H
@@ -116,7 +120,11 @@
 #define __CORRECT_ISO_CPP_WCHAR_H_PROTO
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(wchar.h)
+#else
 #include_next 
+#endif
 
 // Determine whether we have const-correct overloads for wcschr and friends.
 #if defined(_WCHAR_H_CPLUSPLUS_98_CONFORMANCE_)
Index: include/string.h
===
--- include/string.h
+++ include/string.h
@@ -58,7 +58,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(string.h)
+#else
 #include_next 
+#endif
 
 // MSVCRT, GNU libc and its derivates may already have the correct prototype in
 // . This macro can be defined by users if their C library provides
Index: include/stdlib.h
===
--- include/stdlib.h
+++ include/stdlib.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDLIB_H)
 #define _LIBCPP_STDLIB_H
@@ -91,7 +95,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdio.h
===
--- include/stdio.h
+++ include/stdio.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdio.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDIO_H)
 #define _LIBCPP_STDIO_H
@@ -105,7 +109,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stdio.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdint.h
===
--- include/stdint.h
+++ include/stdint.h
@@ -116,6 +116,10 @@
 #   define __STDC_CONSTANT_MACROS
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_VC(stdint.h)
+#else
 #include_next 
+#endif
 
 #endif  // _LIBCPP_STDINT_H
Index: include/stddef.h
===
--- include/stddef.h
+++ include/stddef.h
@@ -15,7 +15,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stddef.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDDEF_H)
 #define _LIBCPP_STDDEF_H
@@ -43,7 +47,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_UCRT(stddef.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdbool.h
===
--- include/stdbool.h
+++ include/stdbool.h
@@ -26,7 +26,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT_VC(stdbool.h)
+#else
 #include_next 
+#endif

[PATCH] D33082: Fix Libc++ build with MinGW64

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

> Are you suggesting that libc++ should stop declaring/defining these 
> functions, at least publically?

I think that we generally shouldn't be giving functions names that are already 
claimed elsewhere (like mbsnrtowcs and wcsnrtombs).  It is my opinion that 
these should always be qualified as __libcpp_* symbols.  We can easily run into 
trouble if users also defined mbsnrtowcs.  They have just as much of a right to 
do so as we do.

That being said, I think that's a change for a different changelist.


https://reviews.llvm.org/D33082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: include/support/newlib/xlocale.h:20
+#if defined(__NEWLIB__) && (__NEWLIB__ == 2) \
+&& defined(__NEWLIB_MINOR__) && (__NEWLIB_MINOR__ >= 5) \
+&& (!defined(__POSIX_VISIBLE) || (__POSIX_VISIBLE < 200809))

orivej wrote:
> You meant `__NEWLIB_MINOR__ < 5`.
> Could not this be just the following?
> ```
> #if __NEWLIB__ < 2 || __NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
> #include 
> #endif
> ```
I suspect you are right.  Here's the trouble though... I don't have a good way 
to test this.  I spent a couple of days trying to get newlib building on my 
machine, and I ended up giving up.  So I just took Martin J. O'Riordan's patch 
from here:
http://clang-developers.42468.n3.nabble.com/Re-Newlib-v2-5-0-and-LibC-locale-support-BROKEN-tt4054882.html

Are you in a good position to test this change?  Bonus points if you can try 
newlib 2.4 and 2.5.


https://reviews.llvm.org/D32146



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

> I noticed that the Windows STL headers have to do this dance with `new` (even 
> though they do `(foo)(...)` for `min` and `max`). If we're going to need
>  to guard against a bunch of macros I would like to use a single approach.  
> Other than updating the `#if defined(min) || defined(max)` lines it's trivial 
> to guard
>  against additional macros.

I think the guarding of 'new' is done because of user code, not because of 
headers #defining new.  There is a lot of old Windows code out there that would 
define 'new' to something else in order to get extra instrumentation.  I want 
to say this was even done in some of the MFC example 'project templates'

One of the things I don't like about push / pop macro is that it isn't the same 
as doing nothing with the macro.  If something between the push and pop 
specifically wanted to use or modify the macro, the pop macro will destroy that 
state.  Granted, in this situation, that seems both unlikely, and it would be 
evil heaped upon evil.

With push and pop macro, you also need to be very careful about how things are 
positioned.  The push and undef need to happen after all the other includes in 
the header are processed, and care needs to be taken not to include anything 
offensive within the bounds of the push and pop.  You also need to remember to 
do the pop.

push and pop macro are acceptable.  I still have a preference for the 
parenthesis approach though.




Comment at: include/shared_mutex:128-136
+_LIBCPP_PUSH_MACROS
+#if defined(min) || defined(max)
+# include <__undef_macros>
+#endif
+
+
 #if _LIBCPP_STD_VER > 11 || defined(_LIBCPP_BUILDING_SHARED_MUTEX)

Shouldn't the push macro be somewhere below this potential include?


https://reviews.llvm.org/D33080



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33082: Fix Libc++ build with MinGW64

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

I agree that we need to be precise in our platform detection macros.  On 
Windows, we currently need to worry about which compiler is being used, whether 
we are targeting the mingw environment or the native (?) environment, and which 
c-runtime is being used.

For my msvc-kernel work, I'll need some amount of platform detection macros as 
well (I'm not expecting those to be added in this review).  I believe I will 
represent that with some combination of a "freestanding" macro, a different 
name for the CRT, and detection of the MSVC compiler, but I haven't gotten that 
far yet.


https://reviews.llvm.org/D33082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33080: [Libc++] Use #pragma push_macro/pop_macro to better handle min/max on Windows

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

I like the warning that you generate for min and max macros existing.  Is the 
push_macro / pop_macro the right way to go though?  You could throw extra 
parenthesis around the declaration and usages of min/max to avoid macro 
expansion.

  #define add(x,y) (x)-(y)
  
  template 
  T (add)(T x, T y) // parens around add to suppress macro
  {
  return x+y;
  }
  
  int main() {
  return 
  (add)(5, 4) + // uses function
  add(5, 4);// uses macro
  }


https://reviews.llvm.org/D33080



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32988: [libc++] Refactor Windows support headers.

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

LGTM


https://reviews.llvm.org/D32988



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32988: [libc++] Refactor Windows support headers.

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: include/__config:232-235
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+

I can see this helping when we are build libc++, but I don't think it helps 
client apps.  They could have included windows.h before including our headers.

Don't get me wrong, I dislike the min and max macros, and bear no hard feelings 
towards people that define this project wide on the command line, I'm just not 
sure it will get things done right here.

In the past, I've just surrounded my min and max declarations with parenthesis 
to suppress macro expansion.


https://reviews.llvm.org/D32988



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32927: [libc++] Implement exception_ptr on Windows

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

Getting the test suite green sooner rather than later seems like a good reason 
to temporarily pick a 1-3 year solution rather than a 5+ year solution.  Also, 
as you mention, this isn't all throw away work, so it's still progress.

So yeah, this is fine to go in as is.  LGTM.


https://reviews.llvm.org/D32927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32927: [libc++] Implement exception_ptr on Windows

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

libstdc++ and the Visual Studio C++ runtime have very different compatibility 
expectations.

libstdc++ is generally bundled with the operating system.  It maintains binary 
compatibility over very long stretches of time.  In most systems, there can 
only be one libstdc++ loaded in a process at a time.  Throwing a std::exception 
in one shared library and catching it in another generally works, even if the 
two libraries were built with different compilers.  So pretty similar to libc++ 
on Mac.  This is almost certainly not news to you.

The Visual Studio C++ runtime is not bundled with the operating system.  It 
generally does not maintain binary compatibility over more than a major 
release.  Having multiple versions of the Visual Studio C++ runtime loaded in 
the same process is pretty common.  Throwing a std::exception in one DLL and 
catching it in another will generally won't work if the two libraries were 
built with different compilers.

When you code against libstdc++ internals, your code will likely last 5+ years 
before it breaks, and when it breaks it would likely be considered a bug.  When 
you code against Visual Studio C++ internals, your code has a shelf life of 1-3 
years, and then the code has a high probability of breaking.

I think the seh internals are more stable than the ExceptionPtr helper 
functions.  One thing that works reasonably well is throwing an exception that 
doesn't derive from a CRT type in one DLL, and then catching it in another, 
even with different compiler versions.  "catch(...)" also works reasonably well 
across DLL / compiler version boundaries.

Is there a particular reason to go with a shorter term solution now, rather 
than straight to the long term solution?  Is there a deadline or a particular 
milestone we are trying to hit for clang-cl support?  Or is there a suspicion 
that future changes will make a really good implementation possible, 
incentivizing us to do something inexpensive for now?


https://reviews.llvm.org/D32927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32927: [libc++] Implement exception_ptr on Windows

[Ben Craig via Phabricator via cfe-commits]

bcraig added inline comments.



Comment at: 
test/std/language.support/support.exception/except.nested/rethrow_if_nested.pass.cpp:12
+
+// This test fails due to a stack overflow
 // XFAIL: LIBCXX-WINDOWS-FIXME

EricWF wrote:
> BillyONeal wrote:
> > FWIW it does not pass for our implementation either.
> > 
> > Note that unlike the Itanium ABI, the stack is not popped back to the catch 
> > block when handling an exception (because SEH can act like a signal handler 
> > and say EXCEPTION_CONTINUE_EXECUTION).
> Thanks for pointing that out! Otherwise I may have gone down a rabbit hole 
> trying to figure it out.
Also means that MSABI doesn't need to allocate heap memory in a failure case.  
That always bugged me about the Itanium ABI.  In "the real world", I don't know 
if stack overflows are any better, but at least, in theory, maximum stack usage 
could be calculated by a determined enough programmer.



Comment at: 
test/std/language.support/support.exception/propagation/current_exception.pass.cpp:10-11
 
-// exception_ptr has not been implemented on Windows
+// This test needs to be rewritten for the Windows exception_ptr semantics
+// which copy the exception each time the exception_ptr is copied.
 // XFAIL: LIBCXX-WINDOWS-FIXME

EricWF wrote:
> EricWF wrote:
> > BillyONeal wrote:
> > > We don't copy the exception each time the exception_ptr is copied -- 
> > > exception_ptr models shared_ptr. See: "C:\Program Files (x86)\Microsoft 
> > > Visual 
> > > Studio\2017\Enterprise\VC\Tools\MSVC\14.10.25017\crt\src\stl\excptptr.cpp"
> > > 
> > > We do make a copy when making the initial exception_ptr.
> > Ah thanks for the clarification. So `std::current_exception()` always 
> > returns a different copy.
> >  exception_ptr models shared_ptr. 
> 
> Oh fudge me, it not only models it but it actually uses MSVC's `shared_ptr` 
> implementation. This seems so much sketchier now.
You may want to pursue your long term plan instead.

Rather than use the msvcprt functions, dig into the underlying SEH structures, 
find the pointer to the exception, and work from there.

Headers in the older crt source have a lot of useful information, and that may 
be useful enough to get things going.

The blog mentions where to fine one of those useful headers (ehdata.h):
http://workblog.pilin.name/2014/03/decoding-parameters-of-thrown-c.html
"Welcome - ehdata.h. This header appears only in VS2012 and VS2013 (see 
"%Program Files (x86)%\Microsoft Visual Studio 12.0\VC\crt\src\ehdata.h"):"

The other useful header is mtdll.h.  I found it in my local msvc 9.0 install, 
but didn't find it in my msvc-14.0 install.  I don't have 10.0, 11.0, or 12.0 
installed locally right now to see if mtdll.h is present in any of those.  
mtdll.h describes the layout of _tiddata, a thread local data store for the 
CRT.  The interesting fields there are void *_curexception;  and void 
*_curcontext;.



https://reviews.llvm.org/D32927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

I'm still working on this.

Things I've learned: A clang install is bundled with re-implementations of 
several of these headers.  The default #include_next behavior ends up pulling 
in lots of those.

I have been able to get things working with a VC include path and a UCRT 
include path, but the default include paths set by the compiler driver cause 
the clang versions of stdarg.h and vadefs.h to get pulled in.  The combination 
of clang include path and ucrt include path also works.

I did end up running into the same errno problems that were mentioned earlier, 
and ended up coming to the same workaround.


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig added a comment.

In https://reviews.llvm.org/D32411#735128, @halyavin wrote:

> BTW, the list of include files which are located in [PROGRAM_FILES]\Microsoft 
> Visual Studio 14.0\VC\include directory is
>
> - stdbool.h
> - limits.h
> - stdint.h
> - setjmp.h
>
>   The rest is in [PROGRAM_FILES]\Windows Kits\10\Include\10.0.whatever.0\ucrt 
> directory. Which directory @_LIBCPP_INCLUDE_NEXT@ is supposed to point to?


My immediate goal is getting a sort-of freestanding implementation of libc++ to 
work in the Windows kernel.  That means that I have _LIBCPP_INCLUDE_NEXT 
defined as ../../km/crt.

I'll have to stew on what I want the solution to this to be.  I don't feel bad 
at all switching between one compiler extension (#include_next) and another 
(computed includes).  I would feel a little bad cooking in more knowledge of 
the MSVC directory layout though.  Maybe that's just the cost to pay though.  
There is some pretty deep knowledge of glibc headers in libc++, so it wouldn't 
be terribly unusual to have similar knowledge of the MSVC CRT.

The errno changes you are suggesting may be fine, but I wasn't planning on 
addressing them with this patch.


https://reviews.llvm.org/D32411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Ben Craig via Phabricator via cfe-commits]

bcraig created this revision.
Herald added a subscriber: mgorny.

Visual Studio 2015 and 2017 don't implement include_next, so we'll use a 
combination of a computed include and a CMAKE input to make it work.  Also, 
retrofit all the existing invocations of #include_next that we could hit in a 
hypothetical MSVC build.

This relies on implementation defined behavior in the MSVC preprocessor.
See C11, 6.10.2.4 ("Source file inclusion") for the statement on implementation 
defined vs. undefined.


https://reviews.llvm.org/D32411

Files:
  CMakeLists.txt
  include/__config
  include/__config_site.in
  include/complex.h
  include/cstddef
  include/ctype.h
  include/errno.h
  include/float.h
  include/inttypes.h
  include/limits.h
  include/locale.h
  include/math.h
  include/setjmp.h
  include/stdbool.h
  include/stddef.h
  include/stdint.h
  include/stdio.h
  include/stdlib.h
  include/string.h
  include/wchar.h
  include/wctype.h

Index: include/wctype.h
===
--- include/wctype.h
+++ include/wctype.h
@@ -51,7 +51,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(wctype.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/wchar.h
===
--- include/wchar.h
+++ include/wchar.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(wchar.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_WCHAR_H)
 #define _LIBCPP_WCHAR_H
@@ -116,7 +120,11 @@
 #define __CORRECT_ISO_CPP_WCHAR_H_PROTO
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(wchar.h)
+#else
 #include_next 
+#endif
 
 // Determine whether we have const-correct overloads for wcschr and friends.
 #if defined(_WCHAR_H_CPLUSPLUS_98_CONFORMANCE_)
Index: include/string.h
===
--- include/string.h
+++ include/string.h
@@ -58,7 +58,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(string.h)
+#else
 #include_next 
+#endif
 
 // MSVCRT, GNU libc and its derivates may already have the correct prototype in
 // . This macro can be defined by users if their C library provides
Index: include/stdlib.h
===
--- include/stdlib.h
+++ include/stdlib.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDLIB_H)
 #define _LIBCPP_STDLIB_H
@@ -91,7 +95,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdlib.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdio.h
===
--- include/stdio.h
+++ include/stdio.h
@@ -14,7 +14,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdio.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDIO_H)
 #define _LIBCPP_STDIO_H
@@ -105,7 +109,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdio.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdint.h
===
--- include/stdint.h
+++ include/stdint.h
@@ -116,6 +116,10 @@
 #   define __STDC_CONSTANT_MACROS
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdint.h)
+#else
 #include_next 
+#endif
 
 #endif  // _LIBCPP_STDINT_H
Index: include/stddef.h
===
--- include/stddef.h
+++ include/stddef.h
@@ -15,7 +15,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stddef.h)
+#else
 #include_next 
+#endif
 
 #elif !defined(_LIBCPP_STDDEF_H)
 #define _LIBCPP_STDDEF_H
@@ -43,7 +47,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stddef.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 
Index: include/stdbool.h
===
--- include/stdbool.h
+++ include/stdbool.h
@@ -26,7 +26,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#include _LIBCPP_INCLUDE_NEXT(stdbool.h)
+#else
 #include_next 
+#endif
 
 #ifdef __cplusplus
 #undef bool
Index: include/setjmp.h
===
--- include/setjmp.h
+++ include/setjmp.h
@@ -32,7 +32,11 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_HAS_NO_INCLUDE_NEXT)
+#i

[PATCH] D32147: [PR32479] Avoid newlib vasprintf, since it requires gnu extensions

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 95520.
bcraig added reviewers: jyknight, mclow.lists, EricWF.
bcraig added a subscriber: cfe-commits.

https://reviews.llvm.org/D32147

Files:
  include/__bsd_locale_fallbacks.h
  include/__config


Index: include/__config
===
--- include/__config
+++ include/__config
@@ -871,6 +871,10 @@
 #endif
 #endif
 
+#if defined(_NEWLIB_VERSION)
+#define _LIBCPP_HAS_NO_ASPRINTF 1
+#endif
+
 #ifdef __FreeBSD__
 #define _DECLARE_C99_LDBL_MATH 1
 #endif
Index: include/__bsd_locale_fallbacks.h
===
--- include/__bsd_locale_fallbacks.h
+++ include/__bsd_locale_fallbacks.h
@@ -118,7 +118,26 @@
 va_list __va;
 va_start(__va, __format);
 __locale_raii __current( uselocale(__l), uselocale );
+#if _LIBCPP_HAS_NO_ASPRINTF
+va_list __va2;
+va_copy(__va2, __va);
+int __res = vsnprintf(0, 0, __format, __va2);
+va_end(__va2);
+if(__res < 0)
+{
+va_end(__va);
+return __res;
+}
+*__s = (char *)malloc(__res+1);
+if(*__s == 0)
+{
+va_end(__va);
+return -1;
+}
+vsnprintf(*__s, __res+1, __format, __va);
+#else
 int __res = vasprintf(__s, __format, __va);
+#endif
 va_end(__va);
 return __res;
 }


Index: include/__config
===
--- include/__config
+++ include/__config
@@ -871,6 +871,10 @@
 #endif
 #endif
 
+#if defined(_NEWLIB_VERSION)
+#define _LIBCPP_HAS_NO_ASPRINTF 1
+#endif
+
 #ifdef __FreeBSD__
 #define _DECLARE_C99_LDBL_MATH 1
 #endif
Index: include/__bsd_locale_fallbacks.h
===
--- include/__bsd_locale_fallbacks.h
+++ include/__bsd_locale_fallbacks.h
@@ -118,7 +118,26 @@
 va_list __va;
 va_start(__va, __format);
 __locale_raii __current( uselocale(__l), uselocale );
+#if _LIBCPP_HAS_NO_ASPRINTF
+va_list __va2;
+va_copy(__va2, __va);
+int __res = vsnprintf(0, 0, __format, __va2);
+va_end(__va2);
+if(__res < 0)
+{
+va_end(__va);
+return __res;
+}
+*__s = (char *)malloc(__res+1);
+if(*__s == 0)
+{
+va_end(__va);
+return -1;
+}
+vsnprintf(*__s, __res+1, __format, __va);
+#else
 int __res = vasprintf(__s, __format, __va);
+#endif
 va_end(__va);
 return __res;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

[Ben Craig via Phabricator via cfe-commits]

bcraig created this revision.

newlib 2.5 added the locale management functions, and so our stub 
__nop_locale_mgmt.h shouldn't be included, as it conflicts with newlibs 
definitions.  newlib 2.4 and earlier still need the header though.

Patch by Martin J. O'Riordan


https://reviews.llvm.org/D32146

Files:
  include/support/newlib/xlocale.h


Index: include/support/newlib/xlocale.h
===
--- include/support/newlib/xlocale.h
+++ include/support/newlib/xlocale.h
@@ -16,7 +16,11 @@
 #include 
 #include 
 #include 
+#if defined(__NEWLIB__) && (__NEWLIB__ == 2) \
+&& defined(__NEWLIB_MINOR__) && (__NEWLIB_MINOR__ >= 5) \
+&& (!defined(__POSIX_VISIBLE) || (__POSIX_VISIBLE < 200809))
 #include 
+#endif
 #include 
 #include 
 


Index: include/support/newlib/xlocale.h
===
--- include/support/newlib/xlocale.h
+++ include/support/newlib/xlocale.h
@@ -16,7 +16,11 @@
 #include 
 #include 
 #include 
+#if defined(__NEWLIB__) && (__NEWLIB__ == 2) \
+&& defined(__NEWLIB_MINOR__) && (__NEWLIB_MINOR__ >= 5) \
+&& (!defined(__POSIX_VISIBLE) || (__POSIX_VISIBLE < 200809))
 #include 
+#endif
 #include 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits