[PATCH] D77848: [llvm-dwarfdump] Interface cleanup. NFC

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