samitolvanen added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2255-2260
+  for (const char &C : Name) {
+    if (llvm::isAlnum(C) || C == '_' || C == '.')
+      continue;
+    return false;
+  }
+  return true;
----------------
nickdesaulniers wrote:
> samitolvanen wrote:
> > nickdesaulniers wrote:
> > > Is this more concise using `llvm::all_of` from llvm/ADT/STLExtras.h?
> > Yes, but also slower. This is the same function that we use in LLVM to 
> > check for acceptable promotion alias names 
> > (https://reviews.llvm.org/D104058). Should this be moved into a helper 
> > function somewhere? If so, where?
> Seeing as it was originally based off of MCAsmInfo::isAcceptableChar, perhaps 
> MCAsmInfo or one of its subclasses?
> perhaps MCAsmInfo or one of its subclasses?

See rnk's comment from the ThinLTO patch 
(https://reviews.llvm.org/D104058#2891756):

> We can't call MC from here due to library layering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to