jingham requested changes to this revision.
jingham added a comment.

It seems a little odd to choose to represent this as an array with 0 elements.  
I think I would be confused by that.  You can maybe choose a better name like 
"value" for your cloned object.  But it looks like it is also possible to make 
a Synthetic Child Provider that just replaces the value of the object with the 
value provided by the synthetic child provider.  That's done in the Python 
front end by saying you have no children and then providing a the "get_value" 
method.  It must be possible to do this from C++ as well, but it will take a 
little thought to figure out how that works.  But that would be the most 
natural way to present this, I think.



================
Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:26-38
+        self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+        bkpt = self.target().FindBreakpointByID(
+            lldbutil.run_break_set_by_source_regexp(
+                self, "break here"))
+
+        self.runCmd("run", RUN_SUCCEEDED)
----------------
you can do this more cleanly with:

(self.target, _, _, bkpt) = lldbutil.run_to_source_breakpoint(self, "break 
here", lldb.SBFileSpec("main.cpp"))


================
Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:39-48
+        # This is the function to remove the custom formats in order to have a
+        # clean slate for the next test case.
+        def cleanup():
+            self.runCmd('type format clear', check=False)
+            self.runCmd('type summary clear', check=False)
+            self.runCmd('type filter clear', check=False)
+            self.runCmd('type synth clear', check=False)
----------------
I don't think you need this cleanup, you aren't adding any formatters, you're 
just testing built-in ones.


================
Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63
+  if (!val_sp) {
+      fprintf( stderr, "no __val_!\n" ) ;
+    return ValueObjectSP();
----------------
If you want to log this, get the formatters log channel and put the text there. 
 Then somebody will see this when they turn on the log.  An fprintf like this 
won't help anybody.


https://reviews.llvm.org/D49271



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

Reply via email to