Remove unnecessary .get() on smart pointers in isa, cast, and dyn_cast.

[jlebar]

Summary

Depends on D15910.

[dblaikie]

Looks good - a few optional comments. Feel free to commit with or without the suggested changes, as you see fit.

lib/DebugInfo/PDB/PDBContext.cpp

104

Might want to ensure we have '*'s on the LHS especially when casting from a smart pointer - might be extra confusing getting a raw pointer from a smart pointer.

lib/Object/ELFYAML.cpp

716

These might be more obvious (then there's no interesting smart pointer to raw pointer conversions, etc) if the * was inside the cast instead of outside - but not sure.

tools/llvm-pdbdump/ClassDefinitionDumper.cpp

86

some more missing '*' (I know, not introduced by this patch, but possibly exacerbated)

[jlebar]

Address dblaikie's review.

[jlebar]

Thank you for the review. I also like auto*; I went through and added that where applicable.

I'll check this in once we're agreed on D15910.

include/llvm/DebugInfo/PDB/IPDBSession.h

public:
   virtual std::unique_ptr<PDBSymbol> getSymbolById(uint32_t SymbolId) const = 0;
 
   template <typename T>
   std::unique_ptr<T> getConcreteSymbolById(uint32_t SymbolId) const {
     auto Symbol(getSymbolById(SymbolId));
     if (!Symbol)
       return nullptr;
 
-    T *ConcreteSymbol = dyn_cast<T>(Symbol.get());
+    T *ConcreteSymbol = dyn_cast<T>(Symbol);
     if (!ConcreteSymbol)
       return nullptr;
     Symbol.release();
     return std::unique_ptr<T>(ConcreteSymbol);
   }
 
   virtual std::unique_ptr<PDBSymbol>
   findSymbolByAddress(uint64_t Address, PDB_SymType Type) const = 0;

lib/DebugInfo/PDB/PDBContext.cpp

 DILineInfo PDBContext::getLineInfoForAddress(uint64_t Address,
                                              DILineInfoSpecifier Specifier) {
   DILineInfo Result;
   Result.FunctionName = getFunctionName(Address, Specifier.FNKind);
 
   uint32_t Length = 1;
   std::unique_ptr<PDBSymbol> Symbol =
       Session->findSymbolByAddress(Address, PDB_SymType::None);
-  if (auto Func = dyn_cast_or_null<PDBSymbolFunc>(Symbol.get())) {
+  if (auto Func = dyn_cast_or_null<PDBSymbolFunc>(Symbol)) {
     Length = Func->getLength();
-  } else if (auto Data = dyn_cast_or_null<PDBSymbolData>(Symbol.get())) {
+  } else if (auto Data = dyn_cast_or_null<PDBSymbolData>(Symbol)) {
     Length = Data->getLength();
   }
 
   // If we couldn't find a symbol, then just assume 1 byte, so that we get
   // only the line number of the first instruction.
   auto LineNumbers = Session->findLineNumbersByAddress(Address, Length);
   if (!LineNumbers || LineNumbers->getChildCount() == 0)
     return Result;
if (NameKind == DINameKind::None)
     return std::string();
 
   if (NameKind == DINameKind::LinkageName) {
     // It is not possible to get the mangled linkage name through a
     // PDBSymbolFunc.  For that we have to specifically request a
     // PDBSymbolPublicSymbol.
     auto PublicSym =
         Session->findSymbolByAddress(Address, PDB_SymType::PublicSymbol);
-    if (auto PS = dyn_cast_or_null<PDBSymbolPublicSymbol>(PublicSym.get()))
+    if (auto PS = dyn_cast_or_null<PDBSymbolPublicSymbol>(PublicSym))
       return PS->getName();
   }
 
   auto FuncSymbol =
       Session->findSymbolByAddress(Address, PDB_SymType::Function);
 
   // This could happen either if there was no public symbol (e.g. not
   // external) or the user requested the short name.  In the former case,
   // although they technically requested the linkage name, if the linkage
   // name is not available we fallback to at least returning a non-empty
   // string.
-  if (auto Func = dyn_cast_or_null<PDBSymbolFunc>(FuncSymbol.get()))
-      return Func->getName();
+  if (auto Func = dyn_cast_or_null<PDBSymbolFunc>(FuncSymbol))
 
   return std::string();
 }

lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp

 
 bool SymbolizableObjectFile::shouldOverrideWithSymbolTable(
     FunctionNameKind FNKind, bool UseSymbolTable) const {
   // When DWARF is used with -gline-tables-only / -gmlt, the symbol table gives
   // better answers for linkage names than the DIContext. Otherwise, we are
   // probably using PEs and PDBs, and we shouldn't do the override. PE files
   // generally only contain the names of exported symbols.
   return FNKind == FunctionNameKind::LinkageName && UseSymbolTable &&
-         isa<DWARFContext>(DebugInfoContext.get());
+         isa<DWARFContext>(DebugInfoContext);
 }
 
 DILineInfo SymbolizableObjectFile::symbolizeCode(uint64_t ModuleOffset,
                                                  FunctionNameKind FNKind,
                                                  bool UseSymbolTable) const {
   DILineInfo LineInfo;
   if (DebugInfoContext) {
     LineInfo = DebugInfoContext->getLineInfoForAddress(

lib/Object/ELFYAML.cpp

void MappingTraits<std::unique_ptr<ELFYAML::Section>>::mapping(
   else
     IO.mapRequired("Type", sectionType);
 
   switch (sectionType) {
   case ELF::SHT_REL:
   case ELF::SHT_RELA:
     if (!IO.outputting())
       Section.reset(new ELFYAML::RelocationSection());
-    sectionMapping(IO, *cast<ELFYAML::RelocationSection>(Section.get()));
+    sectionMapping(IO, *cast<ELFYAML::RelocationSection>(Section));
     break;
   case ELF::SHT_GROUP:
     if (!IO.outputting())
       Section.reset(new ELFYAML::Group());
-    groupSectionMapping(IO, *cast<ELFYAML::Group>(Section.get()));
+    groupSectionMapping(IO, *cast<ELFYAML::Group>(Section));
     break;
   case ELF::SHT_NOBITS:
     if (!IO.outputting())
       Section.reset(new ELFYAML::NoBitsSection());
-    sectionMapping(IO, *cast<ELFYAML::NoBitsSection>(Section.get()));
+    sectionMapping(IO, *cast<ELFYAML::NoBitsSection>(Section));
     break;
   case ELF::SHT_MIPS_ABIFLAGS:
     if (!IO.outputting())
       Section.reset(new ELFYAML::MipsABIFlags());
-    sectionMapping(IO, *cast<ELFYAML::MipsABIFlags>(Section.get()));
+    sectionMapping(IO, *cast<ELFYAML::MipsABIFlags>(Section));
     break;
   default:
     if (!IO.outputting())
       Section.reset(new ELFYAML::RawContentSection());
-    sectionMapping(IO, *cast<ELFYAML::RawContentSection>(Section.get()));
+    sectionMapping(IO, *cast<ELFYAML::RawContentSection>(Section));
   }
 }
 
 StringRef MappingTraits<std::unique_ptr<ELFYAML::Section>>::validate(
     IO &io, std::unique_ptr<ELFYAML::Section> &Section) {
-  const auto *RawSection = dyn_cast<ELFYAML::RawContentSection>(Section.get());
+  const auto *RawSection = dyn_cast<ELFYAML::RawContentSection>(Section);
   if (!RawSection || RawSection->Size >= RawSection->Content.binary_size())
     return StringRef();
   return "Section size must be greater or equal to the content size";
 }
 
 namespace {
 struct NormalizedMips64RelType {
   NormalizedMips64RelType(IO &)

lib/ProfileData/CoverageMappingReader.cpp

 loadBinaryFormat(MemoryBufferRef ObjectBuffer, InstrProfSymtab &ProfileNames,
                  StringRef &CoverageMapping, uint8_t &BytesInAddress,
                  support::endianness &Endian, StringRef Arch) {
   auto BinOrErr = object::createBinary(ObjectBuffer);
   if (std::error_code EC = BinOrErr.getError())
     return EC;
   auto Bin = std::move(BinOrErr.get());
   std::unique_ptr<ObjectFile> OF;
-  if (auto *Universal = dyn_cast<object::MachOUniversalBinary>(Bin.get())) {
+  if (auto *Universal = dyn_cast<object::MachOUniversalBinary>(Bin)) {
     // If we have a universal binary, try to look up the object for the
     // appropriate architecture.
     auto ObjectFileOrErr = Universal->getObjectForArch(Arch);
     if (std::error_code EC = ObjectFileOrErr.getError())
       return EC;
     OF = std::move(ObjectFileOrErr.get());
-  } else if (isa<object::ObjectFile>(Bin.get())) {
+  } else if (isa<object::ObjectFile>(Bin)) {
     // For any other object file, upcast and take ownership.
     OF.reset(cast<object::ObjectFile>(Bin.release()));
     // If we've asked for a particular arch, make sure they match.
     if (!Arch.empty() && OF->getArch() != Triple(Arch).getArch())
       return object_error::arch_not_found;
   } else
     // We can only handle object files.
     return coveragemap_error::malformed;

lib/Support/YAMLTraits.cpp

 }
 
 bool Input::bitSetMatch(const char *Str, bool) {
   if (EC)
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     unsigned Index = 0;
     for (auto &N : SQ->Entries) {
-      if (ScalarHNode *SN = dyn_cast<ScalarHNode>(N.get())) {
+      if (ScalarHNode *SN = dyn_cast<ScalarHNode>(N)) {
         if (SN->value().equals(Str)) {
           BitValuesUsed[Index] = true;
           return true;
         }
       } else {
         setError(CurrentNode, "unexpected scalar in sequence of bit values");
       }
       ++Index;

tools/dsymutil/BinaryHolder.cpp

for (auto MemRef : ArchiveBuffers) {
     CurrentArchives.push_back(std::move(*ErrOrArchive));
   }
   return GetArchiveMemberBuffers(Filename, Timestamp);
 }
 
 ErrorOr<const object::ObjectFile &>
 BinaryHolder::getObjfileForArch(const Triple &T) {
   for (const auto &Obj : CurrentObjectFiles) {
-    if (const auto *MachO = dyn_cast<object::MachOObjectFile>(Obj.get())) {
+    if (const auto *MachO = dyn_cast<object::MachOObjectFile>(Obj)) {
       if (getTriple(*MachO).str() == T.str())
         return *MachO;
     } else if (Obj->getArch() == T.getArch())
       return *Obj;
   }
 
   return make_error_code(object::object_error::arch_not_found);
 }

tools/llvm-pdbdump/ClassDefinitionDumper.cpp

void ClassDefinitionDumper::start(const PDBSymbolTypeUDT &Class) {
 
   while (auto Child = Children->getNext()) {
     PDB_MemberAccess Access = Child->getRawSymbol().getAccess();
     if (isa<PDBSymbolTypeBaseClass>(*Child))
       continue;
 
     auto &AccessGroup = Groups.find((int)Access)->second;
 
-    if (auto Func = dyn_cast<PDBSymbolFunc>(Child.get())) {
+    if (auto Func = dyn_cast<PDBSymbolFunc>(Child)) {
       if (Func->isCompilerGenerated() && opts::ExcludeCompilerGenerated)
         continue;
       if (Func->getLength() == 0 && !Func->isPureVirtual() &&
           !Func->isIntroVirtualFunction())
         continue;
       Child.release();
       AccessGroup.Functions.push_back(std::unique_ptr<PDBSymbolFunc>(Func));
-    } else if (auto Data = dyn_cast<PDBSymbolData>(Child.get())) {
+    } else if (auto Data = dyn_cast<PDBSymbolData>(Child)) {
       Child.release();
       AccessGroup.Data.push_back(std::unique_ptr<PDBSymbolData>(Data));
     } else {
       AccessGroup.Unknown.push_back(std::move(Child));
     }
   }
 
   int Count = 0;

tools/llvm-pdbdump/FunctionDumper.cpp

 }
 
 void FunctionDumper::dump(const PDBSymbolTypePointer &Symbol) {
   uint32_t PointeeId = Symbol.getTypeId();
   auto PointeeType = Symbol.getSession().getSymbolById(PointeeId);
   if (!PointeeType)
     return;
 
-  if (auto FuncSig = dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType.get())) {
+  if (auto FuncSig = dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType)) {
     FunctionDumper NestedDumper(Printer);
     PointerType Pointer =
         Symbol.isReference() ? PointerType::Reference : PointerType::Pointer;
     NestedDumper.start(*FuncSig, nullptr, Pointer);
   } else {
     if (Symbol.isConstType())
       WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
     if (Symbol.isVolatileType())

tools/llvm-pdbdump/TypedefDumper.cpp

void TypedefDumper::dump(const PDBSymbolTypePointer &Symbol) {
   if (Symbol.isConstType())
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
   if (Symbol.isVolatileType())
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "volatile ";
   uint32_t PointeeId = Symbol.getTypeId();
   auto PointeeType = Symbol.getSession().getSymbolById(PointeeId);
   if (!PointeeType)
     return;
-  if (auto FuncSig = dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType.get())) {
+  if (auto FuncSig = dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType)) {
     FunctionDumper::PointerType Pointer = FunctionDumper::PointerType::Pointer;
     if (Symbol.isReference())
       Pointer = FunctionDumper::PointerType::Reference;
     FunctionDumper NestedDumper(Printer);
     NestedDumper.start(*FuncSig, nullptr, Pointer);
   } else {
     PointeeType->dump(*this);
     Printer << ((Symbol.isReference()) ? "&" : "*");

tools/llvm-pdbdump/VariableDumper.cpp

 
 void VariableDumper::dump(const PDBSymbolTypeFunctionSig &Symbol) {}
 
 void VariableDumper::dump(const PDBSymbolTypePointer &Symbol) {
   auto PointeeType = Symbol.getPointeeType();
   if (!PointeeType)
     return;
 
-  if (auto Func = dyn_cast<PDBSymbolFunc>(PointeeType.get())) {
+  if (auto Func = dyn_cast<PDBSymbolFunc>(PointeeType)) {
     FunctionDumper NestedDumper(Printer);
     FunctionDumper::PointerType Pointer =
         Symbol.isReference() ? FunctionDumper::PointerType::Reference
                              : FunctionDumper::PointerType::Pointer;
     NestedDumper.start(*Func, Pointer);
   } else {
     if (Symbol.isConstType())
       WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";

 }
 
 void VariableDumper::dumpSymbolTypeAndName(const PDBSymbol &Type,
                                            StringRef Name) {
   if (auto *ArrayType = dyn_cast<PDBSymbolTypeArray>(&Type)) {
     std::string IndexSpec;
     raw_string_ostream IndexStream(IndexSpec);
     std::unique_ptr<PDBSymbol> ElementType = ArrayType->getElementType();
-    while (auto NestedArray = dyn_cast<PDBSymbolTypeArray>(ElementType.get())) {
+    while (auto NestedArray = dyn_cast<PDBSymbolTypeArray>(ElementType)) {
       IndexStream << "[";
       IndexStream << NestedArray->getCount();
       IndexStream << "]";
       ElementType = NestedArray->getElementType();
     }
     IndexStream << "[" << ArrayType->getCount() << "]";
     ElementType->dump(*this);
     WithColor(Printer, PDB_ColorItem::Identifier).get() << " " << Name;
     Printer << IndexStream.str();
   } else {
     if (!tryDumpFunctionPointer(Type, Name)) {
       Type.dump(*this);
       WithColor(Printer, PDB_ColorItem::Identifier).get() << " " << Name;
     }
   }
 }
 
 bool VariableDumper::tryDumpFunctionPointer(const PDBSymbol &Type,
                                             StringRef Name) {
   // Function pointers come across as pointers to function signatures.  But the
   // signature carries no name, so we have to handle this case separately.
   if (auto *PointerType = dyn_cast<PDBSymbolTypePointer>(&Type)) {
     auto PointeeType = PointerType->getPointeeType();
-    if (auto *FunctionSig =
-            dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType.get())) {
+    if (auto *FunctionSig = dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType)) {
       FunctionDumper Dumper(Printer);
       FunctionDumper::PointerType PT = FunctionDumper::PointerType::Pointer;
       if (PointerType->isReference())
         PT = FunctionDumper::PointerType::Reference;
       std::string NameStr(Name.begin(), Name.end());
       Dumper.start(*FunctionSig, NameStr.c_str(), PT);
       return true;
     }
   }
   return false;
 }