[PATCH] D52999: [FileCheck] Annotate input dump (1/7)

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

It really seems like DiagList and AnnotationList ought to be vectors, not lists.  They are append-only, and AnnotationList gets converted to a vector anyway to sort it.  The code doesn't depend on the element-pointer stability guarantee of a list, except in one place noted below which can be fixed.
It's quite possible a vector would perform less well, in the face of many diags/annotations, but as the diags/annotations are the unusual case, performance is not really a big consideration.



================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:232
+  for (auto DiagItr = DiagList.begin(), DiagEnd = DiagList.end();
+       DiagItr != DiagEnd; ++DiagItr) {
+    AnnotationList.emplace_back();
----------------
range-for here?


================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:270
+        InputAnnotation &B = AnnotationList.back();
+        B.CheckLine = A.CheckLine;
+        B.Label = A.Label;
----------------
This appears to be the only place that functionally depends on AnnotationList being a `<list>`.  But if you built B as a stack instance first, then you can `push_back` when you're done, and then AnnotationList can be a `<vector>` instead.


https://reviews.llvm.org/D52999