[cfe-dev] Implementing the proposed resolution of C++ CWG 1423

[Nikola Smiljanic]

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.

On Thu, May 28, 2015 at 10:30 AM, dyp <dyp-cpp at gmx.net> wrote:

> 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 ;)
>
> 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
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150528/0694625e/attachment.html>