[PATCH] D49169: Use debug-prefix-map for AT_NAME

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 11 05:35:02 PDT 2018


JDevlieghere added inline comments.


================
Comment at: lib/MC/MCContext.cpp:545
+  const auto &DebugPrefixMap = this->DebugPrefixMap;
+  const auto RemapDebugPath = [&DebugPrefixMap](std::string *Path) {
+    for (const auto &Entry : DebugPrefixMap)
----------------
starsid wrote:
> starsid wrote:
> > JDevlieghere wrote:
> > > JDevlieghere wrote:
> > > > I might have missed something, but can't this be a std::string&?
> > > I probably would've made this a static non-member function, but I don't think there's anything wrong with a lambda. 
> > Ack.
> > 
> > Let me know if you actually do want me to convert this to a static non-member function.
> It can be std::string&, but I thought when your intention is to modify the argument, you take it as a pointer. Sorry, this is my first change following LLVM code style so I am not familiar with what is expected.
No, it's perfectly fine. I was only curious if there was a particular reason that I didn't consider :-) 


================
Comment at: lib/MC/MCContext.cpp:545
+  const auto &DebugPrefixMap = this->DebugPrefixMap;
+  const auto RemapDebugPath = [&DebugPrefixMap](std::string *Path) {
+    for (const auto &Entry : DebugPrefixMap)
----------------
JDevlieghere wrote:
> starsid wrote:
> > starsid wrote:
> > > JDevlieghere wrote:
> > > > JDevlieghere wrote:
> > > > > I might have missed something, but can't this be a std::string&?
> > > > I probably would've made this a static non-member function, but I don't think there's anything wrong with a lambda. 
> > > Ack.
> > > 
> > > Let me know if you actually do want me to convert this to a static non-member function.
> > It can be std::string&, but I thought when your intention is to modify the argument, you take it as a pointer. Sorry, this is my first change following LLVM code style so I am not familiar with what is expected.
> No, it's perfectly fine. I was only curious if there was a particular reason that I didn't consider :-) 
No worries, I don't think they coding standard has an explicit entry on this. I think a non-const ref is the way to go here because passing nullptr doesn't mean anything here. Based on your reply I've made the change before committing, as it didn't seem worth the trouble to have you update the patch. 


Repository:
  rL LLVM

https://reviews.llvm.org/D49169





More information about the llvm-commits mailing list