<div dir="ltr"><div dir="ltr">On Sat, Jul 13, 2019 at 4:13 AM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 12, 2019 at 10:31 AM David Greene <<a href="mailto:dag@cray.com" target="_blank">dag@cray.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> writes:<br>
<br>
> Why would enums be less elegant than named boolean constants as you've shown here?<br>
<br>
Casting, mainly.  If the parameters were also changed to an enum type<br>
that would be fine too, probably better than inline variables even.<br></blockquote><div><br>Oh, yeah, I meant changing the parameter type to a custom enum - forcing users to have to use the more legible enum names rather than the inscrutible boolean constants.<br></div></div></div></blockquote><div><br></div><div>FWIW, I found that clang-tidy can fix up /*foo=*/-style comments. I ran the tool and submitted it as <a href="https://reviews.llvm.org/rL366177">https://reviews.llvm.org/rL366177</a>.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> <a href="http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html" rel="noreferrer" target="_blank">http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html</a><br>
> (at a random googling for "bool parameters considered harmful")<br>
> reasonably suggests splitting functions, but that's not always<br>
> practical - often lots of common code, etc (but writing wrappers isn't<br>
> too bad in that case either - two different functions with different<br>
> names that both call the common one with the boolean parameter - so<br>
> it's not hard to trace from the call to the implementation to know<br>
> what that bool does, and is obvious by these two side-by-side<br>
> differently named functions) and the comments on that article discuss<br>
> enums as well.<br>
<br>
I like the two functions idea.  Of course scaling becomes a problem with<br>
multiple Boolean parameters, as pointed out in the comments.<br>
<br>
                           -David<br>
<br>
> On Fri, Jul 12, 2019 at 9:04 AM David Greene via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
>  Rui Ueyama via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> writes:<br>
><br>
>  >  - LLVM's `/*foo=*/`-style comment to annotate function arguments need<br>
>  >  to be handled automatically to make the tool scalable. So is the<br>
>  >  doxygen @parameter.<br>
><br>
>  This is a bit of a side note, but in my own work I've more recently<br>
>  tried to move from this style:<br>
><br>
>  foo.h<br>
><br>
>  int foo(int a, bool doSomething);<br>
><br>
>  foo.cpp<br>
><br>
>  x = foo(a, /*doSomething=*/true);<br>
>  y = foo(a, /*doSomething=*/false);<br>
><br>
>  to something like:<br>
><br>
>  foo.h<br>
><br>
>  inline constexpr DoDoSomething   = true;<br>
>  inline constexpr DontDoSomething = false;<br>
><br>
>  int foo(int a, bool doSomething);<br>
><br>
>  foo.cpp<br>
><br>
>  x = foo(a, DoDoSomething);<br>
>  y = foo(a, DontDoSomething);<br>
><br>
>  One doesn't need the inline variable (and thus not C++17) if one uses<br>
>  macros or enums or something else less elegant.<br>
><br>
>  This kind of thing is slightly more cumbersome to do if the parameter<br>
>  takes a wide range of values, but the most common place I see the former<br>
>  style is for Boolean arguments.  Nevertheless, the wide-ranging case can<br>
>  be handled:<br>
><br>
>  bar.h<br>
><br>
>  inline int Threshold(int x) { return x; }<br>
><br>
>  int bar(int a, int threshold);<br>
><br>
>  bar.cpp<br>
><br>
>  x = bar(a, Threshold(1000));<br>
>  y = bar(a, Threshold(100));<br>
><br>
>  With either technique the "named values" could be wrapped in their own<br>
>  namespaces to avoid collisions.<br>
><br>
>  I wonder if there is any appetite for doing something similar in LLVM.<br>
><br>
>                         -David<br>
>  _______________________________________________<br>
>  LLVM Developers mailing list<br>
>  <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>  <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>