[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-12 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG406ad187486b: [lldb/DataFormatters] Display null C++ 
pointers as nullptr (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D77153?vs=304614=304983#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py
  lldb/test/API/lang/objc/objc-builtin-types/Makefile
  lldb/test/API/lang/objc/objc-builtin-types/TestObjCBuiltinTypes.py
  lldb/test/API/lang/objc/objc-builtin-types/main.cpp
  lldb/test/API/lang/objcxx/objc-builtin-types/Makefile
  lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
  lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp

Index: lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
===
--- lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
+++ lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
@@ -1,7 +1,5 @@
 """Test that the expression parser doesn't get confused by 'id' and 'Class'"""
 
-
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -21,7 +19,6 @@
 self.main_source, '// Set breakpoint here.')
 
 @add_test_categories(['pyapi'])
-# [regression] Can't print ivar value: error: reference to 'id' is ambiguous
 def test_with_python_api(self):
 """Test expression parser respect for ObjC built-in types."""
 self.build()
@@ -57,4 +54,7 @@
 
 self.expect("expr (foo)", patterns=["\(ns::id\) \$.* = 0"])
 
-self.expect("expr id my_id = 0; my_id", patterns=["\(id\) \$.* = nil"])
+self.expect("expr --language Objective-C++ -- id my_id = 0; my_id",
+patterns=["\(id\) \$.* = nil"])
+self.expect("expr --language C++ -- id my_id = 0; my_id",
+patterns=["\(id\) \$.* = nullptr"])
Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = NULL"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -80,6 +80,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -117,6 +118,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-11 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.

It seems weird that even if you had a summary formatter for some pointer type 
that was trying to print "DANGER WILL ROBINSON" when the pointer value was 0x0, 
we will override that and print "nullptr" in a C++ context or "nil" in an ObjC 
context.  Seems like we even if the value is Nil we should first consult the 
ValueObject's summary value and only do the nullptr/nil computation if it was 
empty.

But that bad behavior was already present for ObjC objects before this patch, 
you're just extending it to C++ or anything else that implements 
IsNilReference.  So fixing that doesn't seem required for this patch.




Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:368
+  summary.assign(lang_plugin->GetNilReferenceSummaryString().str());
+  } else if (IsUninitialized()) {
+summary.assign("");

If you wanted to you could check here if the languages hadn't assigned anything 
to Summary, and default to "NULL"?  That would be consistent with our treating 
C as the fallback language rather than as a separate Language plugin.  Not 
required...


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-11 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 304614.

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,11 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = 0x0"])
+self.expect("expression --format d -- pz", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["(type_z *) $", " = 0"])
+self.expect("expression --format b -- pz", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["(type_z *) $", " = 0b0"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector) $0 = {', 'f = 0x', '}'])
+substrs=['(std::__1::vector) $0 = {', 'f = nullptr', '}'])
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -27,6 +27,8 @@
 return lldb::eLanguageTypeObjC_plus_plus;
   }
 
+  llvm::StringRef GetNilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-11 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

A few comments, but otherwise this seems good.




Comment at: lldb/include/lldb/Target/Language.h:214
 
+  virtual llvm::StringRef NilReferenceSummaryString() { return {}; }
+

`/// Returns the summary string that should be used for a ValueObject for which 
IsNilReference() is true` or something like that.

Also I think this probably should have a `Get` prefix like the other methods in 
this class.



Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:367
+  else
+summary.assign("0x0");
+} else if (IsUninitialized())

JDevlieghere wrote:
> If we had a C runtime we could print "NULL", but I don't think it's worth 
> adding that just for this. Another alternative would be to just have a switch 
> based on the `LanguageType`, but that seems like a pretty bad idea from a 
> layering perspective. 
Not sure what to think about hardcoding `0x0` here. You can specify all kind of 
formatters that would format the value here differently and this seems kinda 
inconsistent.
```
(lldb) expr --format d -- nullptr
(nullptr_t) $2 = 0
(lldb) expr --format b -- nullptr
(nullptr_t) $3 = 
0b
```

Could we just fall to the default formatter if we don't have a special summary 
from the plugin?



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1138
+bool CPlusPlusLanguage::IsNilReference(ValueObject ) {
+  if (valobj.GetCompilerType().GetMinimumLanguage() !=
+  eLanguageTypeC_plus_plus ||

Nit: 
`!Language::LanguageIsCPlusPlus(valobj.GetCompilerType().GetMinimumLanguage())` 
? (as we might return a specific C++ standard in the future from 
GetMinimumLanguage).


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-11 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 304415.
JDevlieghere added a comment.

Fix the C test


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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = 0x0"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector) $0 = {', 'f = 0x', '}'])
+substrs=['(std::__1::vector) $0 = {', 'f = nullptr', '}'])
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -27,6 +27,8 @@
 return lldb::eLanguageTypeObjC_plus_plus;
   }
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject ) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-10 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 304352.

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
  lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = nullptr"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-cpp/main.cpp
@@ -63,6 +63,7 @@
 {
 
 int iAmInt = 1;
+	int* iAmIntPtr = 0;
 const float& IAmFloat = float(2.45);
 
 RealNumber RNILookChar = 3.14;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -289,3 +289,6 @@
 matching=False,
 substrs=['(int) iAmInt = 0x0001'])
 self.expect("frame variable iAmInt", substrs=['(int) iAmInt = 1'])
+
+self.expect("frame variable iAmIntPtr",
+substrs=['(int*) iAmIntPtr = nullptr'])
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector)

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-10 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D77153#2386740 , @shafik wrote:

> So this work for primitive types like `int*` etc?

No, Jim probably know the details, but this only applies to composite types.


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-10 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

So this work for primitive types like `int*` etc?


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

https://reviews.llvm.org/D77153

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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-10 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 304282.
JDevlieghere added a comment.

- Add fallback for finding the language
- Fix tests


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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py

Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = nullptr"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
@@ -39,4 +39,4 @@
 # Both `std::vector` and the type of the member have forward
 # declarations before their definitions.
 self.expect("expr --raw -- v",
-substrs=['(std::__1::vector) $0 = {', 'f = 0x', '}'])
+substrs=['(std::__1::vector) $0 = {', 'f = nullptr', '}'])
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -27,6 +27,8 @@
 return lldb::eLanguageTypeObjC_plus_plus;
   }
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject ) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

ValueObjects know their execution context, and so they can get to their frame, 
if they have one.  The language of the frame seems like the best thing to clue 
off here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits 
friss added a comment.

This looks like what I'd like to implement, but unfortunately it breaks other 
tests. Some C tests start printing null pointers as `nullptr` too. I suppose 
this is because the expression evaluator is always in C++ mode. Is there a way 
to get the origin type/language of a variable through a ValueObject?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits 
friss updated this revision to Diff 255579.
friss added a comment.

Remove the now useless code in TypeSummary.cpp, and change the C++
implementation of IsNilReference() to return true soemtimes. This
breaks other tests though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -71,6 +71,7 @@
 std::u32string u32_string(U"");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject ) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -88,6 +88,10 @@
   HardcodedFormatters::HardcodedSyntheticFinder
   GetHardcodedSynthetics() override;
 
+  bool IsNilReference(ValueObject ) override;
+
+  llvm::StringRef NilReferenceSummaryString() override { return "nullptr"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1133,6 +1133,15 @@
   return g_formatters;
 }
 
+bool CPlusPlusLanguage::IsNilReference(ValueObject ) {
+  if (valobj.GetCompilerType().GetMinimumLanguage() != eLanguageTypeC_plus_plus ||
+  !valobj.IsPointerType())
+return false;
+  bool canReadValue = true;
+  bool isZero = valobj.GetValueAsUnsigned(0, ) == 0;
+  return canReadValue && isZero;
+}
+
 bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".cpp", ".cxx", ".c++", ".cc",  ".c",
  ".h",   ".hh",  ".hpp", ".hxx", ".h++"};
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -361,9 +361,11 @@
 error.assign(err_cstr);
 
   if (ShouldPrintValueObject()) {
-if (IsNil())
-  summary.assign("nil");
-else if (IsUninitialized())
+if (IsNil()) {
+  Language *lang_plugin =
+  Language::FindPlugin(m_valobj->GetObjectRuntimeLanguage());
+

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits 
friss updated this revision to Diff 255571.
friss added a comment.

Implement null C++ printing at the ValueObjectPrinter level


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/TypeSummary.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -71,6 +71,7 @@
 std::u32string u32_string(U"");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject ) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -88,6 +88,10 @@
   HardcodedFormatters::HardcodedSyntheticFinder
   GetHardcodedSynthetics() override;
 
+  bool IsNilReference(ValueObject ) override;
+
+  llvm::StringRef NilReferenceSummaryString() override { return "nullptr"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return _highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1133,6 +1133,17 @@
   return g_formatters;
 }
 
+bool CPlusPlusLanguage::IsNilReference(ValueObject ) {
+  const uint32_t mask = eTypeIsCPlusPlus | eTypeIsPointer;
+  bool isCPPpointer =
+  (((valobj.GetCompilerType().GetTypeInfo(nullptr)) & mask) == mask);
+  if (!isCPPpointer)
+return false;
+  bool canReadValue = true;
+  bool isZero = valobj.GetValueAsUnsigned(0, ) == 0;
+  return canReadValue && isZero;
+}
+
 bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".cpp", ".cxx", ".c++", ".cc",  ".c",
  ".h",   ".hh",  ".hpp", ".hxx", ".h++"};
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -361,9 +361,11 @@
 error.assign(err_cstr);
 
   if (ShouldPrintValueObject()) {
-if (IsNil())
-  summary.assign("nil");
-else if (IsUninitialized())
+if (IsNil()) {
+  Language *lang_plugin =
+

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-02 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D77153#1955829 , @friss wrote:

> I was checking whether there is a way to catch null pointer before a type 
> summary is even chosen. And found out that such logic exists, but only for 
> Objective-C. Languages have a `IsNilReference()` virtual method that can tell 
> whether a `ValueObject` is a nil reference. It's only implemented for 
> ObjectiveC and value objects that test positive for this method are displayed 
> as "nil" is the generic code of `ValueObjectPrinter.cpp`. I can see a few 
> options:
>
> - I can rework the patch to affect only the libc++ string formatter which was 
> the issue I was trying to solve in the first place
> - I can implement `IsNilReference()` for C++ and change `ValueObjectPrinter` 
> to print C++ "nil references" as "nullptr".
> - I do the above and add another customization point in the Language plugins 
> which tells `ValueObjectPrinter` out to display "nil" references, so that 
> there's no Language specific code in `ValueObjectPrinter`.


All of those options sound fine to me. Making that generic is obviously 
preferable, but I don't know how much work that would be.

The thing that I found surprising when looking at this is that `std::string` 
has the exact same summary as a `std::string *` which points to it. I would 
have expected at least some subtle hint (perhaps by prepending the summary with 
a hex pointer value?) to indicate that we're looking at a pointer. This would 
be doable if the generic code had some way to understand pointers (and their 
nullness).

> A few additional questions come to mind though:
> 
> - Was the intent if `IsNilReference()` to be Objective-C specific in the 
> first place?
> - Should we do the same thing for C and NULL?
> - C, C++ and Objective-C(++) being highly intermingled, would something like 
> this even do the right thing in all cases?

Good questions, and I don't really have an answer for that. I don't think that 
`IsNilReference` should be c++-specific. Printing C pointers as NULL would be 
nice, but i don't know if we have any summaries for C types anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-01 Thread Frederic Riss via Phabricator via lldb-commits 
friss added a comment.

I was checking whether there is a way to catch null pointer before a type 
summary is even chosen. And found out that such logic exists, but only for 
Objective-C. Languages have a `IsNilReference()` virtual method that can tell 
whether a `ValueObject` is a nil reference. It's only implemented for 
ObjectiveC and value objects that test positive for this method are displayed 
as "nil" is the generic code of `ValueObjectPrinter.cpp`. I can see a few 
options:

- I can rework the patch to affect only the libc++ string formatter which was 
the issue I was trying to solve in the first place
- I can implement `IsNilReference()` for C++ and change `ValueObjectPrinter` to 
print C++ "nil references" as "nullptr".
- I do the above and add another customization point in the Language plugins 
which tells `ValueObjectPrinter` out to display "nil" references, so that 
there's no Language specific code in `ValueObjectPrinter`.

A few additional questions come to mind though:

- Was the intent if `IsNilReference()` to be Objective-C specific in the first 
place?
- Should we do the same thing for C and NULL?
- C, C++ and Objective-C(++) being highly intermingled, would something like 
this even do the right thing in all cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-01 Thread Frederic Riss via Phabricator via lldb-commits 
friss added a comment.

In D77153#1954154 , @labath wrote:

> It might be worth mentioning though that the title of the patch is somewhat 
> misleading -- I believe the `CXX` in `CXXFunctionSummaryFormat` refers to the 
> language the formatter is implemented in, not the language of the type 
> itself. So this patch affects all types whose formatters are implemented as 
> c++ functions, not all c++ pointers in the target program.


Wow, thanks for pointing this out. It should have clicked when I saw 
`ScriptSummaryFormat` besides it, but I was indeed completely confused. Let me 
rework this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D77153#1952952 , @friss wrote:

> I haven't tested the libstdc++ part, would be great if someone could confirm 
> this works.


This doesn't work work for libstdc++ (I get ``), presumably because the code uses the simple `StringSummaryFormat` 
(`new StringSummaryFormat(stl_summary_flags, "${var._M_dataplus._M_p}")`) 
instead of `CXXFunctionSummaryFormat` However, it doesn't make the situation 
any worse either, so you can just drop the libstdc++ test changes and carry on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

It might be worth mentioning though that the title of the patch is somewhat 
misleading -- I believe the `CXX` in `CXXFunctionSummaryFormat` refers to the 
language the formatter is implemented in, not the language of the type itself. 
So this patch affects all types whose formatters are implemented as c++ 
functions, not all c++ pointers in the target program.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-03-31 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp:13
 std::basic_string uchar(5, 'a');
+std::string *null = nullptr;
 S.assign(L"!"); // Set break point at this line.

How about `string_ptr_null` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-03-31 Thread Frederic Riss via Phabricator via lldb-commits 
friss added a comment.

I haven't tested the libstdc++ part, would be great if someone could confirm 
this works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-03-31 Thread Frederic Riss via Phabricator via lldb-commits 
friss created this revision.
friss added reviewers: jingham, labath.
Herald added a project: LLDB.
friss added a comment.

I haven't tested the libstdc++ part, would be great if someone could confirm 
this works.


The original motivation for this patch was to display a null
std::string pointer as nullptr instead of "", but the fix
seemed generic enough to be done for all C++ summary providers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77153

Files:
  lldb/source/DataFormatters/TypeSummary.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -10,6 +10,7 @@
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
 std::basic_string uchar(5, 'a');
+std::string *null = nullptr;
 S.assign(L"!"); // Set break point at this line.
 return 0;
 }
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -59,6 +59,7 @@
 var_q = self.frame().FindVariable('q')
 var_Q = self.frame().FindVariable('Q')
 var_uchar = self.frame().FindVariable('uchar')
+var_null = self.frame().FindVariable('null')
 
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary 
wrong")
 self.assertEqual(
@@ -76,6 +77,7 @@
 var_Q.GetSummary(), '"quite a long std::strin with lots of info 
inside it"',
 "Q summary wrong")
 self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
+self.assertEqual(var_null.GetSummary(), 'nullptr', "null summary 
wrong")
 
 self.runCmd("next")
 
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -71,6 +71,7 @@
 std::u32string u32_string(U"");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(std::string *) null = nullptr',
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(std::string *) null = nullptr',
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/source/DataFormatters/TypeSummary.cpp
===
--- lldb/source/DataFormatters/TypeSummary.cpp
+++ lldb/source/DataFormatters/TypeSummary.cpp
@@ -123,6 +123,17 @@
 std::string ,
 const TypeSummaryOptions ) 
{
   dest.clear();
+
+  if (valobj && valobj->IsPointerType()) {
+bool success = true;
+if (!valobj->GetValueAsUnsigned(0, )) {
+  if (!success)
+return false;
+  dest = "nullptr";
+  return true;
+}
+  }
+
   StreamString stream;
   if (!m_impl || !m_impl(*valobj, stream, options))
 return false;


Index: