[cfe-dev] [libc++] RFC: Add Windows kernel support and partial MSVC 2015 support

[Billy O'Neal (VC LIBS) via cfe-dev]

> MSVC 2015 doesn't support them either.

MSVC added variable templates in 2015 Update 2.

From: Ben Craig<mailto:ben.craig at ni.com>
Sent: Wednesday, March 29, 2017 3:38 PM
To: Eric Fiselier<mailto:eric at efcs.ca>
Cc: Billy O'Neal (VC LIBS)<mailto:bion at microsoft.com>; via cfe-dev<mailto:cfe-dev at lists.llvm.org>; mclow.lists at gmail.com<mailto:mclow.lists at gmail.com>; Stephan T. Lavavej<mailto:stl at exchange.microsoft.com>; Casey at Carter.net<mailto:Casey at carter.net>
Subject: RE: [libc++] RFC: Add Windows kernel support and partial MSVC 2015 support

I can definitely see maintaining a restricted include set to be burdensome  I will have to think on how to address that in a reasonable way.  Your clang-tidy suggestion may be useful here.
I don't think maintaining a restricted use of features will be particularly hard though.  For <array>, <tuple>, <utility>, and <algorithm>, do you foresee changes where an existing function doesn't throw, but starts to throw?  Or doesn't currently allocate, but starts to allocate?  If we were talking about the containers, or iostreams, or regex, I would definitely agree that the implementation flexibility is needed.  I'm not sure I agree when it's a smaller list of low level headers though.

I agree with your examples. Existing functions are unlikely to suddenly require exceptions or allocations. However it is conceivable that <tuple> or <utility> could require some internals, which themselves don't allocate or throw, that were previously only used by restricted features, or that live in restricted headers. For example libc++ often uses unique_ptr as a scope guard, but the class is primarily used to deal with dynamically memory and likely wouldn't be a part of freestanding mode. Forbidding its use in freestanding features could quickly become problematic. (Note: <algorithm> already widely depends on unique_ptr for this purpose).

I posit that the implementation requires the general freedom to use classes not supported in freestanding mode. We'll likely be able to strike a balance between what the implementation needs and what freestanding users expect, but there needs to be leeway; Leeway that restricting entire headers in freestanding mode does not provide.

Unique_ptr is a strange beast for exactly the reasons you describe.  I’m tempted to include it in a freestanding definition, but it would definitely feel odd pulling it along, but not taking make_unique, and not allowing the default deleter.  I think I also agree that restricting by header is too blunt of an instrument to validate the freestanding restrictions.

> I think it would be much better to get as much of libc++ compiling as possible, even if it depends on restricted
> features, and then finding another mechanism to restrict the features end-users are allowed to use (Such as clang-tidy).
> This would eliminate the need to restructure headers or spam out internal #ifdef guards.

One thing I want to avoid is turning straightforward mistakes into errors that are even more cryptic than what C++ users have grown accustomed to.  I don't really want the failure condition of inplace_merge to be a linker error complaining about a missing operator new.  clang-tidy might help with that.  #ifdefs are a straightforward way to address that concern, though it requires a substantial amount of effort.  Would you be opposed to widespread static_asserts?

What kind of static asserts? Can you provide an example?
What I had in mind here was for the first line of inplace_merge (and other allocating algorithms) to be something along the lines of static_assert(!_LIBCPP_HAS_NO_ALLOC).  I think you had a much better suggestion though…

>> * I'll also need to figure out how to not drag along unwanted header dependencies.

> I don't see how libc++ could upstream changes since it requires every header
> it currently includes (modulo bugs).
The straightforward (and labor intensive) way is with #ifdef _LIBCPP_HAS_NO_ALLOC


_LIBCPP_HAS_NO_ALLOC is exactly the invasive change I wanted to avoid. First <new> is currently a freestanding header,
and I think it should stay that way. Second I want to avoid the maintenance cost of such a restrictive
configuration. This macro would have to guard the the vast majority of the library, unlike
_LIBCPP_HAS_EXCEPTIONS and _LIBCPP_NO_RTTI which have a much smaller and more manageable scope.

If the end goal is simply to diagnose when end users use features that dynamically allocate I suspect there
are less invasive ways to do it. First there are two important things to note:

(1) Libc++ has very few naked new/delete calls. Almost all dynamic allocations go through std::allocator.
(2) std::allocator is typically used in dependent contexts (templates).

Instead of #ifdef'ing out std::allocator and everything that uses it, we could make instantiating std::allocator::allocate
produce a diagnostic. This would make the changes needed within libc++ significantly smaller while still
producing reasonable diagnostics when users attempt to use those types.

Putting a static assert (or some other kind of diagnostic) in std::allocator::allocate seems like an excellent idea to me.  I’d also want to hunt down the other places where raw new is used, but I think this approach has a lot of promise.  We can document what we officially support, but say that other things might work.  If someone instantiates code that hits std::allocator, they will get a static assert.  If they write their own code that calls new or delete, they will get a linker error (in the Windows Kernel at least).  If they write their own allocator that uses ExAllocatePoolWithTag, or malloc, or whatnot, then that’s fine, they will just need to be super careful that they don’t shoot themselves in the foot with std::list gotchas.

> Using hard-coded kernel paths is not a change I see being upstream-able. However
> we might be able to convince MSVC to implement #include_next if we can provide
> strong enough rational for why we need it.
These wouldn't be hard coded paths.  The code would look something like...
#if __LIBCPP_HAS_INCLUDE_NEXT
#include_next <limits.h>
#else
#include __LIBCPP_HACK_INCLUDE_NEXT(limits.h)
#endif
__config would have the definition of __LIBCPP_HACK_INCLUDE_NEXT, and
config_site.in<http://config_site.in> would have the relative path needed to glue everything together ("../../km/crt" in my case).

That sounds like a fairly reasonable solution. Although aren't macro expansions inside #include directives UB?

The GCC docs claim it is implementation-defined behavior ( https://gcc.gnu.org/onlinedocs/cpp/Computed-Includes.html ).  I haven’t done the research in the spec to know for sure.  STLPort does it though, and the computed include hackery there seems to work fine for lots of versions of GCC, Clang, MSVC, and I think even Intel’s compiler.


> I'm violently against working around MSVC template bugs. Every time I've seen
> it done it makes the implementation worse.
That's fair.  I will say that not _all_ of the workarounds make worse code,
but quite a few do.  This may end up being what forces me to MSVC 2017.

One example of a workaround that I don't think made things worse...
struct _LIBCPP_TYPE_VIS __check_tuple_constructor_fail {
     template <class ...>
-    static constexpr bool __enable_default() { return false; }
+    static constexpr bool __enable_default = false;

Your example uses C++14 variable templates, which I don't think you intended because MSVC 2015 doesn't support
them either.

Ironically that's a perfect example of a change that would make things drasticly worse.
First the suggested fix cannot be applied because variable templates are C++14 features.
Second we couldn't use a non-template in this case because it's important that the result is
type dependent in the non-fail cases to prevent eager SFINAE evaluation.

The <tuple> SFINAE is very complicated and it's full of subtle but important details.
It can barely manage it with a fully conforming C++11 compiler.

I almost didn’t include an example at all, because this was going to be the inevitable outcome, especially since I picked something involving std::tuple.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170329/c83d5d0b/attachment.html>