aaron.ballman added a comment.

In D111400#3088133 <https://reviews.llvm.org/D111400#3088133>, 
@hubert.reinterpretcast wrote:

> In D111400#3087877 <https://reviews.llvm.org/D111400#3087877>, @aaron.ballman 
> wrote:
>
>> So your concern is that allowing this patch to be used as an extension in 
>> older language modes may change the behavior of existing code? Or is your 
>> concern more broad than just this patch?
>
> The behaviour of Clang before this patch in older language modes is 
> inconsistent with the specification before P2242R3 (for the template case) in 
> such a way that it could appear that P2242R3 is already implemented for the 
> template case (but not following the usual convention for extensions 
> affecting SFINAE); however, the mechanism by which that occurs might be 
> something other than an implementation of P2242R3.
>
> My concern is that the status quo of the Clang implementation with respect to 
> this area is broken, which in turn makes it possible for this patch to 
> exacerbate the issue by building on top of the brokenness and then making a 
> fix more complicated. At the very least, this patch does not demonstrate that 
> the "extension" does not affect SFINAE.
>
> In other words, my concern is that this patch is necessarily incomplete 
> unless if the situation around the template case is resolved.

Thank you for the explanation, that's helpful! I'm sympathetic to not wanting 
to build on top of an unstable foundation, but I'm also a bit worried that 
we're asking a lot of @cor3ntin in terms of this PR because it sounds like this 
is a general request to fix template instantiation before doing more constexpr 
work (because anything constexpr is generally SFINAE-able). I think we need to 
fix the template instantiation issues, but I'm also not certain we should gate 
constexpr work on those fixes.

I'd definitely like to hear thoughts from @rsmith on the right way to proceed.

>> Thank you for the example code, that helps add clarity. I think it's worth 
>> noting that implementations disagree here in a few different ways: 
>> https://godbolt.org/z/f9KnMhTGd
>
> ICC fails to SFINAE -- but at least it is consistent with GCC in considering 
> the expression to be non-constant.

Yeah, and MSVC agrees with current Clang. (I hope that doesn't mean we 
additionally need to think about `-fms-compatibility`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

Reply via email to