[PATCH] D46593: Allow copy elision in path concatenation

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

@chandlerc thanks for catching this.

As the original author I agree to contribute this patch to libc++ under the 
terms of the MIT and the University of Illinois licences, and under the terms 
of "Apache 2 with LLVM exception" if necessary in future.

This permission applies only to this patch, not any other code from GCC.


Repository:
  rCXX libc++

https://reviews.llvm.org/D46593



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

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

The simplest solution would be to change libstdc++'s `` to:

  #if __cplusplus >= 201703L && __cpp_impl_destroying_delete
  #define __cpp_lib_destroying_delete 201806L

Or we could be fancier and do:

  #if __cplusplus >= 201103L && __cpp_impl_destroying_delete
  namespace std
  {
struct destroying_delete_t
{
  explicit destroying_delete_t() = default;
};
  #if __cplusplus >= 201703L
  # define __cpp_lib_destroying_delete 201806L
inline constexpr destroying_delete_t destroying_delete{};
  #endif
  }
  #endif // destroying delete

i.e. define the type for C++11 and later, and the inline variable (and 
feature-test macro) only for C++17 when inline variables are supported.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55741



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

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

I'd be happy to restrict this to > C++17 only (which is automatically the case 
when using G++ because there's no `-fdestroying-delete` to enable it, you only 
get it with `-std=c++2a`,  `-std=gnu++2a` etc.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55741



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

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

As it looks like libc++ isn't going to define the type for earlier dialects, 
libstdc++ doesn't either (which seems right anyway because the names aren't 
reserved prior to C++20).

We now define the type and variable unconditionally for C++2a, but only define 
the `__cpp_lib_destroying_delete` macro if compiling for C++2a **and** the 
compiler advertises support via the impl macro.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55741



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

[PATCH] D51262: Implement P0553 and P0556

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D51262#1213514 , @mclow.lists wrote:

> I should also mention that as a conforming extension, I have implemented the 
> non-numeric bit operations for `std::byte`


I thought there was a DR or proposal to enable that, but I don't see one now.




Comment at: include/bit:405
+
+template 
+inline _LIBCPP_INLINE_VISIBILITY constexpr

mclow.lists wrote:
> EricWF wrote:
> > Please write the SFINAE using a default template parameter.
> > 
> > See 
> > http://libcxx.llvm.org/docs/DesignDocs/ExtendedCXX03Support.html#use-default-template-parameters-for-sfinae
> I think that document is misguided in cases like this; the `enable_if` on the 
> return type cannot be "interfered with" by the caller the way an extra 
> template parameter can be.
> 
> 
Yes (except on constructors) libstdc++ always uses the return type instead of a 
default template argument list, to stop "clever" people doing `ceil2(3)` 
and avoiding the contraint.



Comment at: libcxx/include/bit:378
+   const unsigned __retVal = 1u << (__n + __extra);
+   return (_Tp) (__retVal >> __extra);
+}

mclow.lists wrote:
> mclow.lists wrote:
> > mclow.lists wrote:
> > > Quuxplusone wrote:
> > > > mclow.lists wrote:
> > > > > mclow.lists wrote:
> > > > > > Quuxplusone wrote:
> > > > > > > Why so complicated? Is there a unit test that demonstrates why 
> > > > > > > you can't just use `return _Tp{1} << __n;` in this case as well?
> > > > > > > 
> > > > > > > Also, is this a kosher use of `sizeof(X) * 8` as a stand-in for 
> > > > > > > `numeric_limits::digits`?
> > > > > > > 
> > > > > > > Also, speaking of unit tests, I don't see any unit tests for e.g. 
> > > > > > > `std::ceil2(256)` or `std::ceil2(65536)`. Shouldn't there be some?
> > > > > > Yes. I want to generate some UB here, so that this is not a "core 
> > > > > > constant expression" as per P1355.
> > > > > Yes, the `ceil2.fail.cpp` test will not fail (for short types) if I 
> > > > > just return `_Tp{1} << __n;` - because of integer promotion.
> > > > > 
> > > > Yikes. Sounds like there's some "library maintainer"ship going on here, 
> > > > then. Naively, I would have thought that the Working Draft had left the 
> > > > behavior of such constructs undefined in order to make the library 
> > > > vendor's life **easier** and the code **more efficient**. But you're 
> > > > saying that you need to pessimize the code here in order to make it 
> > > > "sufficiently undefined" so that its behavior at compile time will be 
> > > > well-defined and produce a diagnostic instead of being undefined and 
> > > > producing garbage?
> > > > 
> > > > Maybe WG21 needs to invent a "really actually undefined behavior" that 
> > > > does not have unwanted interactions with constexpr, so that library 
> > > > writers can go back to generating fast clean code!
> > > > 
> > > > Rant aside, why don't you just do `_LIBCPP_ASSERT(__n < 
> > > > numeric_limits<_Tp>::digits);` and leave it at that?  An assertion 
> > > > failure isn't a compile-time constant, is it?
> > > > Also, is this a kosher use of sizeof(X) * 8 as a stand-in for 
> > > > numeric_limits::digits?
> > > 
> > > Good catch.
> > that's exactly what P1355 says.
> There are three things that I'd like this code to do in the case of bad 
> inputs:
> * Produce a diagnostic if called at constexpr time (required by P1355)
> * Cause UBSAN to catch it at runtime
> * Cause a libc++ assertion to go off if they are enabled.
> 
> Note that these are all more or less independent.
Those are exactly the three things that my implementation also does, which is 
not a coincidence because this requirement of P1355 has been discussed offline 
(and feedback from implementation experience sent to the author about this 
particular aspect of it).

@Quuxplusone an assertion failure is not enabled by default, so doesn't do any 
good in a constant expression. There are a few different ways to make the shift 
undefined when the operand promotes, but you can't just avoid the complexity 
altogether.


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

https://reviews.llvm.org/D51262



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

[PATCH] D112906: [PowerPC] Emit warning for ieeelongdouble on older GNU toolchain

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

> GCC 12 should have proper support for IEEE-754 compliant 128-bit floating 
> point in libstdc++.

Yes, but it's unconditionally disabled when including the headers with Clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112906

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

[PATCH] D112906: [PowerPC] Emit warning for ieeelongdouble on older GNU toolchain

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D112906#3304925 , @qiucf wrote:

> Is that because clang lacks something required by this feature? (for example. 
> clang-12 doesn't have `__ibm128` and many builtins) If so, clang-14 should 
> have these fixed.

When I implemented the libstdc++ changes Clang didn't have any of your work 
yet. Now that everything seems to be present, and `__LONG_DOUBLE_IEEE128__` and 
`__LONG_DOUBLE_IBM128__` are defined as appropriate, I think I can enable it 
for clang. I'll look into that tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112906

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

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely 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

ldionne wrote:
> 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?
GCC 9 doesn't support constexpr allocation at all.


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

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely 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

jwakely wrote:
> ldionne wrote:
> > 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?
> GCC 9 doesn't support constexpr allocation at all.
GCC 9 doesn't support constexpr allocation at all. You can use 
`__cpp_constexpr_dynamic_alloc` to test for the feature.


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

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely 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

ldionne wrote:
> jwakely wrote:
> > jwakely wrote:
> > > ldionne wrote:
> > > > 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?
> > > GCC 9 doesn't support constexpr allocation at all.
> > GCC 9 doesn't support constexpr allocation at all. You can use 
> > `__cpp_constexpr_dynamic_alloc` to test for the feature.
> Does GCC 10 support it?
Yes


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

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D68364#2293971 , @leonardchan wrote:

> Is there a recommended way for working around this? We're using GCC 10.2.1. 
> Thanks.

I don't think your implementation is valid. I think P0784 only allows 
new-expressions and calls to `std::allocator::allocate` in constexpr functions, 
not calls to `operator new`.

Can you use something like `if (__builtin_is_constant_evaluated() return new 
unsigned char[n];` and the equivalent in the corresponding deallocation 
function.


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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D87974#2438723 , @zoecarver wrote:

> In D87974#2438682 , @BillyONeal 
> wrote:
>
>>> Are they actually the same, with the same handling of corner cases like 
>>> unions and tail padding?
>>> There's more to this than just the name, and if they aren't the same, it 
>>> seems better to have two names.
>>
>> They are both implementing the same C++ feature, with the same desired 
>> semantics of zeroing out any bits in the object representation that aren't 
>> in the value representation. If they differ, one or the other would have a 
>> bug.

Do they support non-trivially copyable types? That isn't required for the 
atomic compare exchange feature, but is relevant for a feature exposed to 
users. What about extensions like zero-sized arrays or C99 flexible array 
members?

> I agree, they either need to be identical (including corner cases) or there 
> needs to be two of them (i.e., GCC ships both `__builtin_zero_non_value_bits` 
> and `__builtin_clear_padding` and the first is the same as MSVC, Clang, and 
> NVCC).

GCC doesn't need to support both. It only works with libstdc++ so it only needs 
to support the one used by libstdc++ (although there is a patch to add 
`-stdlib=libc++` to GCC).

If libstdc++ uses `__has_builtin` to check what the compiler supports then 
Clang doesn't even need to support GCC's built-in, because libstdc++ wouldn't 
use it if not supported (and could use `__builtin_zero_non_value_bits` instead 
when supported).

The Intel compiler would need to support both though.

>>> Is there a specification for __builtin_zero_non_value_bits available 
>>> somewhere?
>>
>> I don't know if there is a formal spec for it beyond the actual C++ standard.
>
> I think P0528 is the relevant paper but other than that, no, there's not a 
> spec. I think that's going to be the most time sensitive part of implementing 
> this: coming up with the spec and making sure all the tests pass on all the 
> implementations.

GCC has publicly available documentation describing its built-in, and publicly 
available tests for it. That's the kind of spec I'm looking for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D87974#2440533 , @BillyONeal wrote:

> Of course if it's already publicly documented for you the horse has 
> presumably already left the barn which makes the discussion moot?

It's not in a shipping release yet. But the point of documenting such built-ins 
partly so that other compiler implementors (and vendors of tools such as static 
analyzers) know what they're trying to be consistent with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

As of a few hours ago, GCC has 
[`__builtin_clear_padding`](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclear_005fpadding)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D87974#2436169 , @BillyONeal wrote:

> The name MSFT is already shipping in production is 
> `__builtin_zero_non_value_bits`. If gcc is already shipping another name in 
> production I think clang is stuck supporting both names, if gcc has not yet 
> shipped their implementation perhaps we can choose one. That seems to be more 
> on gcc than it is on clang given clang's desire to be more or less a drop in 
> replacement for either gcc or msvc.

Are they actually the same, with the same handling of corner cases like unions 
and tail padding?

There's more to this than just the name, and if they aren't the same, it seems 
better to have two names.

Is there a specification for `__builtin_zero_non_value_bits` available 
somewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely requested changes to this revision.
jwakely added a comment.
This revision now requires changes to proceed.

You can't use `__GLIBCXX__` this way. It will be different for different 
snapshots from the gcc-11 branch.

I would just check RELEASE == 11. If `__failed_assertion` is present, you'll 
rename it. If it's not present, nothing gets renamed but it works anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D102936: [CUDA] Work around compatibility issue with libstdc++ 11.1.0

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely accepted this revision.
jwakely added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102936

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> qiucf wrote:
> > hubert.reinterpretcast wrote:
> > > qiucf wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > Not sure what the best method is to implement this, but `long double` 
> > > > > and `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` 
> > > > > is in effect.
> > > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I 
> > > > saw `__float128` and `long double` are the same for GCC but not for 
> > > > clang.
> > > Have you checked whether the new libstdc++ for which this support is 
> > > being added needs the GCC behaviour to work properly?
> > > 
> > > The GCC behaviour allows the following to be compiled without introducing 
> > > novel overload resolution tiebreakers:
> > > ```
> > > void f(__float128);
> > > void f(__ibm128);
> > > void f(int);
> > > 
> > > long double ld;
> > > 
> > > int main() { f(ld); }
> > > ```
> > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > 
> > ```
> > std::cout << "long double:\n";
> > std::cout << std::numeric_limits::max() << std::endl;
> > std::cout << std::numeric_limits::min() << std::endl;
> > 
> > std::cout << "__float128:\n";
> > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > 
> > std::cout << "__ibm128:\n";
> > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > ```
> @jwakely, are the overload resolution errors expected? @qiucf, are you sure 
> you have a sufficiently new libstdc++?
> @jwakely, are the overload resolution errors expected? 

Yes. Iostreams support `long double` but not `__float128`, unless that happens 
to be the same type as `long double` (due to a `-mabi=ieeelongdouble` option).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> jwakely wrote:
> > hubert.reinterpretcast wrote:
> > > qiucf wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > qiucf wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > Seems clang is also different from GCC under 
> > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > the same for GCC but not for clang.
> > > > > Have you checked whether the new libstdc++ for which this support is 
> > > > > being added needs the GCC behaviour to work properly?
> > > > > 
> > > > > The GCC behaviour allows the following to be compiled without 
> > > > > introducing novel overload resolution tiebreakers:
> > > > > ```
> > > > > void f(__float128);
> > > > > void f(__ibm128);
> > > > > void f(int);
> > > > > 
> > > > > long double ld;
> > > > > 
> > > > > int main() { f(ld); }
> > > > > ```
> > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > 
> > > > ```
> > > > std::cout << "long double:\n";
> > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > 
> > > > std::cout << "__float128:\n";
> > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > 
> > > > std::cout << "__ibm128:\n";
> > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > ```
> > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > sure you have a sufficiently new libstdc++?
> > > @jwakely, are the overload resolution errors expected? 
> > 
> > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > happens to be the same type as `long double` (due to a 
> > `-mabi=ieeelongdouble` option).
> Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> diverge from GCC.
> 
> For example, Clang reports the call below as ambiguous even with 
> `-mabi=ieeelongdouble`:
> ```
> void f(double);
> void f(long double);
> 
> void g(__float128 f128) { f(f128); }
> ```
> 
> https://godbolt.org/z/dhYEKa
> 
> Insofar as the user experience goes, is this lack of iostreams support for 
> `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> intended design of libstdc++?
The lack of iostreams support for `__float128` is the intended design.

On power we support `float`, `double` and three types of `long double`. If 
`__float128` is a distinct type from all those `long double` types it won't 
work.

GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it is 
the same as one of the `long double` types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.
Herald added a subscriber: abrachet.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

musl only declares `getentropy` in `` not ``. Glibc 
declares it in both. Should `` also be included when 
`_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

jwakely wrote:
> musl only declares `getentropy` in `` not ``. Glibc 
> declares it in both. Should `` also be included when 
> `_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?
I suppose it doesn't really matter, since `_LIBCPP_USING_GETENTROPY` is 
currently only defined for fuchsia and wasi, which both declare `getentropy` in 
``.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added subscribers: fweimer, dalias.
jwakely added inline comments.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

mcgrathr wrote:
> jwakely wrote:
> > jwakely wrote:
> > > musl only declares `getentropy` in `` not ``. 
> > > Glibc declares it in both. Should `` also be included when 
> > > `_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?
> > I suppose it doesn't really matter, since `_LIBCPP_USING_GETENTROPY` is 
> > currently only defined for fuchsia and wasi, which both declare 
> > `getentropy` in ``.
> FWIW, if there is an emerging norm to declare getentropy in  then 
> Fuchsia's libc can add it to our  and there's no real problem 
> relying on that in libc++ sources within about a week of when we land that 
> change in Fuchsia's trunk.
> 
@fweimer added it to glibc's unistd.h for compatibility with existing BSD code:
https://sourceware.org/bugzilla/show_bug.cgi?id=17252#c9

Maybe @dalias can confirm whether musl did it for the same reason?

Does it make sense to "standardize" on declaring `getentropy` in `unistd.h`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D131963: [libc++] Add custom clang-tidy checks

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: libcxx/test/tools/clang_tidy_checks/robust_against_adl.cpp:22
+AST_MATCHER(clang::UnresolvedLookupExpr, isCustomizationPoint) {
+  // TODO: Are make_error_code and make_error_condition actually customization 
points?
+  return std::ranges::any_of(

Mordante wrote:
> ldionne wrote:
> > This is funny, I actually saw a LWG issue about that go by a few weeks ago. 
> > I'll try to get more details.
> @ldionne @jwakely posted this bug report about it 
> https://github.com/llvm/llvm-project/issues/57614
They're definitely customization points. If they weren't, specializing 
`is_error_code_enum` and `is_error_condition_enum` would be completely useless 
and serve no purpose.

LWG confirmed that and [LWG 3629](https://wg21.link/lwg3629) is Tentatively 
Ready now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131963

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

[PATCH] D131963: [libc++] Add custom clang-tidy checks

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: libcxx/test/tools/clang_tidy_checks/robust_against_adl.cpp:22
+AST_MATCHER(clang::UnresolvedLookupExpr, isCustomizationPoint) {
+  // TODO: Are make_error_code and make_error_condition actually customization 
points?
+  return std::ranges::any_of(

jwakely wrote:
> Mordante wrote:
> > ldionne wrote:
> > > This is funny, I actually saw a LWG issue about that go by a few weeks 
> > > ago. I'll try to get more details.
> > @ldionne @jwakely posted this bug report about it 
> > https://github.com/llvm/llvm-project/issues/57614
> They're definitely customization points. If they weren't, specializing 
> `is_error_code_enum` and `is_error_condition_enum` would be completely 
> useless and serve no purpose.
> 
> LWG confirmed that and [LWG 3629](https://wg21.link/lwg3629) is Tentatively 
> Ready now.
I created https://reviews.llvm.org/D134943 to fix those customization points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131963

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

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/include/clang/Basic/TokenKinds.def:528
+TYPE_TRAIT_1(__is_nothrow_copy_constructible, IsNothrowCopyConstructible, 
KEYCXX)
+TYPE_TRAIT_1(__is_trivially_copy_constructible, IsTriviallyCopyConstructible, 
KEYCXX)
 TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX)

cjdb wrote:
> ldionne wrote:
> > cjdb wrote:
> > > erichkeane wrote:
> > > > royjacobson wrote:
> > > > > erichkeane wrote:
> > > > > > cjdb wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > So this one is a whole 'thing'.  The Clang definition of 
> > > > > > > > 'trivially copy constructible' is a few DRs behind.  We should 
> > > > > > > > probably discuss this with libcxx to make sure use of this 
> > > > > > > > wouldn't be broken.
> > > > > > > I'd prefer to get those DRs in before finalising D135238 and 
> > > > > > > subsequent ones. Do you know the DR numbers I should be looking 
> > > > > > > at, or should I just poke npaperbot?
> > > > > > Not off the top of my head, Aaron and I both poked at it at one 
> > > > > > point trying to get trivially constructible right at one point, but 
> > > > > > I think we both gave up due to the ABI/versioning concerns.
> > > > > Maybe DR1734? Although it's about the trivially copyable trait, not 
> > > > > trivially copy constructible. 
> > > > > 
> > > > Yeah, I think that was the DR, that number sounds familiar.
> > > The `__is_trivially_*` traits were, in part, what inspired the Great 
> > > Split of D116208. I could remove them for now and revisit once I rip my 
> > > hair out over these DRs, if that would substantially improve the chances 
> > > of these commits landing (other commentary notwithstanding).
> > I am not sure I see a problem with the "triviality" part of this -- we 
> > already use a compiler builtin for `std::is_trivially_constructible`, so I 
> > would expect either this patch is fine, or we already have a latent bug in 
> > libc++.
> > 
> > I think I can echo @philnik's comment about this not necessarily providing 
> > the biggest benefit since our implementation of 
> > `std::is_trivially_copy_constructible` is a fairly trivial wrapper on top 
> > of `__is_trivially_constructible`, but I wouldn't object to the patch on 
> > that basis. I think it would probably be possible to instead provide a set 
> > of basis builtin operations that we can then build all of the library type 
> > traits on top of -- that would provide the highest bang-for-our-buck ratio.
> > 
> > At the same time, there's something kind of enticing in the consistency of 
> > defining every single type trait as a builtin, without exception. If that's 
> > the end goal, I think that would also be neat and we'd likely regroup all 
> > of our type traits under a single header, since each of them would 
> > literally be a one liner.
> > 
> > There's also the question of whether GCC provides these builtins -- if they 
> > don't and if they don't have plans to, then we'd actually need to add 
> > complexity in libc++ to support both, which we would be unlikely to do 
> > given that there's probably not a huge compile-time performance benefit.
> > 
> > TLDR, I think the two questions that can help gauge how much interest there 
> > will be from libc++ to use this are:
> > 
> > 1. Is the plan to provide *all* the type traits as builtins?
> > 2. Will GCC implement them?
> > 
> > That being said, libc++ might not be the only potential user of these 
> > builtins, so I wouldn't necessarily make it a hard requirement to satisfy 
> > us.
> > 
> > I think I can echo @philnik's comment about this not necessarily providing 
> > the biggest benefit since our implementation of 
> > `std::is_trivially_copy_constructible` is a fairly trivial wrapper on top 
> > of `__is_trivially_constructible`, but I wouldn't object to the patch on 
> > that basis.
> 
> I haven't had time to do anything properly in the way of benchmarking, but 
> after looking at @philnik's quoted code, I see that I'd naively addressed 
> `__is_constructible(T, T const&)`, forgetting that `__add_lvalue_reference` 
> would've fixed that issue.
> 
> > 1. Is the plan to provide *all* the type traits as builtins?
> 
> Yes, with the possible exception of `enable_if` and `add_const` etc. (see 
> D116203 for why the qualifier ones aren't already in). The hardest ones will 
> probably be `common_type`, `common_reference`, `*invocable*`, and 
> `*swappable*`. The former two depend on technology that doesn't exist in 
> Clang yet, and the latter two are likely hard due there not being prior art.
> 
> > 2. Will GCC implement them?
> 
> @jwakely do you know if there can be cross-compiler synergy here?
> 
> 
Which traits are you asking about, just the 
__is_{,trivially,nothrow}_copy_constructible ones? Or all type traits?

Either way, I think the answer is no. We already use 
__is_{,trivially,nothrow}_constructible for the copy-constructible traits, and 
it work

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

In D151952#4396485 , @cjdb wrote:

> Since we're doing this, it may also be worth checking that `T1s...` are 
> unique in `T2s...`, which I'd intended to be a separate feature.

It seems like it would be more efficient to include the uniqueness check in the 
intrinsic, otherwise `std::get` and helpers like libstdc++'s 
`variant::__index_of` will still need some template metaprogramming to check 
for uniqueness. Even if there's a separate intrinsic for that, it seems more 
useful to just have one intrinsic that does both. If you have a use case for 
finding the index of the first T in a pack, maybe add an additional boolean 
that requests uniqueness and would cause `__type_pack_index` to fail if T is 
not unique.

As @BertalanD also said, it doesn't seem useful to be ill-formed if the type 
isn't in the pack. That makes it unusable for `std::get` and `std::variant`. 
Libstdc++'s `__find_uniq_type_in_pack` returns `sizeof...(Types)` 
if `T` isn't found (either this or `-1uz` seems fine, you can check the 
returned index is `< sizeof...(Types)` to tell if the type was found).




Comment at: clang/docs/LanguageExtensions.rst:1660
+T& get(tuple& ts) noexcept {
+  return std::get<__type_pack_index(T, Ts...)>(ts);
+}

If this intrinsic doesn't (yet?) check for uniqueness, then this `get` 
doesn't match `std::get`. It might be worth mentioning that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952

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

[PATCH] D28981: Use GNU-style attributes for several __throw_XXX() functions

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

The __throw_xxx functions are not part of the public libstdc++ API and whatever 
Firefox is trying to do with them is not supported by libstdc++ and is 
undefined behaviour. Make it stop.


https://reviews.llvm.org/D28981



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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:2668
   // ::= g  # __float128
+  // ::= g  # __ibm128
   // UNSUPPORTED:::= Dd # IEEE 754r decimal floating point (64 bits)

steven.zhang wrote:
> This is a bit confusing as the 'g' is for both __float128 and __ibm128. I 
> know that PowerPC will mangle the __float128 as "u9__ieee128", and "g" for 
> __ibm128. But c++filt demangle the "g" as __float128 which seems not right. 
The (de)mangling is very confusing, but that's consistent with gcc.
See comment 3 and 4 at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98389#c3

I think double-double should never have used `g` it should have been something 
else (e.g. `u8__ibm128`), and then we could have `g` for the `__ieee128` (aka 
`__float128`) type. But it is many years too late to change that now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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