[ELF] - Make SymbolTable::addDefined return Defined.

[grimar]

Summary

Now it returns Symbol. This should be NFC that
avoids doing cast at the caller's sides.

[peter.smith]

This looks fine to me. It makes sense for addDefined to actually return a Defined.

[grimar]

Ping.

[peter.smith]

LGTM. I think that this one is low risk anyway.

[grimar]

In D54627#1306099, @peter.smith wrote:

LGTM. I think that this one is low risk anyway.

Thanks!

[ruiu]

lld/trunk/ELF/SymbolTable.cpp

477 ↗

(On Diff #175034)

If Cmp == 0, doesn't this cast fail with an assertion failure because S is not guaranteed to be a Defined symbol?

[grimar]

lld/trunk/ELF/SymbolTable.cpp

477 ↗

(On Diff #175034)

Note that if Cmp == 0 we call the reportDuplicate which do the cast<Defined> inside:
https://github.com/llvm-mirror/lld/blob/master/ELF/SymbolTable.cpp#L427

I think in all cases S here is a Defined.

ELF/SymbolTable.h

public:
   void wrap(Symbol *Sym, Symbol *Real, Symbol *Wrap);
 
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
 
   template <class ELFT>
   Symbol *addUndefined(StringRef Name, uint8_t Binding, uint8_t StOther,
                        uint8_t Type, bool CanOmitFromDynSym, InputFile *File);
 
-  Symbol *addDefined(StringRef Name, uint8_t StOther, uint8_t Type,
+  Defined *addDefined(StringRef Name, uint8_t StOther, uint8_t Type,
                       uint64_t Value, uint64_t Size, uint8_t Binding,
                       SectionBase *Section, InputFile *File);
 
   template <class ELFT>
   void addShared(StringRef Name, SharedFile<ELFT> &F,
                  const typename ELFT::Sym &Sym, uint32_t Alignment,
                  uint32_t VerdefIndex);
 
   template <class ELFT>
   void addLazyArchive(StringRef Name, ArchiveFile &F,

ELF/SymbolTable.cpp

if (!Src1.empty())
     Msg += Src1 + "\n>>>            ";
   Msg += Obj1 + "\n>>> defined at ";
   if (!Src2.empty())
     Msg += Src2 + "\n>>>            ";
   Msg += Obj2;
   error(Msg);
 }
 
-Symbol *SymbolTable::addDefined(StringRef Name, uint8_t StOther, uint8_t Type,
+Defined *SymbolTable::addDefined(StringRef Name, uint8_t StOther, uint8_t Type,
                                  uint64_t Value, uint64_t Size, uint8_t Binding,
                                  SectionBase *Section, InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, getVisibility(StOther),
                                     /*CanOmitFromDynSym*/ false, File);
   int Cmp = compareDefinedNonCommon(S, WasInserted, Binding, Section == nullptr,
                                     Value, Name);
   if (Cmp > 0)
     replaceSymbol<Defined>(S, File, Name, Binding, StOther, Type, Value, Size,
                            Section);
   else if (Cmp == 0)
     reportDuplicate(S, File, dyn_cast_or_null<InputSectionBase>(Section),
                     Value);
-  return S;
+  return cast<Defined>(S);
 }
 
 template <typename ELFT>
 void SymbolTable::addShared(StringRef Name, SharedFile<ELFT> &File,
                             const typename ELFT::Sym &Sym, uint32_t Alignment,
                             uint32_t VerdefIndex) {
   // DSO symbols do not affect visibility in the output, so we pass STV_DEFAULT
   // as the visibility, which will leave the visibility in the symbol table

ELF/Writer.cpp

 }
 
 static Defined *addOptionalRegular(StringRef Name, SectionBase *Sec,
                                    uint64_t Val, uint8_t StOther = STV_HIDDEN,
                                    uint8_t Binding = STB_GLOBAL) {
   Symbol *S = Symtab->find(Name);
   if (!S || S->isDefined())
     return nullptr;
-  Symbol *Sym = Symtab->addDefined(Name, StOther, STT_NOTYPE, Val,
-                                   /*Size=*/0, Binding, Sec,
-                                   /*File=*/nullptr);
-  return cast<Defined>(Sym);
+  return Symtab->addDefined(Name, StOther, STT_NOTYPE, Val,
 }
 
 static Defined *addAbsolute(StringRef Name) {
-  return cast<Defined>(Symtab->addDefined(Name, STV_HIDDEN, STT_NOTYPE, 0, 0,
-                                          STB_GLOBAL, nullptr, nullptr));
+  return Symtab->addDefined(Name, STV_HIDDEN, STT_NOTYPE, 0, 0, STB_GLOBAL,
+                            nullptr, nullptr);
 }
 
 // The linker is expected to define some symbols depending on
 // the linking result. This function defines such symbols.
 void elf::addReservedSymbols() {
   if (Config->EMachine == EM_MIPS) {
     // Define _gp for MIPS. st_value of _gp symbol will be updated by Writer
     // so that it points to an absolute address which by default is relative