[PATCH] D79624: [NFC][DwarfDebug] Prefer explicit to auto type deduction

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D79624#2030991 <https://reviews.llvm.org/D79624#2030991>, @djtodoro wrote:

> In D79624#2030876 <https://reviews.llvm.org/D79624#2030876>, @dblaikie wrote:
>
> > > We should use explicit type instead of auto type deduction when the type is so obvious.
> >
> > This seems like it goes against the LLVM Style Guide, which says "Don’t “almost always” use auto, but **do use auto with** initializers like cast<Foo>(...) or other **places where the type is already obvious from the context**."
>
>
> Oh, I see... My intuition went in wrong direction.


The style guide doesn't cover the case you're probably thinking of, but everyone tends to agree - 'auto' is great when a type is really long/unnecessary, like an iterator type, etc - which is sort of the opposite end of the spectrum. The /specific/ type there might be fairly non-obvious, but usually what you can do with it ("oh, it's an iterator, great - I'll do iterator things with it") is obvious.

>> I don't think it's super necessary (I wouldn't advocate for someone changing this code to use auto alone, anymore than I advocate for the change to remove auto here), but equally I'm not sure this change was warranted - please revert it.
> 
> I wanted to remove ambiguity, but this may not be the best way.

Honestly, it might not be a bad change/way to write code - I was/am just a bit skittish about the specific motivation. The general idea of "use auto if it makes the code easier to understand" is a very fuzzy/uncertain thing, really.

> I reverted this. Thanks for the comment.

Thanks a bunch! I committed the dereferencing part of your fix though ( aa99da5ace4587440973c97a4cd5f486e7bb3c33 <https://reviews.llvm.org/rGaa99da5ace4587440973c97a4cd5f486e7bb3c33> ) which is certainly good to do (shouldn't be binding "auto&" to a pointer type if it's obviously a pointer/something readers will want to treat pointer-like). Thanks for that!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79624/new/

https://reviews.llvm.org/D79624