[LLD] [COFF] Wrap file location pair<StringRef,int> in Optional<>. NFC.

[mstorsjo]

Summary

IIRC this was requested in some earlier review touching this code.

It makes returning of None and checking for a failed response clearer.

[MaskRay]

I am in favor of this change. It makes the error case clearer.

lld/COFF/PDB.cpp

1818

return {{filename, *lineNumber}}; works (but I am not sure whether others like it)

[mstorsjo]

lld/COFF/PDB.cpp

1818

Ah, that looks totally fine to me, and is nicely compact. std::make_pair() works as well, allowing omitting the template parameters.

[rnk]

lld/COFF/PDB.cpp

1818

I like either make_pair or double braces, as long we don't have to write the template arguments.

[mstorsjo]

Updated to use std::make_pair, which makes it consistent with lld/ELF.

[ruiu]

LGTM

lld/COFF/PDB.h

 //===- PDB.h ----------------------------------------------------*- 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 LLD_COFF_PDB_H
 #define LLD_COFF_PDB_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace llvm {
 namespace codeview {
 union DebugInfo;
 }
 }
 
 namespace lld {
 namespace coff {
 class OutputSection;
 class SectionChunk;
 class SymbolTable;
 
 void createPDB(SymbolTable *symtab,
                llvm::ArrayRef<OutputSection *> outputSections,
                llvm::ArrayRef<uint8_t> sectionTable,
                llvm::codeview::DebugInfo *buildId);
 
-std::pair<llvm::StringRef, uint32_t> getFileLineCodeView(const SectionChunk *c,
-                                                         uint32_t addr);
-}
-}
+llvm::Optional<std::pair<llvm::StringRef, uint32_t>>
+getFileLineCodeView(const SectionChunk *c, uint32_t addr);
+} // namespace coff
+} // namespace lld
 
 #endif

lld/COFF/PDB.cpp

   }
 
   return false;
 }
 
 // Use CodeView line tables to resolve a file and line number for the given
 // offset into the given chunk and return them, or {"", 0} if a line table was
 // not found.
-std::pair<StringRef, uint32_t> getFileLineCodeView(const SectionChunk *c,
-                                                         uint32_t addr) {
+Optional<std::pair<StringRef, uint32_t>>
+getFileLineCodeView(const SectionChunk *c, uint32_t addr) {
   ExitOnError exitOnErr;
 
   DebugStringTableSubsectionRef cVStrTab;
   DebugChecksumsSubsectionRef checksums;
   DebugLinesSubsectionRef lines;
   uint32_t offsetInLinetable;
 
   if (!findLineTable(c, addr, cVStrTab, checksums, lines, offsetInLinetable))
-    return {"", 0};
+    return None;
 
   Optional<uint32_t> nameIndex;
   Optional<uint32_t> lineNumber;
   for (LineColumnEntry &entry : lines) {
     for (const LineNumberEntry &ln : entry.LineNumbers) {
       LineInfo li(ln.Flags);
       if (ln.Offset > offsetInLinetable) {
         if (!nameIndex) {
           nameIndex = entry.NameIndex;
           lineNumber = li.getStartLine();
         }
         StringRef filename =
             exitOnErr(getFileName(cVStrTab, checksums, *nameIndex));
-        return {filename, *lineNumber};
+        return std::pair<StringRef, uint32_t>{filename, *lineNumber};
       }
       nameIndex = entry.NameIndex;
       lineNumber = li.getStartLine();
     }
   }
   if (!nameIndex)
-    return {"", 0};
+    return None;
   StringRef filename = exitOnErr(getFileName(cVStrTab, checksums, *nameIndex));
-  return {filename, *lineNumber};
+  return std::pair<StringRef, uint32_t>{filename, *lineNumber};
 }
 
 } // namespace coff
 } // namespace lld

lld/COFF/SymbolTable.cpp

static std::vector<std::string> getSymbolLocations(BitcodeFile *file) {
   std::string res("\n>>> referenced by ");
   StringRef source = file->obj->getSourceFileName();
   if (!source.empty())
     res += source.str() + "\n>>>               ";
   res += toString(file);
   return {res};
 }
 
-static std::pair<StringRef, uint32_t> getFileLineDwarf(const SectionChunk *c,
-                                                       uint32_t addr) {
+static Optional<std::pair<StringRef, uint32_t>>
+getFileLineDwarf(const SectionChunk *c, uint32_t addr) {
   if (!config->symbolizer)
     config->symbolizer = make<symbolize::LLVMSymbolizer>();
   Expected<DILineInfo> expectedLineInfo = config->symbolizer->symbolizeCode(
       *c->file->getCOFFObj(), {addr, c->getSectionNumber() - 1});
   if (!expectedLineInfo)
-    return {"", 0};
+    return None;
   const DILineInfo &lineInfo = *expectedLineInfo;
   if (lineInfo.FileName == DILineInfo::BadString)
-    return {"", 0};
-  return {saver.save(lineInfo.FileName), lineInfo.Line};
+    return None;
+  return std::pair<StringRef, uint32_t>{saver.save(lineInfo.FileName),
+                                        lineInfo.Line};
 }
 
-static std::pair<StringRef, uint32_t> getFileLine(const SectionChunk *c,
-                                                  uint32_t addr) {
+static Optional<std::pair<StringRef, uint32_t>>
+getFileLine(const SectionChunk *c, uint32_t addr) {
   // MinGW can optionally use codeview, even if the default is dwarf.
-  std::pair<StringRef, uint32_t> fileLine = getFileLineCodeView(c, addr);
+  Optional<std::pair<StringRef, uint32_t>> fileLine =
+      getFileLineCodeView(c, addr);
   // If codeview didn't yield any result, check dwarf in MinGW mode.
-  if (fileLine.first.empty() && config->mingw)
+  if (!fileLine && config->mingw)
     fileLine = getFileLineDwarf(c, addr);
   return fileLine;
 }
 
 // Given a file and the index of a symbol in that file, returns a description
 // of all references to that symbol from that file. If no debug information is
 // available, returns just the name of the file, else one string per actual
 // reference as described in the debug info.
 std::vector<std::string> getSymbolLocations(ObjFile *file, uint32_t symIndex) {
   struct Location {
     Symbol *sym;
     std::pair<StringRef, uint32_t> fileLine;
   };
   std::vector<Location> locations;
 
   for (Chunk *c : file->getChunks()) {
     auto *sc = dyn_cast<SectionChunk>(c);
     if (!sc)
       continue;
     for (const coff_relocation &r : sc->getRelocs()) {
       if (r.SymbolTableIndex != symIndex)
         continue;
-      std::pair<StringRef, uint32_t> fileLine =
+      Optional<std::pair<StringRef, uint32_t>> fileLine =
           getFileLine(sc, r.VirtualAddress);
       Symbol *sym = getSymbol(sc, r.VirtualAddress);
-      if (!fileLine.first.empty() || sym)
-        locations.push_back({sym, fileLine});
+      if (fileLine)
+        locations.push_back({sym, *fileLine});
+      else if (sym)
+        locations.push_back({sym});
     }
   }
 
   if (locations.empty())
     return std::vector<std::string>({"\n>>> referenced by " + toString(file)});
 
   std::vector<std::string> symbolLocations(locations.size());
   size_t i = 0;