[llvm-dev] clang-tidy makes review a pain

[Stephen Kelly via llvm-dev]

On 21/04/2021 00:34, David Blaikie wrote:
> On Tue, Apr 20, 2021 at 3:39 PM Stephen Kelly via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> On 19/04/2021 20:32, David Blaikie via llvm-dev wrote:
>>> eh, I think that's probably the wrong direction for LLVM, actually - I
>>> think we've generally encouraged "const" being explicit when it's
>>> otherwise wrapped up in "auto" - same as for *.
>> Neither make sense to be to be honest. I'd very much like to see
>> clang-tidy in reviews not complain about it. The '*' is quite easy to
>> miss and
>>
>>       const auto *j1 = getPointer();
>>       const auto j2 = getPointer();
>>
>> mean very different things.
> They do, which to me I think means it's valuable/important to include
> both const and * to clarify which thing is intended. It's valuable to
> know that something is a pointer - cheap to copy, non-owning (not a
> unique_ptr, don't have to use std::move on it), etc. It doesn't mean
> every type that is cheap to copy and non-owning is documented in this
> way - but does help for some types without making the type name
> significantly longer/making expressions more unwieldy, etc.
>
> (I'm surprised there wasn't much more discussion around it (perhaps
> there was on an llvm-dev thread or the like) when this rule first went
> in: https://github.com/llvm-mirror/llvm/commit/fc9031cdffa3063ef747bd3a98833f164d07fc4a#diff-38d8333325264c104bb94d32db2248c0384fd39d7dbd8512fb4bb4939e3cf2a4
> or
>
>> The latter is also easier to port to a smart
>> (or dumb) pointer.
> I think it's more important that the code is a bit easier to read than
> easier to modify in this way.


Yes, I know not everyone sees it the same way (even though it's a common 
point of view outside of llvm somehow), but I just wanted to represent 
the alternative point of view :).

Also, for clarity, when I referred to "the latter" in my email, I was 
referring to non-use of both const and *. I just forgot to paste the

      auto j2 = getPointer();

code snippet which would then have been "latter".

Thanks,

Stephen.