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

Wed Apr 15 10:24:27 PDT 2020

MaskRay added a comment.

In D77848#1982839 <https://reviews.llvm.org/D77848#1982839>, @jhenderson wrote:

> 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.

OK.

> Is there not already a Statistics.h?

Statistics.h does not exist. `llvm-dwarfdump.h` is now the only header.

>> 
>> 
>>> 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