[PATCH] D20414: IR: Allow multiple global metadata attachments with the same type.

[Adrian Prantl via llvm-commits]

aprantl added a comment.

Couple of small inline comments.


================
Comment at: include/llvm/IR/GlobalObject.h:108
@@ -97,6 +107,3 @@
 
-  /// Drop metadata not in the given list.
-  ///
-  /// Drop all metadata from \c this not included in \c KnownIDs.
-  void dropUnknownMetadata(ArrayRef<unsigned> KnownIDs);
+  void eraseMetadata(unsigned KindID);
 
----------------
Short comment?

================
Comment at: lib/IR/Metadata.cpp:1159
@@ +1158,3 @@
+  // need to preserve the original insertion order though.
+  if (Result.size() > 1)
+    std::stable_sort(Result.begin(), Result.end(),
----------------
Why is this check necessary?

================
Comment at: lib/IR/Metadata.cpp:1327
@@ -1288,1 +1326,3 @@
+    return;
+  return getContext().pImpl->GlobalObjectMetadata[this].get(KindID, MDs);
 }
----------------
Maybe:

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

================
Comment at: lib/IR/Metadata.cpp:1334
@@ -1294,1 +1333,3 @@
+    return;
+  getMetadata(getContext().getMDKindID(Kind), MDs);
 }
----------------
Same here

================
Comment at: lib/IR/Metadata.cpp:1338
@@ -1297,2 +1337,3 @@
+void GlobalObject::addMetadata(unsigned KindID, MDNode *MD) {
   if (MD) {
     if (!hasMetadata())
----------------
```
if (!MD)
  return;
```

================
Comment at: lib/IR/Metadata.cpp:1343
@@ -1302,2 +1342,3 @@
+    getContext().pImpl->GlobalObjectMetadata[this].add(KindID, *MD);
     return;
   }
----------------
redundant

================
Comment at: lib/IR/Metadata.cpp:1382
@@ +1381,3 @@
+  eraseMetadata(KindID);
+  if (N)
+    addMetadata(KindID, N);
----------------
This condition is redundant since addMetadata will check for a nullptr.

================
Comment at: lib/IR/Verifier.cpp:2002
@@ -2000,1 +2001,3 @@
       case LLVMContext::MD_dbg:
+        AssertDI(++NumDebugAttachments <= 1,
+                 "function must have a single !dbg attachment", &F, I.second);
----------------
```
++NumDebugAttachments <= 1
```
can never be <1, right?


http://reviews.llvm.org/D20414