[PATCH] D77848: [llvm-dwarfdump] Interface cleanup. NFC
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 15 00:30:07 PDT 2020
jhenderson added a comment.
In D77848#1981137 <https://reviews.llvm.org/D77848#1981137>, @MaskRay wrote:
> In D77848#1980201 <https://reviews.llvm.org/D77848#1980201>, @jhenderson wrote:
>
> > I'd have rejected this change, partly, if I'd seen this before the commit. I REALLY hate it when definitions and declarations for functions are not in matching header/source files. Why did the declarations relating to the SectionSizes.cpp stuff get moved into an unrelated header?
>
>
> Not sure whether `SectionSizes.cpp` should be placed in its own file. Merging it into `Statistics.cpp` and adding a `Statistics.h`, does that sound good to you?
Merging the two seems reasonable in this case, since they're all to do with statistics, although I do find LLVM's propensity to put everything in one monolitihic file (see virtually every one of the tools in the tools directory) hard to work with.
Is there not already a Statistics.h?
>
>
>> definitions and declarations for functions are not in matching header/source files
>
> The previous state might be worse to you? A `.cpp` file made the declarations...
I missed that somehow. Thanks. That should indeed be fixed by putting it in a header.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77848/new/
https://reviews.llvm.org/D77848
More information about the llvm-commits
mailing list