IR: Allow multiple global metadata attachments with the same type.

[pcc]

Summary

This will be necessary to allow the global merge pass to attach
multiple debug info metdata nodes to global variables once we reverse
the edge from DIGlobalVariable to GlobalVariable.

[aprantl]

Couple of small inline comments.

include/llvm/IR/GlobalObject.h

114–115

Short comment?

lib/IR/Metadata.cpp

1159

Why is this check necessary?

1322

Maybe:

if (hasMetadata())
  getContext().pImpl->GlobalObjectMetadata[this].get(KindID, MDs);

1328

Same here

1332–1333

if (!MD)
  return;

1336–1337

redundant

1375

This condition is redundant since addMetadata will check for a nullptr.

lib/IR/Verifier.cpp

2002

++NumDebugAttachments <= 1

can never be <1, right?

[pcc]

• Address review comments

lib/IR/Metadata.cpp

1159

It isn't, removed.

1332–1333

Rethinking this, maybe this function should always expect an non-null MDNode, since unlike setMetadata the intent is always to add an attachment. WDYT?

1375

See above

lib/IR/Verifier.cpp

2002

No, changed to ==

[pcc]

Ping.

[pcc]

Ping.

[aprantl]

lib/IR/LLVMContextImpl.h

1013

Comment mentioning what happens if the ID is invalid?

1014

No t sure: should we define a type that inherits from std:pair<unsigned, MDNode &> and defines an operator< or would that be over-engineering?

1014

Usually, we're calling this method insert().

1016

Comment explaining the sorting guarantees on the output?

lib/IR/Metadata.cpp

1332–1335

Could/should we then change the signature to use MDNode &MD?

1383–1389

The doxygen comment in the header should probably mention that this only works for singular attachments, though it's kind of obvious from the function signature.

1386

Please add an assertion message.

lib/IR/Verifier.cpp

2002

Thanks! I also think I'd be more comfortable with the hoisting the increment onto its own line, given that AssertDI is a macro and all.

unittests/IR/MetadataTest.cpp

2244–2245

Can you remind me why it is safe to drop this test without replacement?

[pcc]

• Address review comments

lib/IR/LLVMContextImpl.h

1014

To me it would be clearer to define the ordering just in getAll. Defining the ordering in an operator< function would imply that two attachments with the same ID are "equal", while at the moment it's clear from reading the code for getAll that that isn't the case.

lib/IR/Metadata.cpp

1383–1389

From GlobalObject.h:

The functions that return an MDNode require that the function have at most a
single attachment of the given kind

:)

But I've split up the doxygen comment for getMetadata into two "blocks" so it's clearer which comments pertain to which functions.

unittests/IR/MetadataTest.cpp

2244–2245

Its purpose is to test the GlobalObject::dropUnknownMetadata function, which has no non-test users [1], so there's no point in keeping it.
[1] http://llvm-cs.pcc.me.uk/lib/IR/Metadata.cpp/rdropUnknownMetadata

[aprantl]

Looking good now.

lib/IR/LLVMContextImpl.h

1014

Ok.

include/llvm/IR/GlobalObject.h

public:
   bool hasComdat() const { return getComdat() != nullptr; }
   const Comdat *getComdat() const { return ObjComdat; }
   Comdat *getComdat() { return ObjComdat; }
   void setComdat(Comdat *C) { ObjComdat = C; }
 
   /// Check if this has any metadata.
   bool hasMetadata() const { return hasMetadataHashEntry(); }
 
-  /// Get the current metadata attachment, if any.
+  /// Get the current metadata attachments for the given kind, if any.
   ///
-  /// Returns \c nullptr if such an attachment is missing.
+  /// These functions require that the function have at most a single attachment
+  /// of the given kind, and return \c nullptr if such an attachment is missing.
   /// @{
   MDNode *getMetadata(unsigned KindID) const;
   MDNode *getMetadata(StringRef Kind) const;
   /// @}
 
+  /// Appends all attachments with the given ID to \c MDs in insertion order.
+  /// If the global has no attachments with the given ID, or if ID is invalid,
+  /// leaves MDs unchanged.
+  /// @{
+  void getMetadata(unsigned KindID, SmallVectorImpl<MDNode *> &MDs) const;
+  void getMetadata(StringRef Kind, SmallVectorImpl<MDNode *> &MDs) const;
+  /// @}
+
   /// Set a particular kind of metadata attachment.
   ///
   /// Sets the given attachment to \c MD, erasing it if \c MD is \c nullptr or
   /// replacing it if it already exists.
   /// @{
   void setMetadata(unsigned KindID, MDNode *MD);
   void setMetadata(StringRef Kind, MDNode *MD);
   /// @}
 
-  /// Get all current metadata attachments.
+  /// Add a metadata attachment.
+  /// @{
+  void addMetadata(unsigned KindID, MDNode &MD);
+  void addMetadata(StringRef Kind, MDNode &MD);
+  /// @}
+
+  /// Appends all attachments for the global to \c MDs, sorting by attachment
+  /// ID. Attachments with the same ID appear in insertion order.
   void
   getAllMetadata(SmallVectorImpl<std::pair<unsigned, MDNode *>> &MDs) const;
 
-  /// Drop metadata not in the given list.
-  ///
-  /// Drop all metadata from \c this not included in \c KnownIDs.
-  void dropUnknownMetadata(ArrayRef<unsigned> KnownIDs);
+  /// Erase all metadata attachments with the given kind.
+  void eraseMetadata(unsigned KindID);
 
   void copyAttributesFrom(const GlobalValue *Src) override;
 
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const Value *V) {
     return V->getValueID() == Value::FunctionVal ||
            V->getValueID() == Value::GlobalVariableVal;
   }
 
-protected:
   void clearMetadata();
 
 private:
   bool hasMetadataHashEntry() const {
     return getGlobalValueSubClassData() & (1 << HasMetadataHashEntryBit);
   }
   void setHasMetadataHashEntry(bool HasEntry) {
     unsigned Mask = 1 << HasMetadataHashEntryBit;
     setGlobalValueSubClassData((~Mask & getGlobalValueSubClassData()) |
                                (HasEntry ? Mask : 0u));
   }
 };
 
 } // End llvm namespace
 
 #endif

lib/AsmParser/LLParser.cpp

 /// ParseGlobalObjectMetadataAttachment
 ///   ::= !dbg !57
 bool LLParser::ParseGlobalObjectMetadataAttachment(GlobalObject &GO) {
   unsigned MDK;
   MDNode *N;
   if (ParseMetadataAttachment(MDK, N))
     return true;
 
-  GO.setMetadata(MDK, N);
+  GO.addMetadata(MDK, *N);
   return false;
 }
 
 /// ParseOptionalFunctionMetadata
 ///   ::= (!dbg !57)*
 bool LLParser::ParseOptionalFunctionMetadata(Function &F) {
   while (Lex.getKind() == lltok::MetadataVar)
     if (ParseGlobalObjectMetadataAttachment(F))

lib/Bitcode/Reader/BitcodeReader.cpp

std::error_code BitcodeReader::parseGlobalObjectAttachment(
   assert(Record.size() % 2 == 0);
   for (unsigned I = 0, E = Record.size(); I != E; I += 2) {
     auto K = MDKindMap.find(Record[I]);
     if (K == MDKindMap.end())
       return error("Invalid ID");
     MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[I + 1]);
     if (!MD)
       return error("Invalid metadata attachment");
-    GO.setMetadata(K->second, MD);
+    GO.addMetadata(K->second, *MD);
   }
   return std::error_code();
 }
 
 /// Parse metadata attachments.
 std::error_code BitcodeReader::parseMetadataAttachment(Function &F) {
   if (Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
     return error("Invalid record");

lib/Bitcode/Writer/ValueEnumerator.cpp

ValueEnumerator::ValueEnumerator(const Module &M,
 
   // Insert constants and metadata that are named at module level into the slot
   // pool so that the module symbol table can refer to them...
   EnumerateValueSymbolTable(M.getValueSymbolTable());
   EnumerateNamedMetadata(M);
 
   SmallVector<std::pair<unsigned, MDNode *>, 8> MDs;
   for (const GlobalVariable &GV : M.globals()) {
+    MDs.clear();
     GV.getAllMetadata(MDs);
     for (const auto &I : MDs)
       EnumerateMetadata(&GV, I.second);
   }
 
   // Enumerate types used by function bodies and argument lists.
   for (const Function &F : M) {
     for (const Argument &A : F.args())
       EnumerateType(A.getType());
 
     // Enumerate metadata attached to this function.
+    MDs.clear();
     F.getAllMetadata(MDs);
     for (const auto &I : MDs)
       EnumerateMetadata(&F, I.second);
 
     for (const BasicBlock &BB : F)
       for (const Instruction &I : BB) {
         for (const Use &Op : I.operands()) {
           auto *MD = dyn_cast<MetadataAsValue>(&Op);

lib/IR/LLVMContextImpl.h

public:
   /// Erases all attachments matching the \c shouldRemove predicate.
   template <class PredTy> void remove_if(PredTy shouldRemove) {
     Attachments.erase(
         std::remove_if(Attachments.begin(), Attachments.end(), shouldRemove),
         Attachments.end());
   }
 };
 
+/// Multimap-like storage for metadata attachments for globals. This differs
+/// from MDAttachmentMap in that it allows multiple attachments per metadata
+/// kind.
+class MDGlobalAttachmentMap {
+  struct Attachment {
+    unsigned MDKind;
+    TrackingMDNodeRef Node;
+  };
+  SmallVector<Attachment, 1> Attachments;
+
+public:
+  bool empty() const { return Attachments.empty(); }
+
+  /// Appends all attachments with the given ID to \c Result in insertion order.
+  /// If the global has no attachments with the given ID, or if ID is invalid,
+  /// leaves Result unchanged.
+  void get(unsigned ID, SmallVectorImpl<MDNode *> &Result);
+
+  void insert(unsigned ID, MDNode &MD);
+  void erase(unsigned ID);
+
+  /// Appends all attachments for the global to \c Result, sorting by attachment
+  /// ID. Attachments with the same ID appear in insertion order. This function
+  /// does \em not clear \c Result.
+  void getAll(SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const;
+};
+
 class LLVMContextImpl {
 public:
   /// OwnedModules - The set of modules instantiated in this context, and which
   /// will be automatically deleted if this context is deleted.
   SmallPtrSet<Module*, 4> OwnedModules;
   
   LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler;
   void *InlineAsmDiagContext;
#include "llvm/IR/Metadata.def"
   
   /// CustomMDKindNames - Map to hold the metadata string to ID mapping.
   StringMap<unsigned> CustomMDKindNames;
 
   /// Collection of per-instruction metadata used in this context.
   DenseMap<const Instruction *, MDAttachmentMap> InstructionMetadata;
 
   /// Collection of per-GlobalObject metadata used in this context.
-  DenseMap<const GlobalObject *, MDAttachmentMap> GlobalObjectMetadata;
+  DenseMap<const GlobalObject *, MDGlobalAttachmentMap> GlobalObjectMetadata;
 
   /// DiscriminatorTable - This table maps file:line locations to an
   /// integer representing the next DWARF path discriminator to assign to
   /// instructions in different blocks at the same location.
   DenseMap<std::pair<const char *, unsigned>, unsigned> DiscriminatorTable;
 
   int getOrAddScopeRecordIdxEntry(MDNode *N, int ExistingIdx);
   int getOrAddScopeInlinedAtIdxEntry(MDNode *Scope, MDNode *IA,int ExistingIdx);

lib/IR/Metadata.cpp

void MDAttachmentMap::getAll(
     SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const {
   Result.append(Attachments.begin(), Attachments.end());
 
   // Sort the resulting array so it is stable.
   if (Result.size() > 1)
     array_pod_sort(Result.begin(), Result.end());
 }
 
+void MDGlobalAttachmentMap::insert(unsigned ID, MDNode &MD) {
+  Attachments.push_back({ID, TrackingMDNodeRef(&MD)});
+}
+
+void MDGlobalAttachmentMap::get(unsigned ID,
+                                SmallVectorImpl<MDNode *> &Result) {
+  for (auto A : Attachments)
+    if (A.MDKind == ID)
+      Result.push_back(A.Node);
+}
+
+void MDGlobalAttachmentMap::erase(unsigned ID) {
+  auto Follower = Attachments.begin();
+  for (auto Leader = Attachments.begin(), E = Attachments.end(); Leader != E;
+       ++Leader) {
+    if (Leader->MDKind != ID) {
+      if (Follower != Leader)
+        *Follower = std::move(*Leader);
+      ++Follower;
+    }
+  }
+  Attachments.resize(Follower - Attachments.begin());
+}
+
+void MDGlobalAttachmentMap::getAll(
+    SmallVectorImpl<std::pair<unsigned, MDNode *>> &Result) const {
+  for (auto &A : Attachments)
+    Result.emplace_back(A.MDKind, A.Node);
+
+  // Sort the resulting array so it is stable with respect to metadata IDs. We
+  // need to preserve the original insertion order though.
+  std::stable_sort(
+      Result.begin(), Result.end(),
+      [](const std::pair<unsigned, MDNode *> &A,
+         const std::pair<unsigned, MDNode *> &B) { return A.first < B.first; });
+}
+
 void Instruction::setMetadata(StringRef Kind, MDNode *Node) {
   if (!Node && !hasMetadata())
     return;
   setMetadata(getContext().getMDKindID(Kind), Node);
 }
 
 MDNode *Instruction::getMetadataImpl(StringRef Kind) const {
   return getMetadataImpl(getContext().getMDKindID(Kind));

 }
 
 void Instruction::clearMetadataHashEntries() {
   assert(hasMetadataHashEntry() && "Caller should check");
   getContext().pImpl->InstructionMetadata.erase(this);
   setHasMetadataHashEntry(false);
 }
 
-MDNode *GlobalObject::getMetadata(unsigned KindID) const {
-  if (!hasMetadata())
-    return nullptr;
-  return getContext().pImpl->GlobalObjectMetadata[this].lookup(KindID);
+void GlobalObject::getMetadata(unsigned KindID,
+                               SmallVectorImpl<MDNode *> &MDs) const {
+  if (hasMetadata())
+    getContext().pImpl->GlobalObjectMetadata[this].get(KindID, MDs);
 }
 
-MDNode *GlobalObject::getMetadata(StringRef Kind) const {
-  if (!hasMetadata())
-    return nullptr;
-  return getMetadata(getContext().getMDKindID(Kind));
+void GlobalObject::getMetadata(StringRef Kind,
+                               SmallVectorImpl<MDNode *> &MDs) const {
+  if (hasMetadata())
+    getMetadata(getContext().getMDKindID(Kind), MDs);
 }
 
-void GlobalObject::setMetadata(unsigned KindID, MDNode *MD) {
-  if (MD) {
-    if (!hasMetadata())
-      setHasMetadataHashEntry(true);
+void GlobalObject::addMetadata(unsigned KindID, MDNode &MD) {
 
-    getContext().pImpl->GlobalObjectMetadata[this].set(KindID, *MD);
-    return;
-  }
+  getContext().pImpl->GlobalObjectMetadata[this].insert(KindID, MD);
+}
+
+void GlobalObject::addMetadata(StringRef Kind, MDNode &MD) {
+  addMetadata(getContext().getMDKindID(Kind), MD);
 
+void GlobalObject::eraseMetadata(unsigned KindID) {
   // Nothing to unset.
   if (!hasMetadata())
     return;
 
   auto &Store = getContext().pImpl->GlobalObjectMetadata[this];
   Store.erase(KindID);
   if (Store.empty())
     clearMetadata();
 }
 
-void GlobalObject::setMetadata(StringRef Kind, MDNode *MD) {
-  if (!MD && !hasMetadata())
-    return;
-  setMetadata(getContext().getMDKindID(Kind), MD);
-}
-
 void GlobalObject::getAllMetadata(
     SmallVectorImpl<std::pair<unsigned, MDNode *>> &MDs) const {
   MDs.clear();
 
   if (!hasMetadata())
     return;
 
   getContext().pImpl->GlobalObjectMetadata[this].getAll(MDs);
 }
 
-void GlobalObject::dropUnknownMetadata(ArrayRef<unsigned> KnownIDs) {
+void GlobalObject::clearMetadata() {
   if (!hasMetadata())
     return;
-  if (KnownIDs.empty()) {
-    clearMetadata();
-    return;
-  }
+  getContext().pImpl->GlobalObjectMetadata.erase(this);
+  setHasMetadataHashEntry(false);
 
-  SmallSet<unsigned, 5> KnownSet;
-  KnownSet.insert(KnownIDs.begin(), KnownIDs.end());
 
-  auto &Store = getContext().pImpl->GlobalObjectMetadata[this];
-  assert(!Store.empty());
+void GlobalObject::setMetadata(unsigned KindID, MDNode *N) {
+  eraseMetadata(KindID);
+  if (N)
+    addMetadata(KindID, *N);
+}
 
-  Store.remove_if([&KnownSet](const std::pair<unsigned, TrackingMDNodeRef> &I) {
-    return !KnownSet.count(I.first);
-  });
+void GlobalObject::setMetadata(StringRef Kind, MDNode *N) {
+  setMetadata(getContext().getMDKindID(Kind), N);
+}
 
-  if (Store.empty())
-    clearMetadata();
+MDNode *GlobalObject::getMetadata(unsigned KindID) const {
+  SmallVector<MDNode *, 1> MDs;
+  getMetadata(KindID, MDs);
+  assert(MDs.size() <= 1 && "Expected at most one metadata attachment");
+  if (MDs.empty())
+    return nullptr;
+  return MDs[0];
 }
 
-void GlobalObject::clearMetadata() {
-  if (!hasMetadata())
-    return;
-  getContext().pImpl->GlobalObjectMetadata.erase(this);
-  setHasMetadataHashEntry(false);
+MDNode *GlobalObject::getMetadata(StringRef Kind) const {
+  return getMetadata(getContext().getMDKindID(Kind));
 }
 
 void Function::setSubprogram(DISubprogram *SP) {
   setMetadata(LLVMContext::MD_dbg, SP);
 }
 
 DISubprogram *Function::getSubprogram() const {
   return cast_or_null<DISubprogram>(getMetadata(LLVMContext::MD_dbg));
 }

lib/IR/Verifier.cpp

Assert(pred_empty(Entry),
            "Entry block to function must not have predecessors!", Entry);
 
     // The address of the entry block cannot be taken, unless it is dead.
     if (Entry->hasAddressTaken()) {
       Assert(!BlockAddress::lookup(Entry)->isConstantUsed(),
              "blockaddress may not be used with the entry block!", Entry);
     }
 
+    unsigned NumDebugAttachments = 0;
     // Visit metadata attachments.
     for (const auto &I : MDs) {
       // Verify that the attachment is legal.
       switch (I.first) {
       default:
         break;
       case LLVMContext::MD_dbg:
+        ++NumDebugAttachments;
+        AssertDI(NumDebugAttachments == 1,
+                 "function must have a single !dbg attachment", &F, I.second);
         AssertDI(isa<DISubprogram>(I.second),
                  "function !dbg attachment must be a subprogram", &F, I.second);
         break;
       }
 
       // Verify the metadata itself.
       visitMDNode(*I.second);
     }

lib/Transforms/Utils/CloneFunction.cpp

NewFunc->setAttributes(
           .addAttributes(NewFunc->getContext(), AttributeSet::ReturnIndex,
                          OldAttrs.getRetAttributes())
           .addAttributes(NewFunc->getContext(), AttributeSet::FunctionIndex,
                          OldAttrs.getFnAttributes()));
 
   SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
   OldFunc->getAllMetadata(MDs);
   for (auto MD : MDs)
-    NewFunc->setMetadata(
+    NewFunc->addMetadata(
         MD.first,
-        MapMetadata(MD.second, VMap,
+        *MapMetadata(MD.second, VMap,
                      ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
                      TypeMapper, Materializer));
 
   // Loop over all of the basic blocks in the function, cloning them as
   // appropriate.  Note that we save BE this way in order to handle cloning of
   // recursive functions into themselves.
   //
   for (Function::const_iterator BI = OldFunc->begin(), BE = OldFunc->end();
        BI != BE; ++BI) {
     const BasicBlock &BB = *BI;

lib/Transforms/Utils/ValueMapper.cpp

void Mapper::remapFunction(Function &F) {
   // Remap the operands.
   for (Use &Op : F.operands())
     if (Op)
       Op = mapValue(Op);
 
   // Remap the metadata attachments.
   SmallVector<std::pair<unsigned, MDNode *>, 8> MDs;
   F.getAllMetadata(MDs);
+  F.clearMetadata();
   for (const auto &I : MDs)
-    F.setMetadata(I.first, cast_or_null<MDNode>(mapMetadata(I.second)));
+    F.addMetadata(I.first, *cast<MDNode>(mapMetadata(I.second)));
 
   // Remap the argument types.
   if (TypeMapper)
     for (Argument &A : F.args())
       A.mutateType(TypeMapper->remapType(A.getType()));
 
   // Remap the instructions.
   for (BasicBlock &BB : F)

test/Assembler/metadata.ll

 ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
-; CHECK: @global = global i32 0, !foo [[M2:![0-9]+]], !baz [[M3:![0-9]+]]
-@global = global i32 0, !foo !2, !baz !3
+; CHECK: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]]
+@global = global i32 0, !foo !2, !foo !3, !baz !3
 
 ; CHECK-LABEL: @test
 ; CHECK: ret void, !foo [[M0:![0-9]+]], !bar [[M1:![0-9]+]]
 define void @test() !dbg !1 {
   add i32 2, 1, !bar !0
   add i32 1, 2, !foo !1
   call void @llvm.dbg.func.start(metadata !"foo")
   extractvalue {{i32, i32}, i32} undef, 0, 1, !foo !0

test/Verifier/metadata-function-dbg.ll

 ; RUN: not llvm-as %s -disable-output 2>&1 | FileCheck %s
 
-define void @foo() !dbg !4 !dbg !4 {
+define void @foo() !dbg !4 {
+  unreachable
+}
+
+; CHECK:      function must have a single !dbg attachment
+define void @foo2() !dbg !4 !dbg !4 {
   unreachable
 }
 
 ; CHECK-NOT:  !dbg
 ; CHECK:      function !dbg attachment must be a subprogram
 ; CHECK-NEXT: void ()* @bar
 ; CHECK-NEXT: !{{[0-9]+}} = !{}
 define void @bar() !dbg !6 {

unittests/IR/MetadataTest.cpp

TEST_F(FunctionAttachmentTest, getAll) {
   EXPECT_EQ(LLVMContext::MD_prof, MDs[1].first);
   EXPECT_EQ(Context.getMDKindID("other1"), MDs[2].first);
   EXPECT_EQ(Context.getMDKindID("other2"), MDs[3].first);
   EXPECT_EQ(SP, MDs[0].second);
   EXPECT_EQ(P, MDs[1].second);
   EXPECT_EQ(T1, MDs[2].second);
   EXPECT_EQ(T2, MDs[3].second);
 }
 
-TEST_F(FunctionAttachmentTest, dropUnknownMetadata) {
-  Function *F = getFunction("foo");
-
-  MDTuple *T1 = getTuple();
-  MDTuple *T2 = getTuple();
-  MDTuple *P = getTuple();
-  DISubprogram *SP = getSubprogram();
-
-  F->setMetadata("other1", T1);
-  F->setMetadata(LLVMContext::MD_dbg, SP);
-  F->setMetadata("other2", T2);
-  F->setMetadata(LLVMContext::MD_prof, P);
-
-  unsigned Known[] = {Context.getMDKindID("other2"), LLVMContext::MD_prof};
-  F->dropUnknownMetadata(Known);
-
-  EXPECT_EQ(T2, F->getMetadata("other2"));
-  EXPECT_EQ(P, F->getMetadata(LLVMContext::MD_prof));
-  EXPECT_EQ(nullptr, F->getMetadata("other1"));
-  EXPECT_EQ(nullptr, F->getMetadata(LLVMContext::MD_dbg));
-
-  F->setMetadata("other2", nullptr);
-  F->setMetadata(LLVMContext::MD_prof, nullptr);
-  EXPECT_FALSE(F->hasMetadata());
-}
-
 TEST_F(FunctionAttachmentTest, Verifier) {
   Function *F = getFunction("foo");
   F->setMetadata("attach", getTuple());
 
   // Confirm this has no body.
   ASSERT_TRUE(F->empty());
 
   // Functions without a body cannot have metadata attachments (they also can't
   // be verified directly, so check that the module fails to verify).