ldionne added a subscriber: jwakely.
ldionne added inline comments.

================
Comment at: libcxx/include/new:243
 # ifdef _LIBCPP_HAS_NO_BUILTIN_OVERLOADED_OPERATOR_NEW_DELETE
     return ::operator new(__size, __align_val);
 # else
----------------
This breaks GCC (as of GCC 9). I don't know what mechanism GCC uses to tie into 
constexpr allocation, so I don't know what the fix is.

@jwakely Can you throw some hints at me?


================
Comment at: libcxx/include/version:254
 // # define __cpp_lib_concepts                             201806L
+# define __cpp_lib_constexpr_dynamic_alloc              201907L
 // # define __cpp_lib_constexpr_misc                       201811L
----------------
rsmith wrote:
> ldionne wrote:
> > rsmith wrote:
> > > ldionne wrote:
> > > > rsmith wrote:
> > > > > Should this be conditioned on compiler support being available?
> > > > So.. I've decided not to do that in this patch so far.
> > > > 
> > > > The support for constexpr allocation was checked into Clang about a 
> > > > year ago, right? I actually expect this to be a slightly contentious 
> > > > point, but I'd like to assume that we're using a reasonably recent 
> > > > Clang. I don't see a strong point for being able to use new libc++ 
> > > > headers with an old Clang anyway, since vendors usually release the two 
> > > > together. IOW, supporting this would add complexity for virtually no 
> > > > benefit. I do agree it's a slightly more aggressive stance than we've 
> > > > had so far, but this sort of reasonable assumption makes it so much 
> > > > easier to write stuff for libc++.
> > > OK, just a few thoughts then I'm going to bow out of this; this seems 
> > > like a policy decision for the libc++ maintainers to make.
> > > 
> > > In favor of dropping support for new libc++ + old clang: we generally 
> > > don't permit version skew between different components of LLVM. It seems 
> > > reasonable to expect all wanted parts of a particular LLVM release to be 
> > > built together.
> > > 
> > > Against dropping support for new libc++ + old clang: we do support 
> > > installing more than one version of LLVM (and in particular more than one 
> > > version of Clang) on the same system, but because libc++ defaults to 
> > > being installed in `/usr/include/c++/v1`, we don't seem to encourage 
> > > installing more than one version of libc++, so -- even assuming we only 
> > > support the *newest* version of libc++ going into `/usr/include/c++/v1` 
> > > -- new versions of libc++ need to work with old versions of Clang.
> > > 
> > > I think (largely by accident) Clang will prefer a libc++ installed into 
> > > `/usr/lib/clang/$VER/include` over one from `/usr/include/c++/v1`. If we 
> > > switched to installing libc++ there, I don't see any technical barrier to 
> > > version-locking them, though I'm not sure what story that leaves for use 
> > > of libc++ with GCC and other compilers. It seems worth noting that this 
> > > is exactly what libstdc++ does in order to need to support only one 
> > > version of GCC.
> > > OK, just a few thoughts then I'm going to bow out of this; this seems 
> > > like a policy decision for the libc++ maintainers to make.
> > 
> > Hear hear!
> > 
> > 
> > > I think (largely by accident) Clang will prefer a libc++ installed into 
> > > `/usr/lib/clang/$VER/include` over one from `/usr/include/c++/v1`. If we 
> > > switched to installing libc++ there, I don't see any technical barrier to 
> > > version-locking them, though I'm not sure what story that leaves for use 
> > > of libc++ with GCC and other compilers. It seems worth noting that this 
> > > is exactly what libstdc++ does in order to need to support only one 
> > > version of GCC.
> > 
> > I think it would be great to do that. Honestly, this is a huge 
> > simplification and makes implementing new features much easier. Also, I 
> > think it's reasonable to support not-trunk compilers, like 6 months old or 
> > something like that. But not several years back.
> > 
> > One last question: do you know what controls where the libc++ headers are 
> > installed as part of the LLVM distribution?
> Install paths are set in `libcxx/include/CMakeLists.txt` for the headers and 
> in `libcxx/src/CMakeLists.txt` for the libraries (search for `install(`) 
> based on cmake variables LIBCXX_INSTALL_HEADER_PREFIX, LIBCXX_INSTALL_PREFIX, 
> and LIBCXX_INSTALL_LIBRARY_DIR.
> 
> It would probably make sense to put the libc++ headers into somewhere like 
> `/usr/lib/clang/$VER/include/c++` instead of directly in 
> `/usr/lib/clang/$VER/include`; that'll need some changes to the clang driver 
> to make sure we look there. But of course that's doable if the two are 
> version-locked :)
Oh, I thought it was more complicated than that for the LLVM distribution. Ok, 
thanks for the info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68364

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

Reply via email to