[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added a comment.

In D141954#4060647 <https://reviews.llvm.org/D141954#4060647>, @philnik wrote:

> In D141954#4060246 <https://reviews.llvm.org/D141954#4060246>, @erichkeane wrote:
>
>> In D141954#4060244 <https://reviews.llvm.org/D141954#4060244>, @philnik wrote:
>>
>>> In D141954#4060212 <https://reviews.llvm.org/D141954#4060212>, @erichkeane wrote:
>>>
>>>> @ldionne @Mordante  The libcxx failure isn't clear what it is complaining about, but I thought this fix might end up breaking a libcxx test, since it would be easy to fall into the trap of having an implicit conversion here.
>>>>
>>>> Any chance you could prod at that test a little and see whose fault it is?
>>>>
>>>> ALSO: NOTE TO SELF: need release note!
>>>
>>> I think the problem is that `__type_traits/common_reference.h` doesn't re-export `__type_traits/common_type.h` in the modulemap. It //should// be enough to add `export common_type` to `module common_reference` (`libcxx/include/module.modulemap.in:1399`). See `module is_arithmetic` for an example. This is just a guess though, this stuff can be very weird.
>>
>> Oh man, I hope the others know what you're talking about :)  I don't really know how I caused this though :/
>
> I've applied your patch locally and this diff fixes the issue:
>
>   diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
>   index 5d4cf53aa334..30209b353d6b 100644
>   --- a/libcxx/include/module.modulemap.in
>   +++ b/libcxx/include/module.modulemap.in
>   @@ -747,7 +747,10 @@ module std [system] {
>          module assignable                 { private header "__concepts/assignable.h" }
>          module boolean_testable           { private header "__concepts/boolean_testable.h" }
>          module class_or_enum              { private header "__concepts/class_or_enum.h" }
>   -      module common_reference_with      { private header "__concepts/common_reference_with.h" }
>   +      module common_reference_with      {
>   +        private header "__concepts/common_reference_with.h"
>   +        export type_traits.common_reference
>   +      }
>          module common_with                { private header "__concepts/common_with.h" }
>          module constructible              { private header "__concepts/constructible.h" }
>          module convertible_to             { private header "__concepts/convertible_to.h" }
>
> I don't know why your patch changes the behaviour of clang here, but I wouldn't worry too much about it. There is a lot of weirdness around modules and dependent type names resulting in pretty much useless error messages. I hope the work on C++ modules reduces this weirdness, but I don't really know how that stuff works.

Thanks!  I'll apply that patch here and see if it makes the bot happy.


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

https://reviews.llvm.org/D141954