[llvm-dwarfdump] Interface cleanup. NFC

[MaskRay]

Summary

This patch moves interface declarations into llvm-dwarfdump.h and wrap
declarations in anonymous namespaces as appropriate. At the same time,
the externals are moved into the llvm::dwarfdump namespace`.

[djtodoro]

This looks good (from my side). Thanks.

[jhenderson]

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?

[MaskRay]

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

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

[jhenderson]

In D77848#1981137, @MaskRay wrote:

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

[MaskRay]

In D77848#1982839, @jhenderson wrote:

In D77848#1981137, @MaskRay wrote:

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

llvm/tools/llvm-dwarfdump/SectionSizes.cpp

 //===-- SectionSizes.cpp - Debug section sizes ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "SectionSizes.h"
+#include "llvm-dwarfdump.h"
 
 #define DEBUG_TYPE "dwarfdump"
 
 using namespace llvm;
-using namespace object;
+using namespace llvm::dwarfdump;
+using namespace llvm::object;
 
 static size_t getNameColumnWidth(const SectionSizes &Sizes,
                                  const StringRef SectionNameTitle) {
   // The minimum column width should be the size of "SECTION".
   size_t Width = SectionNameTitle.size();
   for (const auto &DebugSec : Sizes.DebugSectionSizes) {
     StringRef SectionName = DebugSec.getKey();
     Width = std::max(Width, SectionName.size());
static void prettyPrintSectionSizes(const ObjectFile &Obj,
   OS << " Total Size: " << Sizes.TotalDebugSectionsSize << "  ("
      << format("%0.2f", Sizes.TotalDebugSectionsSize /
                             static_cast<double>(Sizes.TotalObjectSize) * 100)
      << "%)\n";
   OS << " Total File Size: " << Sizes.TotalObjectSize << '\n';
   OS << "----------------------------------------------------" << '\n';
 }
 
-void llvm::calculateSectionSizes(const ObjectFile &Obj, SectionSizes &Sizes,
+void dwarfdump::calculateSectionSizes(const ObjectFile &Obj,
+                                      SectionSizes &Sizes,
                                       const Twine &Filename) {
   // Get total size.
   Sizes.TotalObjectSize = Obj.getData().size();
 
   for (const SectionRef &Section : Obj.sections()) {
     StringRef SectionName;
     if (Expected<StringRef> NameOrErr = Section.getName())
       SectionName = *NameOrErr;
     else
       WithColor::defaultWarningHandler(
           createFileError(Filename, NameOrErr.takeError()));
 
     LLVM_DEBUG(dbgs() << SectionName.str() << ": " << Section.getSize()
                       << '\n');
 
     if (!Section.isDebugSection(SectionName))
       continue;
 
     Sizes.TotalDebugSectionsSize += Section.getSize();
     Sizes.DebugSectionSizes[SectionName] += Section.getSize();
   }
 }
 
-bool collectObjectSectionSizes(ObjectFile &Obj, DWARFContext & /*DICtx*/,
-                               const Twine &Filename, raw_ostream &OS) {
+bool dwarfdump::collectObjectSectionSizes(ObjectFile &Obj,
+                                          DWARFContext & /*DICtx*/,
+                                          const Twine &Filename,
+                                          raw_ostream &OS) {
   SectionSizes Sizes;
 
   // Get the section sizes.
   calculateSectionSizes(Obj, Sizes, Filename);
 
   OS << "----------------------------------------------------\n";
   OS << "file: " << Filename.str() << '\n';
 
   prettyPrintSectionSizes(Obj, Sizes, OS);
 
   // TODO: If the input file is an archive, print the cumulative summary of all
   // files from the archive.
 
   return true;
 }

llvm/tools/llvm-dwarfdump/Statistics.cpp

 //===-- Statistics.cpp - Debug Info quality metrics -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm-dwarfdump.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/JSON.h"
 
-#include "SectionSizes.h"
-
 #define DEBUG_TYPE "dwarfdump"
 using namespace llvm;
-using namespace object;
+using namespace llvm::dwarfdump;
+using namespace llvm::object;
 
 /// This represents the number of categories of debug location coverage being
 /// calculated. The first category is the number of variables with 0% location
 /// coverage, but the last category is the number of variables with 100%
 /// location coverage.
 constexpr int NumOfCoverageCategories = 12;
 
+namespace {
 /// Holds statistics for one function (or other entity that has a PC range and
 /// contains variables, such as a compile unit).
 struct PerFunctionStats {
   /// Number of inlined instances of this function.
   unsigned NumFnInlined = 0;
   /// Number of out-of-line instances of this function.
   unsigned NumFnOutOfLine = 0;
   /// Number of inlined instances that have abstract origins.
std::vector<unsigned> VarNonEntryValLocStats{
       std::vector<unsigned>(NumOfCoverageCategories, 0)};
   /// Total number of local variables and function parameters processed.
   unsigned NumVarParam = 0;
   /// Total number of formal parameters processed.
   unsigned NumParam = 0;
   /// Total number of local variables processed.
   unsigned NumVar = 0;
 };
+} // namespace
 
 /// Collect debug location statistics for one DIE.
 static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
                             std::vector<unsigned> &VarParamLocStats,
                             std::vector<unsigned> &ParamLocStats,
                             std::vector<unsigned> &VarLocStats, bool IsParam,
                             bool IsLocalVar) {
   auto getCoverageBucket = [BytesCovered, BytesInScope]() -> unsigned {

 /// Collect debug info quality metrics for an entire DIContext.
 ///
 /// Do the impossible and reduce the quality of the debug info down to a few
 /// numbers. The idea is to condense the data into numbers that can be tracked
 /// over time to identify trends in newer compiler versions and gauge the effect
 /// of particular optimizations. The raw numbers themselves are not particularly
 /// useful, only the delta between compiling the same program with different
 /// compilers is.
-bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
-                               const Twine &Filename, raw_ostream &OS) {
+bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
+                                          const Twine &Filename,
+                                          raw_ostream &OS) {
   StringRef FormatName = Obj.getFileFormatName();
   GlobalStats GlobalStats;
   LocationStats LocStats;
   StringMap<PerFunctionStats> Statistics;
   for (const auto &CU : static_cast<DWARFContext *>(&DICtx)->compile_units())
     if (DWARFDie CUDie = CU->getNonSkeletonUnitDIE(false))
       collectStatsRecursive(CUDie, "/", "g", 0, 0, Statistics, GlobalStats,
                             LocStats);

llvm/tools/llvm-dwarfdump/llvm-dwarfdump.h

-//===- SectionSizes.h - Debug section sizes ---------------------*- C++ -*-===//
+//===-- llvm-dwarfdump - Debug info dumping utility -------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-//===-----------------------------------------------------------------------===/
+//===----------------------------------------------------------------------===//
 
-#ifndef LLVM_TOOLS_SECTION_SIZES_H
-#define LLVM_TOOLS_SECTION_SIZES_H
+#ifndef LLVM_TOOLS_LLVM_DWARFDUMP_LLVM_DWARFDUMP_H
+#define LLVM_TOOLS_LLVM_DWARFDUMP_LLVM_DWARFDUMP_H
 
-#include "llvm/DebugInfo/DIContext.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
-
-using SectionSizeMap = StringMap<uint64_t>;
+namespace dwarfdump {
 
 /// Holds cumulative section sizes for an object file.
 struct SectionSizes {
   /// Map of .debug section names and their sizes across all such-named
   /// sections.
-  SectionSizeMap DebugSectionSizes;
+  StringMap<uint64_t> DebugSectionSizes;
   /// Total number of bytes of all sections.
   uint64_t TotalObjectSize = 0;
   /// Total number of bytes of all debug sections.
   uint64_t TotalDebugSectionsSize = 0;
 };
 
 /// Calculate the section sizes.
 void calculateSectionSizes(const object::ObjectFile &Obj, SectionSizes &Sizes,
                            const Twine &Filename);
 
-} // end namespace llvm
+bool collectStatsForObjectFile(object::ObjectFile &Obj, DWARFContext &DICtx,
+                               const Twine &Filename, raw_ostream &OS);
+bool collectObjectSectionSizes(object::ObjectFile &Obj, DWARFContext &DICtx,
+                               const Twine &Filename, raw_ostream &OS);
+
+} // namespace dwarfdump
+} // namespace llvm
 
-#endif // LLVM_TOOLS_SECTION_SIZES_H
+#endif

llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp

 //===-- llvm-dwarfdump.cpp - Debug info dumping utility for llvm ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This program is a utility that works like "dwarfdump".
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm-dwarfdump.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstdlib>
 
 using namespace llvm;
-using namespace object;
+using namespace llvm::dwarfdump;
+using namespace llvm::object;
 
+namespace {
 /// Parser for options that take an optional offest argument.
 /// @{
 struct OffsetOption {
   uint64_t Val = 0;
   bool HasValue = false;
   bool IsRequested = false;
 };
+} // namespace
 
 namespace llvm {
 namespace cl {
 template <>
 class parser<OffsetOption> final : public basic_parser<OffsetOption> {
 public:
   parser(Option &O) : basic_parser(O) {}
 
void printOptionDiff(const Option &O, OffsetOption V, OptVal Default,
                        size_t GlobalWidth) const {
     printOptionName(O, GlobalWidth);
     outs() << "[=offset]";
   }
 
   // An out-of-line virtual method to provide a 'home' for this class.
   void anchor() override {};
 };
-} // cl
-} // llvm
+} // namespace cl
+} // namespace llvm
 
 /// @}
 /// Command line options.
 /// @{
 
 namespace {
 using namespace cl;
 
static bool lookup(ObjectFile &Obj, DWARFContext &DICtx, uint64_t Address,
   // object::SectionedAddress::UndefSection works for only absolute addresses.
   if (DILineInfo LineInfo = DICtx.getLineInfoForAddress(
           {Lookup, object::SectionedAddress::UndefSection}))
     LineInfo.dump(OS);
 
   return true;
 }
 
-bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
-                               const Twine &Filename, raw_ostream &OS);
-
-bool collectObjectSectionSizes(ObjectFile &Obj, DWARFContext & /*DICtx*/,
-                               const Twine &Filename, raw_ostream &OS);
-
 static bool dumpObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
                            const Twine &Filename, raw_ostream &OS) {
   logAllUnhandledErrors(DICtx.loadRegisterInfo(Obj), errs(),
                         Filename.str() + ": ");
   // The UUID dump already contains all the same information.
   if (!(DumpType & DIDT_UUID) || DumpType == DIDT_All)
     OS << Filename << ":\tfile format " << Obj.getFileFormatName() << '\n';