Page MenuHomePhabricator

[llvm-profdata] --hot-func-list: fix some style issues in D81800
ClosedPublic

Authored by MaskRay on Jun 24 2020, 2:08 PM.

Diff Detail

Event Timeline

MaskRay created this revision.Jun 24 2020, 2:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 24 2020, 2:08 PM
wenlei added a reviewer: wmi.Jun 24 2020, 2:17 PM
hoyFB accepted this revision.Jun 24 2020, 2:19 PM

Thanks for fixing these issues!

This revision is now accepted and ready to land.Jun 24 2020, 2:19 PM

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

MaskRay retitled this revision from [llvm-profdata] --hot-func-list: fix some style issues in D82355 to [llvm-profdata] --hot-func-list: fix some style issues in D81800.Jun 24 2020, 2:56 PM

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

I feel so. For longer type names like std::pair<..., ...> or std::map<...>::iterator people don't typically write the underlying type. I agree with such practice. However, in this file, when we can write a single identifier instead auto , I think we should stick with the rule.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

Thanks. Corrected the title.

wenlei accepted this revision.Jun 24 2020, 3:06 PM

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

I feel so. For longer type names like std::pair<..., ...> or std::map<...>::iterator people don't typically write the underlying type. I agree with such practice. However, in this file, when we can write a single identifier instead auto , I think we should stick with the rule.

Ok, I'm not aware of the single identifier rule, but makes sense to me. Thanks for the touch up.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

Thanks. Corrected the title.

This revision was automatically updated to reflect the committed changes.