[IR] Module's NamedMD table needn't be 'void *'

[modocache]

Summary

In July 21 2010 llvm::NamedMDNode was refactored such that it would no
longer subclass llvm::Value:
https://github.com/llvm/llvm-project/commit/2637cc1a38d7336ea30caf

As part of this change, a map type from metadata names to their named
metadata, llvm::MDSymbolTable, was deleted. In its place, the type
of member llvm::Module::NamedMDSymTab was changed, from
llvm::MDSymbolTable to void *. The underlying memory allocations
for this pointer were changed to new StringMap<NamedMDNode *>().

However, as far as I can tell, there's no need for obscuring the
underlying type being pointed to by the void *, and no need for
static casts from void * to StringMap. In fact, I don't think
there's a need for explicit calls to new and delete at all.

This commit changes NamedMDSymTab from a pointer to a reference, which
automatically couples its lifetime with the lifetime of its owning
llvm::Module instance, thus removing the explicit calls to new and
delete in the llvm::Module constructor and destructor. It also
changes the type from void * to a newly defined NamedMDSymTabType,
and removes the static casts.

Test Plan:
An ASAN-enabled build and run of check-all succeeds with this change
(aside from some tests that always fail for me in ASAN for some reason,
such as check-clang SemaTemplate/stack-exhaustion.cpp).

[dblaikie]

Sounds good -t hanks for all the context. Previously this might've been an effort to keep the dependencies of a core header like Module.h small, but since it now has the dependencies this relies on - this change isn't making the situation worse in any way I can think of.

[modocache]

Previously this might've been an effort to keep the dependencies of a core header like Module.h small, but since it now has the dependencies this relies on - this change isn't making the situation worse in any way I can think of.

I see, thanks, that makes sense. And thanks for the review!

[mehdi_amini]

llvm/lib/IR/Module.cpp

87

Nit: Is this clear useful/needed?

(I don't know about the other ones, but the previous code wasn't doing any explicit "clear()" of the map before deleting it).

[dblaikie]

llvm/lib/IR/Module.cpp

87

Faoir point - did that & cleaned up a couple of other things in 46ed93315fceec4c8c3cd3defada501a55eb96e2

llvm/include/llvm/IR/Module.h

public:
   /// The type for the list of aliases.
   using AliasListType = SymbolTableList<GlobalAlias>;
   /// The type for the list of ifuncs.
   using IFuncListType = SymbolTableList<GlobalIFunc>;
   /// The type for the list of named metadata.
   using NamedMDListType = ilist<NamedMDNode>;
   /// The type of the comdat "symbol" table.
   using ComdatSymTabType = StringMap<Comdat>;
+  /// The type for mapping names to named metadata.
+  using NamedMDSymTabType = StringMap<NamedMDNode *>;
 
   /// The Global Variable iterator.
   using global_iterator = GlobalListType::iterator;
   /// The Global Variable constant iterator.
   using const_global_iterator = GlobalListType::const_iterator;
 
   /// The Function iterators.
   using iterator = FunctionListType::iterator;
OwnedMemoryBuffer;              ///< Memory buffer directly owned by this
                                   ///< module, for legacy clients only.
   std::unique_ptr<GVMaterializer>
   Materializer;                   ///< Used to materialize GlobalValues
   std::string ModuleID;           ///< Human readable identifier for the module
   std::string SourceFileName;     ///< Original source file name for module,
                                   ///< recorded in bitcode.
   std::string TargetTriple;       ///< Platform target triple Module compiled on
                                   ///< Format: (arch)(sub)-(vendor)-(sys0-(abi)
-  void *NamedMDSymTab;            ///< NamedMDNode names.
+  NamedMDSymTabType NamedMDSymTab;  ///< NamedMDNode names.
   DataLayout DL;                  ///< DataLayout associated with the module
 
   friend class Constant;
 
 /// @}
 /// @name Constructors
 /// @{
 public:

llvm/lib/IR/Module.cpp

 
 //===----------------------------------------------------------------------===//
 // Primitive Module methods.
 //
 
 Module::Module(StringRef MID, LLVMContext &C)
     : Context(C), Materializer(), ModuleID(MID), SourceFileName(MID), DL("") {
   ValSymTab = new ValueSymbolTable();
-  NamedMDSymTab = new StringMap<NamedMDNode *>();
   Context.addModule(this);
 }
 
 Module::~Module() {
   Context.removeModule(this);
   dropAllReferences();
   GlobalList.clear();
   FunctionList.clear();
   AliasList.clear();
   IFuncList.clear();
   NamedMDList.clear();
+  NamedMDSymTab.clear();
   delete ValSymTab;
-  delete static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab);
 }
 
 std::unique_ptr<RandomNumberGenerator> Module::createRNG(const Pass* P) const {
   SmallString<32> Salt(P->getPassName());
 
   // This RNG is guaranteed to produce the same random stream only
   // when the Module ID and thus the input filename is the same. This
   // might be problematic if the input filename extension changes

 }
 
 /// getNamedMetadata - Return the first NamedMDNode in the module with the
 /// specified name. This method returns null if a NamedMDNode with the
 /// specified name is not found.
 NamedMDNode *Module::getNamedMetadata(const Twine &Name) const {
   SmallString<256> NameData;
   StringRef NameRef = Name.toStringRef(NameData);
-  return static_cast<StringMap<NamedMDNode*> *>(NamedMDSymTab)->lookup(NameRef);
+  return NamedMDSymTab.lookup(NameRef);
 }
 
 /// getOrInsertNamedMetadata - Return the first named MDNode in the module
 /// with the specified name. This method returns a new NamedMDNode if a
 /// NamedMDNode with the specified name is not found.
 NamedMDNode *Module::getOrInsertNamedMetadata(StringRef Name) {
-  NamedMDNode *&NMD =
-    (*static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab))[Name];
+  NamedMDNode *&NMD = NamedMDSymTab[Name];
   if (!NMD) {
     NMD = new NamedMDNode(Name);
     NMD->setParent(this);
     NamedMDList.push_back(NMD);
   }
   return NMD;
 }
 
 /// eraseNamedMetadata - Remove the given NamedMDNode from this module and
 /// delete it.
 void Module::eraseNamedMetadata(NamedMDNode *NMD) {
-  static_cast<StringMap<NamedMDNode *> *>(NamedMDSymTab)->erase(NMD->getName());
+  NamedMDSymTab.erase(NMD->getName());
   NamedMDList.erase(NMD->getIterator());
 }
 
 bool Module::isValidModFlagBehavior(Metadata *MD, ModFlagBehavior &MFB) {
   if (ConstantInt *Behavior = mdconst::dyn_extract_or_null<ConstantInt>(MD)) {
     uint64_t Val = Behavior->getLimitedValue();
     if (Val >= ModFlagBehaviorFirstVal && Val <= ModFlagBehaviorLastVal) {
       MFB = static_cast<ModFlagBehavior>(Val);