[PATCH] D60577: [X86AsmPrinter] refactor static functions into private methods. NFC
Eric Christopher via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 11 15:29:14 PDT 2019
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
In D60577#1463553 <https://reviews.llvm.org/D60577#1463553>, @smeenai wrote:
> In D60577#1463552 <https://reviews.llvm.org/D60577#1463552>, @srhines wrote:
>
> > In D60577#1463469 <https://reviews.llvm.org/D60577#1463469>, @smeenai wrote:
> >
> > > I have zero context on this change specifically, but "we should prefer functions in a private namespace" seems to be the exact opposite of the advice in https://llvm.org/docs/CodingStandards.html#static
> >
> >
> > While the commit comment references possibly moving things to a private namespace (which I agree is in contradiction to the referenced page), the actual code moves these to being private member functions. For functions that are taking `*this`, or propagating a `*this` in all usage, it seems fairly appropriate to me. Updating the commit message to not disagree with the official guidance would be helpful.
>
>
> Yup, the change itself makes perfect sense, just that part of the commit message stood out :)
Agreed. That said, might be good to clean up the commit message with what you mean.
LGTM and thanks :)
-eric
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60577/new/
https://reviews.llvm.org/D60577
More information about the llvm-commits
mailing list