[Lldb-commits] [PATCH] D106837: Create synthetic symbol names on demand to improve memory consumption and startup times.

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao added a comment.

Hi all, I found this patch causing PR52702  in that 
the parent of this commit 
 and LLDB 12 worked fine.
When disassembling a hello world C program on Linux, LLDB used to show
 `callq  0x401030  ; symbol stub for: puts` 
instead of
`callq  0x401030  ; symbol stub for: ___lldb_unnamed_symbol36`.
Examining the symbol table by running `lldb -b -o 'image dump symtab' a.out` 
used to show:

  [   18] 20   X Undefined   0x
0x 0x0012 puts@GLIBC_2.2.5
   
  [   33] 35   X Code0x00401000
0x001b 0x0212 _init
  [   34] 36  S  Trampoline  0x00401030
0x0010 0x puts
  [   35] 37  SX Code0x00401020
0x0010 0x ___lldb_unnamed_symbol1$$a.out

and now (ToT and LLDB 13) it's:

  [   18] 20   X Undefined   0x
0x 0x0012 puts@GLIBC_2.2.5
   
  [   33] 35   X Code0x00401000
0x001b 0x0212 _init
  [   34] 36  S  Trampoline  0x00401030
0x0010 0x ___lldb_unnamed_symbol36
  [   35] 37  SX Code0x00401020
0x0010 0x ___lldb_unnamed_symbol37

`image dump symtab libc.so.6` gives similar result.
Before ec1a4917  :

  [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
0x0010 0x realloc
  [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
0x0010 0x __tls_get_addr
  [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
0x0010 0x memalign
  [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
0x0010 0x _dl_exception_create
  [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
0x0010 0x __tunable_get_val
  [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
0x0010 0x _dl_find_dso_for_object
  [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
0x0010 0x calloc
  [ 2373]   2373  SX Code0x00025000 0x77df2000 
0x0010 0x ___lldb_unnamed_symbol1$$libc.so.6
  [ 2374]   2373  SX Code0x00025300 0x77df2300 
0x0040 0x ___lldb_unnamed_symbol2$$libc.so.6
  [ 2375]   2373  SX Code0x00025340 0x77df2340 
0x02f0 0x ___lldb_unnamed_symbol3$$libc.so.6
  [ 2376]   2373  SX Code0x00025630 0x77df2630 
0x000c 0x ___lldb_unnamed_symbol4$$libc.so.6

After:

  [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
0x0010 0x ___lldb_unnamed_symbol2367
  [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
0x0010 0x ___lldb_unnamed_symbol2368
  [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
0x0010 0x ___lldb_unnamed_symbol2369
  [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
0x0010 0x ___lldb_unnamed_symbol2370
  [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
0x0010 0x ___lldb_unnamed_symbol2371
  [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
0x0010 0x ___lldb_unnamed_symbol2372
  [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
0x0010 0x ___lldb_unnamed_symbol2373
  [ 2373]   2374  SX Code0x00025000 0x77df2000 
0x0010 0x ___lldb_unnamed_symbol2374
  [ 2374]   2375  SX Code0x00025300 0x77df2300 
0x0040 0x ___lldb_unnamed_symbol2375
  [ 2375]   2376  SX Code0x00025340 0x77df2340 
0x02f0 0x ___lldb_unnamed_symbol2376
  [ 2376]   2377  SX Code0x00025630 0x77df2630 
0x000c 0x ___lldb_unnamed_symbol2377

Is this intended for the performance boost? It seems to me that "`S  
Trampoline`" symbols should be handled differently.

FWIW, I also found that this doesn't affect macOS tho as `puts` is not even a 
synthetic symbol:

  Index   UserID DSX TypeFile Address/Value Load Address   Size 
  Flags  Name
  --- -- --- --- ---

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao created this revision.
rZhBoYao added reviewers: clayborg, wallace, JDevlieghere.
rZhBoYao added a project: LLDB.
rZhBoYao requested review of this revision.
Herald added a subscriber: lldb-commits.

Mangled::operator! claimes to be true if the object has an empty mangled and 
unmangled name, false otherwise, but it was actually true if the object has an 
empty mangled name. Luckily, this operator doesn't have a lot of users.

The broken logical not operator causes PR52702  as 
https://reviews.llvm.org/D106837 used Mangled::operator! in 
Symbol::SynthesizeNameIfNeeded. For example, consider the symbol "puts" in a 
hello world C program:

  // Inside Symbol::SynthesizeNameIfNeeded
  (lldb) p m_mangled
  (lldb_private::Mangled) $0 = (m_mangled = None, m_demangled = "puts")
  (lldb) p !m_mangled
  (bool) $1 = true  # should be false!!

This leads to Symbol::SynthesizeNameIfNeeded overwriting m_demangled part of 
Mangled (in this case "puts").

In conclusion, this patch turns
`callq  0x401030  ; symbol stub for: ___lldb_unnamed_symbol36`
back into
 `callq  0x401030  ; symbol stub for: puts` .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116217

Files:
  lldb/source/Core/Mangled.cpp


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -86,7 +86,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -86,7 +86,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:79-81
 Mangled::operator void *() const {
   return (m_mangled) ? const_cast(this) : nullptr;
 }

clayborg wrote:
> Looks like there is a similar bug here in this function as well. We should 
> convert the convert to pointer function here to "operator bool" and add a 
> unit test.
> 
> ```
> explicit operator bool() const { return m_mangled || m_demangled; }
> ```
This change breaks this test: `lldb-shell :: 
SymbolFile/DWARF/x86/find-qualified-variable.cpp` because the comment in the 
header file says: "This allows code to check a Mangled object to see if it 
contains a valid //**mangled name**//", which differs from the comment here, 
and apparently that's what people's intent was. Modifying 
`lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2143` solves this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao updated this revision to Diff 396261.

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,39 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+  }
+}
+
+TEST(MangledTest, LogicalNotOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,15 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Logical NOT operator. This allows code to check any Mangled objects to see
 // if they are invalid using code such as:
@@ -86,7 +84,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -86,12 +86,12 @@
   /// \return
   /// A pointer to this object if either the mangled or unmangled
   /// name is set, NULL otherwise.
-  operator void *() const;
+  explicit operator bool() const;
 
   /// Logical NOT operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they are invalid
+  /// using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao updated this revision to Diff 396263.

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,39 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+  }
+}
+
+TEST(MangledTest, LogicalNotOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,15 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Logical NOT operator. This allows code to check any Mangled objects to see
 // if they are invalid using code such as:
@@ -86,7 +84,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,14 +84,14 @@
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
+  /// Returns \b true if either the mangled or unmangled name is set, \b
+  /// false otherwise.
+  explicit operator bool() const;
 
   /// Logical NOT operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they are invalid
+  /// using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:87
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 

Should we remove `Mangled::operator!` altogether? I find it less confusing 
without. 


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:87
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 

labath wrote:
> rZhBoYao wrote:
> > Should we remove `Mangled::operator!` altogether? I find it less confusing 
> > without. 
> Does `!mangled` work without it? Would it be sufficient to implement the `!` 
> operator in terms of bool conversion function?
Yes, I tried removing it and the unit test I added (`TEST(MangledTest, 
LogicalNotOperator)`) passed. Keeping it this way is fine tho. 


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao updated this revision to Diff 396395.
rZhBoYao retitled this revision from "[lldb] Fix PR52702 by fixing 
Mangled::operator!" to "[lldb] Fix PR52702 by fixing bool conversion of 
Mangled".
rZhBoYao edited the summary of this revision.

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,25 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  /// Returns \b true if the object has an empty mangled and
-  /// unmangled name, \b false otherwise.
-  bool operator!() const;
+  /// Returns \b true if either the mangled or unmangled name is set,
+  /// \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

[PoYao Chang via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG633b002944b9: [lldb] Fix PR52702 by fixing bool conversion 
of Mangled (authored by rZhBoYao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,25 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  /// Returns \b true if the object has an empty mangled and
-  /// unmangled name, \b false otherwise.
-  bool operator!() const;
+  /// Returns \b true if either the mangled or unmangled name is set,
+  /// \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

[PoYao Chang via Phabricator via lldb-commits]

rZhBoYao added a comment.

Thank you all for spending time reviewing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

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