[[implicit]] attribute

[rhalb...@gmail.com]

The Facebook program verifier flint (https://code.facebook.com/posts/729709347050548/under-the-hood-building-and-open-sourcing-flint/) encourages that non-explicit single-argument constructors and conversion operators should be marked with /* implicit */ comments (rules 14 and 22). 

I wonder if there would be Committee support for a proposal for an [[implicit]] attribute. Compilers could be encouraged to warn about missing attribute on non-explicit single argument constructors and conversion operators without such an attribute annotation. Neither gcc nor clang appear to have warnings for this at the moment.

[Moritz Klammler]

I'm not the Committee but I like the idea a lot. It seems very easy to
implement and doesn't hurt anybody who doesn't like it.

The only potential problem I can see is that enabling the warning would
trigger *a lot* of "false" positives for third-party headers that might
never get "fixed" because their authors don't adhere to the coding
standard. Wrapping all third-party `#include`s into `#pragma`s to
temporairly silence the warning is not an option I'd choose eagerly.

[Rein Halbersma]

I guess the usual trick of marking such headers as system headers would avoid the need for wrapping (-isystem on gcc/clang, there is also a CMake flag IIRC)

[Daniel Frey]

> On 23 Sep 2016, at 14:15, Moritz Klammler <moritz....@gmail.com> wrote:
>
> rhalb...@gmail.com writes:
>
>> The Facebook program verifier flint
>> (https://code.facebook.com/posts/729709347050548/under-the-hood-building-and-open-sourcing-flint/)
>> encourages that non-explicit single-argument constructors and
>> conversion operators should be marked with /* implicit */ comments
>> (rules 14 and 22).
>>
>> I wonder if there would be Committee support for a proposal for an
>> [[implicit]] attribute. Compilers could be encouraged to warn about
>> missing attribute on non-explicit single argument constructors and
>> conversion operators without such an attribute annotation. Neither gcc
>> nor clang appear to have warnings for this at the moment.
>
> I'm not the Committee but I like the idea a lot. It seems very easy to
> implement and doesn't hurt anybody who doesn't like it.

It does hurt. A lot. I try to write code that compiles with -Wall, -Wextra, -Werror, but with each new compiler generation it becomes more difficult. Code that was clean and concise before is more and more cluttered with annotations or work-arounds just to get rid of the warnings. In one case, this even forced me to pull in another header and therefore slows down compile times. Also: How would you tell the compiler that this is a system header? In a portable, standardized way! Because I don't want to add another level of complexity to support different compilers. Switching of the warning locally is equally bad, each compiler has its own way and identification for a specific warning.

If you want to use lint-tools, fine, but encouraging compiler writers to add (or even mandate) warnings needs a really good justification. [[fallthough]] I can understand, it helps to catch real bugs and it is used only as an exception. But a non-explicit single-argument ctor is quite common and often the right choice, so I would really like to use it without any further fuss.

[Ville Voutilainen]

On 23 September 2016 at 16:57, Daniel Frey <d.f...@gmx.de> wrote:
>> I'm not the Committee but I like the idea a lot. It seems very easy to
>> implement and doesn't hurt anybody who doesn't like it.
>
> It does hurt. A lot. I try to write code that compiles with -Wall, -Wextra, -Werror, but with each new compiler generation it becomes more difficult. Code that was

I don't believe it's feasible for a C++ implementation to enable the
warning we're talking about by default.

[rhalb...@gmail.com]

The C++ Core guidelines also recommend warning against lack of explicit keywords:

http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a-namerc-explicitac46-by-default-declare-single-argument-constructors-explicit 

http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ro-conversion 

You might be right that it can be an intended design choice to leave a constructor implicit, but since it will silently generate conversions in places where one might not expect it, shouldn't a warning be prudent? You can silence this with [[implicit] in your own code (once) and for legacy code treat them as system headers (if CMake has a SYSTEM flag for it, then all major compilers can do this already).  

[rhalb...@gmail.com]

Could you explain? I'm not suggesting to warn for implicit conversion at the point of call, but only inside the class definition.  

[Ville Voutilainen]

I expect that there's a fair amount of non-annotated code that has
such implicit conversion
constructors in it, not to mention implicit conversion operators.

[Moritz Klammler]

Ville Voutilainen <ville.vo...@gmail.com> writes:

> On 23 September 2016 at 16:57, Daniel Frey <d.f...@gmx.de> wrote:
>>> I'm not the Committee but I like the idea a lot. It seems very easy
>>> to implement and doesn't hurt anybody who doesn't like it.
>>
>> It does hurt. A lot. I try to write code that compiles with -Wall,
>> -Wextra, -Werror, but with each new compiler generation it becomes
>> more difficult. Code that was
>
> I don't believe it's feasible for a C++ implementation to enable the
> warning we're talking about by default.

Yes, it definitely shouldn't be enabled by default. At most, `-Wextra`
could enable it.

But I believe that the code cluttering argument is somewhat reverse. We
shouldn't write code we dislike just to silence warnings but enable
those warnings that flag deviations from the coding standards we
consider good. Naturally, people have different ideas about good coding
standards and thus compilers offer different warnings we can select or
not select. If compilers can only warn about things everybody hates,
that would be sad.

The one exception to this is if your code triggers warnings in other
people's code. But several people have already commented how this point
from my previous message is kinda moot.

[Rein Halbersma]

On Fri, Sep 23, 2016 at 4:24 PM, Ville Voutilainen <ville.vo...@gmail.com> wrote:

On 23 September 2016 at 17:11,  <rhalb...@gmail.com> wrote:

>>
>> I don't believe it's feasible for a C++ implementation to enable the
>> warning we're talking about by default.
>
>
> Could you explain? I'm not suggesting to warn for implicit conversion at the
> point of call, but only inside the class definition.

I expect that there's a fair amount of non-annotated code that has
such implicit conversion
constructors in it, not to mention implicit conversion operators.

OK, so it might be too much work to fix, not too hard to warn. Yet it is what the Core Guidelines recommend. It seems to me a small fix that clang-tidy or similar tools could easily automate.

[Ville Voutilainen]

On 23 September 2016 at 17:29, Rein Halbersma <rhalb...@gmail.com> wrote:
>> I expect that there's a fair amount of non-annotated code that has
>> such implicit conversion
>> constructors in it, not to mention implicit conversion operators.
>
>
> OK, so it might be too much work to fix, not too hard to warn. Yet it is
> what the Core Guidelines recommend. It seems to me a small fix that
> clang-tidy or similar tools could easily automate.

Warning about valid code isn't particularly hard. Avoiding false
positives is some magnitudes
harder.

[Andrzej Krzemieński]

Apart from -Wall and -Wextra compilers like GCC offer other opt-in warnings, that you would not use upon every compilation due to too many false positives, but you could use them from time to time, or as non-errors, to successively refactor or annotate the warned about code.

[Andrzej Krzemieński]

W dniu piątek, 23 września 2016 08:44:07 UTC+2 użytkownik rhalb...@gmail.com napisał:

The Facebook program verifier flint (https://code.facebook.com/posts/729709347050548/under-the-hood-building-and-open-sourcing-flint/) encourages that non-explicit single-argument constructors and conversion operators should be marked with /* implicit */ comments (rules 14 and 22). 

I wonder if there would be Committee support for a proposal for an [[implicit]] attribute. Compilers could be encouraged to warn about missing attribute on non-explicit single argument constructors and conversion operators without such an attribute annotation. Neither gcc nor clang appear to have warnings for this at the moment.

See this GCC feature request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77741

[Jens Åkerblom]

Wouldn't it be better to depricate the explicit keyword in favor of having it by default? That is, replace "explicit" with "implicit".

[Jens Åkerblom]

[Nicol Bolas]

On Wednesday, September 28, 2016 at 12:56:27 AM UTC-4, Jens Åkerblom wrote:

Wouldn't it be better to depricate the explicit keyword in favor of having it by default? That is, replace "explicit" with "implicit".

That would require an additional keyword as well as break lots and lots of code.

[Tony V E]

That is a 20+ year process:

1. Introduce implicit. Compilers can have optional warning on naked constructors 
2. After N  years, compilers warn by default 
3. Deprecate naked constructors
4. Error on naked constructors 
5. Make explicit the default
6. Deprecate explicit
7. Remove explicit

You could add
0. [[implicit]]

Which is a sensible place to start, since it is just for compiler warnings, and doesn't change valid code. 


Sent from my BlackBerry portable Babbage Device
  Original Message  
From: Jens Åkerblom
Sent: Wednesday, September 28, 2016 12:56 AM
To: ISO C++ Standard - Future Proposals
Reply To: std-pr...@isocpp.org
Subject: [std-proposals] [[implicit]] attribute

Wouldn't it be better to depricate the explicit keyword in favor of having it by default? That is, replace "explicit" with "implicit".

--
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Future Proposals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-proposal...@isocpp.org.
To post to this group, send email to std-pr...@isocpp.org.
To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/5cd862e0-d9a8-42bb-96d7-3cf99656f424%40isocpp.org.

[Alisdair Meredith]

I have no interest in 0), unless we plan for the 20+ year process.

Adding an attribute to raise warnings essentially forces a keyword on
all constructors, whether implicit or explicit, adding burden on all users
of the language.

However, I am the mad man who might be interested in 20 year
projects, so you may persuade me yet ;)

AlisdairM

> To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/20160928140550.4919374.10911.17563%40gmail.com.

[Andrzej Krzemieński]

W dniu środa, 28 września 2016 16:09:38 UTC+2 użytkownik Alisdair Meredith napisał:

I have no interest in 0), unless we plan for the 20+ year process.

Adding an attribute to raise warnings essentially forces a keyword on
all constructors, whether implicit or explicit, adding burden on all users
of the language.

Not that much burden: if you are now forced to mark your constructor as explicit: it is a bug fix: it was supposed to be explicit long ago.
If you are forced to mark the constructor [[implicit]]: this does not happen very often: how often do you consciously implement an intended conversion?
 

[Nevin Liber]

On Wed, Sep 28, 2016 at 9:09 AM, Alisdair Meredith <alis...@me.com> wrote:

I have no interest in 0), unless we plan for the 20+ year process.

Adding an attribute to raise warnings essentially forces a keyword on
all constructors, whether implicit or explicit, adding burden on all users
of the language.

+1 to both of these.

And personally, I've found making copy constructors explicit to just be troublesome (and I've never experimented with making move constructors explicit because of that), but I assume the people wanting this feature have the opposite experience, or they would have mentioned this, given that copy constructors are the most popular non-explicit single argument constructors.
--

 Nevin ":-)" Liber  <mailto:ne...@eviloverlord.com>  +1-847-691-1404

[Rein Halbersma]

I didn't mean to propose [[implicit]] to suppress warnings for copy/move constructors, inherited constructors or even conversion to base classes. Only for single argument constructors from / conversions to unrelated types. 

So. e.g. std::bitset has implicit constructors from unsigned long and unsigned long long, they can be surprising (especially since the overloaded bit-operators for std::bitset do not have such conversions on their arguments).

[Nevin Liber]

On Wed, Sep 28, 2016 at 1:00 PM, Rein Halbersma <rhalb...@gmail.com> wrote:

I didn't mean to propose [[implicit]] to suppress warnings for copy/move constructors, inherited constructors or even conversion to base classes.

Oh, you want a complicated rule.

 

Only for single argument constructors from / conversions to unrelated types. 

And what do you do about things like:

struct Foo {

    Foo(std::string s, int i = 0);

    template<typename... Ts>

    Foo(Ts&&...) { /* ... */ }

};

Do we get warnings for either of those?  Both of those?  Neither of those?

So. e.g. std::bitset has implicit constructors from unsigned long and unsigned long long, they can be surprising (especially since the overloaded bit-operators for std::bitset do not have such conversions on their arguments).

That may be (I don't know the original motivation and I'm not motivated enough to do the archeology), but changing that has a non-zero chance of breaking user code.

[Rein Halbersma]

On Wed, Sep 28, 2016 at 8:34 PM, Nevin Liber <ne...@eviloverlord.com> wrote:

Only for single argument constructors from / conversions to unrelated types. 

And what do you do about things like:

struct Foo {

    Foo(std::string s, int i = 0);

    template<typename... Ts>

    Foo(Ts&&...) { /* ... */ }

};

Do we get warnings for either of those?  Both of those?  Neither of those?

OK, fair enough, this is getting complicated.

 

So. e.g. std::bitset has implicit constructors from unsigned long and unsigned long long, they can be surprising (especially since the overloaded bit-operators for std::bitset do not have such conversions on their arguments).

That may be (I don't know the original motivation and I'm not motivated enough to do the archeology), 

the proposals for std::bitset predate the introduction of the explicit keyword...

[Nicol Bolas]

On Wednesday, September 28, 2016 at 2:35:37 PM UTC-4, Nevin ":-)" Liber wrote:

On Wed, Sep 28, 2016 at 1:00 PM, Rein Halbersma <rhalb...@gmail.com> wrote:

I didn't mean to propose [[implicit]] to suppress warnings for copy/move constructors, inherited constructors or even conversion to base classes.

Oh, you want a complicated rule.

No, it would be a simple rule. It's any constructor which would be used for implicit conversion operations, in accord with [over.best.ics]. Well, minus [over.match.list] (list initialization), of course.

Copy/move constructors don't apply, since they are only invoked on the actual type T. And I don't believe that inherited constructors apply either.

 

Only for single argument constructors from / conversions to unrelated types. 

And what do you do about things like:

struct Foo {

    Foo(std::string s, int i = 0);

    template<typename... Ts>

    Foo(Ts&&...) { /* ... */ }

};

Do we get warnings for either of those?  Both of those?  Neither of those?

The point of the warning is to that, if a constructor can be used by an implicit conversion sequence, then you must explicitly mark it as such. Since both of those constructors can be used for implicit conversion sequences under [over.best.ics], then yes, they both get warnings.

That being said, perhaps the warning could be issued due to the use of such a conversion sequence, rather than the mere existence of a constructor that could be used with it.

[Jeffrey Yasskin]

On Wed, Sep 28, 2016 at 11:56 AM, Nicol Bolas <jmck...@gmail.com> wrote:
> On Wednesday, September 28, 2016 at 2:35:37 PM UTC-4, Nevin ":-)" Liber
> wrote:
>>
>> On Wed, Sep 28, 2016 at 1:00 PM, Rein Halbersma <rhalb...@gmail.com>
>> wrote:
>>>
>>>
>>> I didn't mean to propose [[implicit]] to suppress warnings for copy/move
>>> constructors, inherited constructors or even conversion to base classes.
>>
>>
>> Oh, you want a complicated rule.
>
>
> No, it would be a simple rule. It's any constructor which would be used for
> implicit conversion operations, in accord with [over.best.ics]. Well, minus
> [over.match.list] (list initialization), of course.
>
> Copy/move constructors don't apply, since they are only invoked on the
> actual type T. And I don't believe that inherited constructors apply either.

+1, with the caveat that I haven't checked the actual section references.

>>> Only for single argument constructors from / conversions to unrelated
>>> types.
>>
>>
>> And what do you do about things like:
>>
>> struct Foo {
>> Foo(std::string s, int i = 0);
>>
>> template<typename... Ts>
>> Foo(Ts&&...) { /* ... */ }
>> };
>>
>> Do we get warnings for either of those? Both of those? Neither of those?
>
>
> The point of the warning is to that, if a constructor can be used by an
> implicit conversion sequence, then you must explicitly mark it as such.
> Since both of those constructors can be used for implicit conversion
> sequences under [over.best.ics], then yes, they both get warnings.
>
> That being said, perhaps the warning could be issued due to the use of such
> a conversion sequence, rather than the mere existence of a constructor that
> could be used with it.

FWIW, I'd rather warn about a class definition based on rules that can
be applied to the definition itself. Warning about a missing
`implicit` marker based on uses is likely to produce a bunch of
warnings that the warn-ee can't fix in their own code.

Jeffrey

[Andrzej Krzemieński]

W dniu środa, 28 września 2016 19:47:38 UTC+2 użytkownik Nevin ":-)" Liber napisał:

On Wed, Sep 28, 2016 at 9:09 AM, Alisdair Meredith <alis...@me.com> wrote:

I have no interest in 0), unless we plan for the 20+ year process.

Adding an attribute to raise warnings essentially forces a keyword on
all constructors, whether implicit or explicit, adding burden on all users
of the language.

We are not comparing "mark only converting" to "mark every one", but "mark only explicit" to "mark every one". The current one, "mark only explicit" is bug-prone. So, we are really comparing "bug-prone" to "bug-safe", even though at the expense of certain burden.

+1 to both of these.

And personally, I've found making copy constructors explicit to just be troublesome (and I've never experimented with making move constructors explicit because of that), but I assume the people wanting this feature have the opposite experience, or they would have mentioned this, given that copy constructors are the most popular non-explicit single argument constructors.

The rule for warning generation would probably be: Warn when:

1. A constructor that could in C++ be used in conversions (single-argument or one where a second argument has a default value) that is not a copy or move constructor declared without attribute [[implicit]].
2. An explicit constructor declared with attribute [[implicit]].
   

Given your example:

Foo::Foo(std::string s, int i = 0);

It is potentially converting -- will trigger a warning

 template<typename... Ts>
 Foo::Foo(Ts&&...) { /* ... */ }

Is potentially buggy. Do you really want to construct from about *everything*? Such constructors are known to have caused bugs: http://ericniebler.com/2013/08/07/universal-references-and-the-copy-constructo/
They "compete" with copy/move constructors, if not anything else. Nonetheless, it is potentially converting -- will trigger a warning

We are talking about warnings: programmers will surely be allowed to turn them off, when they feel uncomfortable.

 Regards,
&rzej

 

[Nevin Liber]

On Thu, Sep 29, 2016 at 12:54 AM, Andrzej Krzemieński <akrz...@gmail.com> wrote:

Is potentially buggy. Do you really want to construct from about *everything*? Such constructors are known to have caused bugs: http://ericniebler.com/2013/08/07/universal-references-and-the-copy-constructo/
They "compete" with copy/move constructors, if not anything else. Nonetheless, it is potentially converting -- will trigger a warning

I'm confused.  Given that forwarding references are always an exact match without any conversion, how does this proposal address either "construct from about *everything*" or the problem of competing with copy/move constructors?  Please elaborate, because I just don't see it.

[Andrzej Krzemieński]

 
Sorry, It looks I unnecessarily spawned a child thread. It was meant to be a side note on forwarding constructors, not in the context of [[implicit]]: "Unconstrained forwarding constructors forward too much -- consider not using them (rather than adding attributes)"

The only connection between forwarding constructors and [[implicit]] is the answer to your challenge:

 template<typename... Ts>
 Foo::Foo(Ts&&...) { /* ... */ }

It is potentially converting: mark it as either 'explicit' or '[[implicit]]'.

Regards,
&rzej;

[Nicol Bolas]

In your example, `Foo` can be implicitly converted from any type. Therefore, it is legal to do `Foo f = 1`. It is also legal to call `func(1)`, where `func` takes a `Foo`. All of those provoke implicit conversions through your take-anything-constructor.

These are almost never what the idea behind that constructor is for. The point of such a constructor is not to provoke a bunch of implicit conversions. The writer of `Foo` should have marked that constructor `explicit`.

The warning is there to let the user know that they did not. [[implicit]] is there for the occasion when they didn't forget `explicit`; they truly intended for it to be a converting constructor.

[Matthew Woehlke]

On 2016-09-28 00:56, Jens Åkerblom wrote:
> Wouldn't it be better to depricate the explicit keyword in favor of having it by default? That is, replace "explicit" with "implicit".

Not really; this would break 95%¹ of existing C++ code. At best, you
could add `implicit` and have a transition period where not specifying
either is deprecated. Said period would have to be very long, possibly
even indefinite.

(¹ I'll grant the existence of a very small body of C++ code that does
not have classes with user defined constructors.)

--
Matthew