Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC] refactor demangle of llvm-nm #67481

Merged
merged 2 commits into from
Sep 27, 2023
Merged

[NFC] refactor demangle of llvm-nm #67481

merged 2 commits into from
Sep 27, 2023

Conversation

diggerlin
Copy link
Contributor

since we landed the patch https://reviews.llvm.org/D139864 [llvm-cxxfilt] Do not consider the prefix dot as part of the demangled symbol name. we can simply the code on the llvm-nm demanlge by using the llvm::demangle directly.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 26, 2023

@llvm/pr-subscribers-llvm-binary-utilities

Changes

since we landed the patch https://reviews.llvm.org/D139864 [llvm-cxxfilt] Do not consider the prefix dot as part of the demangled symbol name. we can simply the code on the llvm-nm demanlge by using the llvm::demangle directly.


Full diff: https://github.com/llvm/llvm-project/pull/67481.diff

1 Files Affected:

  • (modified) llvm/tools/llvm-nm/llvm-nm.cpp (+2-33)
diff --git a/llvm/tools/llvm-nm/llvm-nm.cpp b/llvm/tools/llvm-nm/llvm-nm.cpp
index 051fa3e5bfa5a98..6812a90c4cfaeb6 100644
--- a/llvm/tools/llvm-nm/llvm-nm.cpp
+++ b/llvm/tools/llvm-nm/llvm-nm.cpp
@@ -629,30 +629,6 @@ static void darwinPrintStab(MachOObjectFile *MachO, const NMSymbol &S) {
     outs() << format("   %02x", NType);
 }
 
-static std::optional<std::string> demangle(StringRef Name) {
-  std::string Demangled;
-  if (nonMicrosoftDemangle(Name, Demangled))
-    return Demangled;
-  return std::nullopt;
-}
-
-static std::optional<std::string> demangleXCOFF(StringRef Name) {
-  if (Name.empty() || Name[0] != '.')
-    return demangle(Name);
-
-  Name = Name.drop_front();
-  std::optional<std::string> DemangledName = demangle(Name);
-  if (DemangledName)
-    return "." + *DemangledName;
-  return std::nullopt;
-}
-
-static std::optional<std::string> demangleMachO(StringRef Name) {
-  if (!Name.empty() && Name[0] == '_')
-    Name = Name.drop_front();
-  return demangle(Name);
-}
-
 static bool symbolIsDefined(const NMSymbol &Sym) {
   return Sym.TypeChar != 'U' && Sym.TypeChar != 'w' && Sym.TypeChar != 'v';
 }
@@ -822,15 +798,8 @@ static void printSymbolList(SymbolicFile &Obj,
 
     std::string Name = S.Name;
     MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(&Obj);
-    if (Demangle) {
-      function_ref<std::optional<std::string>(StringRef)> Fn = ::demangle;
-      if (Obj.isXCOFF())
-        Fn = demangleXCOFF;
-      if (Obj.isMachO())
-        Fn = demangleMachO;
-      if (std::optional<std::string> Opt = Fn(S.Name))
-        Name = *Opt;
-    }
+    if (Demangle)
+      Name = llvm::demangle(Name);
 
     if (PrintFileName)
       writeFileName(outs(), ArchiveName, ArchitectureName);

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with one nit.

Name = *Opt;
}
if (Demangle)
Name = llvm::demangle(Name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need the llvm:: prefix?

@diggerlin diggerlin merged commit 72b8d25 into llvm:main Sep 27, 2023
3 checks passed
legrosbuffle pushed a commit to legrosbuffle/llvm-project that referenced this pull request Sep 29, 2023
since we landed the patch `https://reviews.llvm.org/D139864
[llvm-cxxfilt] Do not consider the prefix dot as part of the demangled
symbol name`. we can simply the code on the llvm-nm demanlge by using
the llvm::demangle directly.

---------

Co-authored-by: zhijian <zhijian@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants