Don't help the compiler!

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Don't help the compiler!

Csaba Raduly
Hi all,
STL (Stephan T. Lavavej) in his talk at Going Native 2013, gave this
example of an attempt to "help the compiler", which is at best
superfluous:

    auto p = std::make_pair<int, double>(42, 3.1415);

and recommended

    auto p = std::make_pair             (42, 3.1415);

I thought of writing a plugin to warn in the "helpful" case. However,
clang -std=c++11 -Xclang -ast-dump produced virtually identical output.

Is there a way to distinguish between these two situations?

Csaba
--
GCS a+ e++ d- C++ ULS$ L+$ !E- W++ P+++$ w++$ tv+ b++ DI D++ 5++
The Tao of math: The numbers you can count are not the real numbers.
Life is complex, with real and imaginary parts.
"Ok, it boots. Which means it must be bug-free and perfect. " -- Linus Torvalds
"People disagree with me. I just ignore them." -- Linus Torvalds

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

pair_expl.ast (3K) Download Attachment
pair_impl.ast (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Don't help the compiler!

Benjamin Kramer-2

> On 23.01.2015, at 17:32, Csaba Raduly <[hidden email]> wrote:
>
> Hi all,
> STL (Stephan T. Lavavej) in his talk at Going Native 2013, gave this
> example of an attempt to "help the compiler", which is at best
> superfluous:
>
>    auto p = std::make_pair<int, double>(42, 3.1415);
>
> and recommended
>
>    auto p = std::make_pair             (42, 3.1415);
>
> I thought of writing a plugin to warn in the "helpful" case. However,
> clang -std=c++11 -Xclang -ast-dump produced virtually identical output.
>
> Is there a way to distinguish between these two situations?

You can check the hasExplicitTemplateArgs bit on the DeclRefExpr to std::make_pair. We also have a clang-tidy checker for this pattern in ExplicitMakePairCheck.cpp that you can use as an example.

- Ben


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Don't help the compiler!

David Chisnall-4
In reply to this post by Csaba Raduly
On 23 Jan 2015, at 16:32, Csaba Raduly <[hidden email]> wrote:

>
> STL (Stephan T. Lavavej) in his talk at Going Native 2013, gave this
> example of an attempt to "help the compiler", which is at best
> superfluous:
>
>    auto p = std::make_pair<int, double>(42, 3.1415);
>
> and recommended
>
>    auto p = std::make_pair             (42, 3.1415);

The latter form seems less readable.  The fact that a floating point literal is a double unless suffixed with f is something that is rarely important for developers to remember, so this extra piece of information increases the cognitive burden when reading the code.

The goal of the former form is not to 'help the compiler', it is to help the poor sap who has to maintain this code in a couple of years.

David


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Don't help the compiler!

Jiří Zárevúcky
On 26 January 2015 at 10:15, David Chisnall <[hidden email]> wrote:
On 23 Jan 2015, at 16:32, Csaba Raduly <[hidden email]> wrote:
>
> STL (Stephan T. Lavavej) in his talk at Going Native 2013, gave this
> example of an attempt to "help the compiler", which is at best
> superfluous:
>
>    auto p = std::make_pair<int, double>(42, 3.1415);
>
> and recommended
>
>    auto p = std::make_pair             (42, 3.1415);

The latter form seems less readable.  The fact that a floating point literal is a double unless suffixed with f is something that is rarely important for developers to remember, so this extra piece of information increases the cognitive burden when reading the code.

The goal of the former form is not to 'help the compiler', it is to help the poor sap who has to maintain this code in a couple of years.


If you really need to think about whether it is float or double, then the unnecessary cognitive burden is somewhere else entirely.


-- J. Z.
 

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Don't help the compiler!

Nicola Gigante
In reply to this post by David Chisnall-4

Hi.

> Il giorno 26/gen/2015, alle ore 10:15, David Chisnall <[hidden email]> ha scritto:
>
> The latter form seems less readable.  The fact that a floating point literal is a double unless suffixed with f is something that is rarely important for developers to remember, so this extra piece of information increases the cognitive burden when reading the code.
>
> The goal of the former form is not to 'help the compiler', it is to help the poor sap who has to maintain this code in a couple of years.
>

While this might be true, the point of std::make_pair is to deduce the type of the arguments.
If one wants to explicitly specify the types, he can simply do:

auto p = std::pair<int,double>(42, 3.14);

or

std::pair<int,double> p{42, 3.14};

which are also a few less keystrokes. It doesn’t need make_pair, which is perfectly redundant if
you don’t want it to deduce the types.

Also in general, explicitly specifying the types of arguments of a template function which is
designed with deduction in mind, seems to me bad practice, because it’s fragile if one
change the arguments but not the types of vice-versa, and could break or slowdown things
when perfect-forwarding comes in the mix.

This example exacerbates the issue:
auto p = std::make_unique<T, ArgT1, ArgT2, ArgT3>(arg1, arg2, arg3); // Please don’t.

So IMHO a check to tell the inexperienced user about this redundancy is very helpful.

> David


Bye,
Nicola
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev