[PATCH] D155723: [DWARFVerifier] Allow simplified template names in debug_name

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp:1363
+              StripTemplateParameters(Result.back()))
+        Result.push_back(*StrippedName);
+    }
----------------
avl wrote:
> fdeazeve wrote:
> > avl wrote:
> > > nit: other places use emplace_back.
> > Other places have a `const char*`, which is why they emplace, not push. In the grand scheme of things, either will generate the same code, but I think the `push` here is the more expressive call: we are communicating that we already have a StringRef. Do you agree?
> I am not sure that I follow... Other places might also use push_back even if they have const char* and vice versa this place can use emplace_back. It is just looks more uniformly if the single variant is used. 
> Other places might also use push_back even if they have const char*

Right, that's true because the `StringRef(const char*)` constructor is not explicit, but it is not generally true for all types. If we `push_back(const char*)`, the StringRef is being implicitly constructed on the **call** to push_back; when we `emplace_back(const char*)`, the StringRef is constructed inside the implementation of `emplace_back`.

But as I mentioned previously, this discussion is largely irrelevant for a simple type like StringRef, so I don't feel strongly either way. I'll use `emplace_back`!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155723/new/

https://reviews.llvm.org/D155723