Implementing the proposed resolution of C++ CWG 1423

Dear clang developers,

I've been trying to implement the resolution of C++ CWG 1423 in clang++
which restricts conversions from nullptr_t to bool to
direct-initialization. This is my first attempt at contributing to
clang, and as far as I can tell, it's the only standard conversion that
is not an implicit conversion.

(I couldn't find any bug report nor mailing on this, and I wanted to
contribute to clang for some time now, so I just went ahead and
implemented it. Even if my solution is too crappy to be accepted, it was
a nice exercise and not too hard I guess :wink:

While I was successful in adding the restriction, there remain some
issues with my fix, and I'm not very comfortable with my
"solution" for list-initialization. Therefore, I'd appreciate any
feedback and suggestions how to introduce the restrictions. I hope my
request is appropriate for this mailing list.

Specifically, I have the following questions:

1. Changes of public member functions
I've modified the signature of two public member functions of the Sema
class by adding an additional parameter. How problematic is this change?
(API etc)

2. OpenMP support
I've added an additional parameter to the first two overloads of
Sema::PerformImplicitConversion, that specifies whether or not we're
performing a direct initialization. This function is also used by
SemaOpenMP.cpp
I have never worked with OpenMP, so I don't really know if the usages in
SemaOpenMP.cpp occur within a direct-initialization context.

3. Diagnostics
How important are good diagnostic messages for this new error case?
Should better/specific diagnostics be included in the patch, or can they
be improved in a later patch?

4. Test cases
Since CWG 1423 is classified as a DR, and g++ implements it also in
C++11 mode, I did not include any language standard switches for this
fix. However, this leads to some of the existing tests now failing. I
have modified them, and added further test cases for this special
implicit conversion. Unfortunately, this means that there's no separate
test for this fix. Is there a better solution?

5. Support for list-initialization
I couldn't figure out how to properly add a distinction between
direct-list-initialization and copy-list-initialization for CWG 1423.
The TryListInitialization function relies on the InitListChecker class
for all scalar initialization. As the comment some lines above points
out, the InitListChecker class always performs copy-initialization, and
it seemed to me to be deeply ingrained in its algorithms. There already
are several special cases in TryListInitialization that handle
direct-list-initialization where the destination type is a record type.
I've tried to add scalar initialization to one case that uses
InitializationSequence::InitializeFrom (which finally dispatches to
Sema::TryImplicitConversion which I've already modified for
direct-nonlist-initialization), but this makes some tests fail
apparently due to a lack of support of initialization of vector types,
different diagnostics and compound literals.
It seems this part could benefit from some refactoring, although I'm not
sure what would be a good approach.
As a quick and dirty fix, I've routed only the nullptr_t -> bool
conversion through this special case, which is of course only a
workaround to prevent the aforementioned issues. It does work, but it
feels like a really dirty hack.

6. A naming issue
It occurred to me when writing this email that my extension of
Sema::TryImplicitConversion might be inappropriate:
Direct-initialization is not an implicit conversion as defined in
C++14[conv]p3.
Sema::TryContextuallyConvertToBool also uses TryImplicitConversion, even
though that isn't strictly an implicit conversion either. (Probably just
a bike-shedding issue.)

Thank you very much and kind regards,

dyp

patch.txt (22.3 KB)

Thumbs up for your first patch, it’s probably best if you start an official review process using Phabricator, make sure you CC someone familiar with this code.