[llvm-dev] RFC: changing variable naming rules in LLVM codebase

[Rui Ueyama via llvm-dev]

On Sat, Jul 13, 2019 at 4:13 AM David Blaikie via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> On Fri, Jul 12, 2019 at 10:31 AM David Greene <dag at cray.com> wrote:
>
>> David Blaikie <dblaikie at gmail.com> writes:
>>
>> > Why would enums be less elegant than named boolean constants as you've
>> shown here?
>>
>> Casting, mainly.  If the parameters were also changed to an enum type
>> that would be fine too, probably better than inline variables even.
>>
>
> 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.
>

FWIW, I found that clang-tidy can fix up /*foo=*/-style comments. I ran the
tool and submitted it as https://reviews.llvm.org/rL366177.


>
>>
>> >
>> http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html
>> > (at a random googling for "bool parameters considered harmful")
>> > reasonably suggests splitting functions, but that's not always
>> > practical - often lots of common code, etc (but writing wrappers isn't
>> > too bad in that case either - two different functions with different
>> > names that both call the common one with the boolean parameter - so
>> > it's not hard to trace from the call to the implementation to know
>> > what that bool does, and is obvious by these two side-by-side
>> > differently named functions) and the comments on that article discuss
>> > enums as well.
>>
>> I like the two functions idea.  Of course scaling becomes a problem with
>> multiple Boolean parameters, as pointed out in the comments.
>>
>>                            -David
>>
>> > On Fri, Jul 12, 2019 at 9:04 AM David Greene via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> >
>> >  Rui Ueyama via llvm-dev <llvm-dev at lists.llvm.org> writes:
>> >
>> >  >  - LLVM's `/*foo=*/`-style comment to annotate function arguments
>> need
>> >  >  to be handled automatically to make the tool scalable. So is the
>> >  >  doxygen @parameter.
>> >
>> >  This is a bit of a side note, but in my own work I've more recently
>> >  tried to move from this style:
>> >
>> >  foo.h
>> >
>> >  int foo(int a, bool doSomething);
>> >
>> >  foo.cpp
>> >
>> >  x = foo(a, /*doSomething=*/true);
>> >  y = foo(a, /*doSomething=*/false);
>> >
>> >  to something like:
>> >
>> >  foo.h
>> >
>> >  inline constexpr DoDoSomething   = true;
>> >  inline constexpr DontDoSomething = false;
>> >
>> >  int foo(int a, bool doSomething);
>> >
>> >  foo.cpp
>> >
>> >  x = foo(a, DoDoSomething);
>> >  y = foo(a, DontDoSomething);
>> >
>> >  One doesn't need the inline variable (and thus not C++17) if one uses
>> >  macros or enums or something else less elegant.
>> >
>> >  This kind of thing is slightly more cumbersome to do if the parameter
>> >  takes a wide range of values, but the most common place I see the
>> former
>> >  style is for Boolean arguments.  Nevertheless, the wide-ranging case
>> can
>> >  be handled:
>> >
>> >  bar.h
>> >
>> >  inline int Threshold(int x) { return x; }
>> >
>> >  int bar(int a, int threshold);
>> >
>> >  bar.cpp
>> >
>> >  x = bar(a, Threshold(1000));
>> >  y = bar(a, Threshold(100));
>> >
>> >  With either technique the "named values" could be wrapped in their own
>> >  namespaces to avoid collisions.
>> >
>> >  I wonder if there is any appetite for doing something similar in LLVM.
>> >
>> >                         -David
>> >  _______________________________________________
>> >  LLVM Developers mailing list
>> >  llvm-dev at lists.llvm.org
>> >  https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190716/a378c6e4/attachment.html>