[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Interpreter/CommandInterpreter.cpp:2733
+
+  const char *k_space_characters = "\t\n\v\f\r ";
+  size_t first_non_space = line.find_first_not_of(k_space_characters);

This looks like duplicate code from from `HandleCommand` I also see that at the 
the top of the file there is `k_white_space` although I am not sure why it is 
not in a anonymous namespace and why perhaps it is not a `ConstString`



Comment at: source/Interpreter/CommandInterpreter.cpp:2939
 flags |= eHandleCommandFlagEchoCommand;
+  if (options->m_echo_commands != eLazyBoolNo)
+flags |= eHandleCommandFlagEchoCommentCommand;

Should this be `m_echo_comment_commands`?


https://reviews.llvm.org/D52788



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

[Lldb-commits] [PATCH] D52788: Add EchoCommentCommands to CommandInterpreterRunOptions in addition to the existing EchoCommands and expose both as interpreter settings.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Interpreter/CommandInterpreter.cpp:96
+{"echo-comment-commands", OptionValue::eTypeBoolean, true, true, nullptr,
+ {}, "If true, LLDB will print a command even if it is a pure  comment "
+ "line."}};

extra space between pure and comment


https://reviews.llvm.org/D52788



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

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, davide, aprantl.
Herald added a reviewer: EricWF.

- Adding support to step into the callable wrapped by libc++ std::function
- Adding test to validate that functionality


https://reviews.llvm.org/D52851

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,13 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+if (cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,77 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // Currently due to the the:
+  //
+  //target.process.thread.step-avoid-regexp
+  //
+  // setting we will currenly step right out of standard library code.
+  //
+  // The new behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  ret_plan_sp.reset(new ThreadPlanStepInRange(thread, range_of_curr_func,
+  sc, eOnlyThisThread,
+  eLazyBoolYes, eLazyBoolYes));
+  return ret_plan_sp;
+}
+  }
+
+  return ret_plan_sp;
+}
Index: packages/Python/lldbsuite/test/lang/cpp/std-func

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 168389.
shafik marked 6 inline comments as done.
shafik added a comment.

Addressing comments and fixing a bug in ThreadPlanStepThrough.cpp where I would 
overwrite  the result of objc_runtime.


https://reviews.llvm.org/D52851

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,13 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+if (!m_sub_plan_sp.get() && cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,77 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // Currently due to the the:
+  //
+  //target.process.thread.step-avoid-regexp
+  //
+  // setting we will currenly step right out of standard library code.
+  //
+  // The new behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  ret_plan_sp.reset(new ThreadPlanStepInRange(thread, range_of_curr_func,
+  sc, eOnlyThisThread,
+  eLazyBoolYes, eLazyBoolYes));
+  return ret_plan_sp;
+}
+  }
+
+  return ret_plan_sp;
+}
Index: packages/Python/lldbsuite/t

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 169105.
shafik marked 5 inline comments as done.
shafik added a comment.

- Addressing comments
- Adding test to make sure we step through a std::function wrapping a member 
variable


https://reviews.llvm.org/D52851

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,15 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+// If the ObjC runtime did not provide us with a step though plan then if we
+// have it check the C++ runtime for a step though plan.
+if (!m_sub_plan_sp.get() && cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,76 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // The behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  // We found the std::function wrapped callable and we have its address.
+  // We now create a ThreadPlan to run to the callable.
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  // We are in std::function but we could not obtain the callable.
+  // We create a ThreadPlan to keep stepping through using the address range
+  // of the current function.
+  ret_plan_sp.reset(new ThreadPlanStepInR

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham I believe I addressed your comments, please review again.


https://reviews.llvm.org/D52851



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

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 169259.
shafik marked an inline comment as done.
shafik added a comment.

Removing uneeded code and making test NO_DEBUG_INFO_TESTCASE


https://reviews.llvm.org/D52851

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,15 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+// If the ObjC runtime did not provide us with a step though plan then if we
+// have it check the C++ runtime for a step though plan.
+if (!m_sub_plan_sp.get() && cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,76 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // The behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  // We found the std::function wrapped callable and we have its address.
+  // We now create a ThreadPlan to run to the callable.
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  // We are in std::function but we could not obtain the callable.
+  // We create a ThreadPlan to keep stepping through using the address range
+  // of the current function.
+  ret_plan_sp.reset(new ThreadPlanStepInRange(thread, range_of_curr_func,
+  

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham removed code and made it NO_DEBUG_INFO_TESTCASE


https://reviews.llvm.org/D52851



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

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344371: Adding support to step into the callable wrapped by 
libc++ std::function (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52851?vs=169259&id=169453#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52851

Files:
  lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  lldb/trunk/source/Target/CPPLanguageRuntime.cpp
  lldb/trunk/source/Target/ThreadPlanStepThrough.cpp

Index: lldb/trunk/source/Target/CPPLanguageRuntime.cpp
===
--- lldb/trunk/source/Target/CPPLanguageRuntime.cpp
+++ lldb/trunk/source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,76 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // The behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  // We found the std::function wrapped callable and we have its address.
+  // We now create a ThreadPlan to run to the callable.
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  // We are in std::function but we could not obtain the callable.
+  // We create a ThreadPlan to keep stepping through using the address range
+  // of the current function.
+  ret_plan_sp.reset(new ThreadPlanStepInRange(thread, range_of_curr_func,
+  sc, eOnlyThisThread,
+  eLazyBoolYes, eLazyBoolYes));
+  return ret_plan_sp;
+}
+  }
+
+  return ret_plan_sp;
+}
Index: lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
===
--- lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
+++ lldb/trunk/source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,15 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->G

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 2 inline comments as done.
shafik added a comment.

@stella.stamenova Thank you for catching this. I fixed the test names, I am 
looking into the best way to fix the skipif now.


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: packages/Python/lldbsuite/test/decorators.py:194
 py_version[0], py_version[1], sys.version_info)
-skip_for_macos_version = (macos_version is None) or (
+skip_for_macos_version = (macos_version is None) or 
(platform.mac_ver()[0] == "") or (
 _check_expected_version(

If I am reading the following code correctly this will default to skipping when 
`platform.mac_ver()[0]  == ""` shouldn't it be the other way around? 


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53208



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

[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

+1 for modernizing code!




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2262
+  // assertion in the call to clang_type.TransferBaseClasses()
+  for (auto &base_class : bases) {
 clang::TypeSourceInfo *type_source_info =

`const auto &`



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3274
+  if (!result)
+break;
+

Is this correct? the if/else below does not seem dependent on this result?


https://reviews.llvm.org/D53590



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

[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, davide.
Herald added a reviewer: EricWF.
Herald added a subscriber: christof.

We currently support formatting for std::string and std::wstring but not 
std::u16string and std::u32string which was added with C++11. This adds support 
for these two types via refactoring for the existing std::string formatter.


https://reviews.llvm.org/D53656

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h

Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -19,9 +19,17 @@
 namespace lldb_private {
 namespace formatters {
 
-bool LibcxxStringSummaryProvider(
+bool LibcxxStringSummaryProviderASCII(
 ValueObject &valobj, Stream &stream,
-const TypeSummaryOptions &options); // libc++ std::string
+const TypeSummaryOptions &summary_options); // libc++ std::string
+
+bool LibcxxStringSummaryProviderUTF16(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &summary_options); // libc++ std::u16string
+
+bool LibcxxStringSummaryProviderUTF32(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &summary_options); // libc++ std::u32string
 
 bool LibcxxWStringSummaryProvider(
 ValueObject &valobj, Stream &stream,
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -599,9 +599,10 @@
   return true;
 }
 
-bool lldb_private::formatters::LibcxxStringSummaryProvider(
-ValueObject &valobj, Stream &stream,
-const TypeSummaryOptions &summary_options) {
+template 
+bool LibcxxStringSummaryProvider(ValueObject &valobj, Stream &stream,
+ const TypeSummaryOptions &summary_options,
+ std::string prefix_token = "") {
   uint64_t size = 0;
   ValueObjectSP location_sp;
 
@@ -630,12 +631,37 @@
 
   options.SetData(extractor);
   options.SetStream(&stream);
-  options.SetPrefixToken(nullptr);
+
+  if (prefix_token.empty())
+options.SetPrefixToken(nullptr);
+  else
+options.SetPrefixToken(prefix_token);
+
   options.SetQuote('"');
   options.SetSourceSize(size);
   options.SetBinaryZeroIsTerminator(false);
-  StringPrinter::ReadBufferAndDumpToStream<
-  StringPrinter::StringElementType::ASCII>(options);
+  StringPrinter::ReadBufferAndDumpToStream(options);
 
   return true;
 }
+
+bool lldb_private::formatters::LibcxxStringSummaryProviderASCII(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &summary_options) {
+  return LibcxxStringSummaryProvider(
+  valobj, stream, summary_options);
+}
+
+bool lldb_private::formatters::LibcxxStringSummaryProviderUTF16(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &summary_options) {
+  return LibcxxStringSummaryProvider(
+  valobj, stream, summary_options, "u");
+}
+
+bool lldb_private::formatters::LibcxxStringSummaryProviderUTF32(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &summary_options) {
+  return LibcxxStringSummaryProvider(
+  valobj, stream, summary_options, "U");
+}
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -405,8 +405,17 @@
 
 #ifndef LLDB_DISABLE_PYTHON
   lldb::TypeSummaryImplSP std_string_summary_sp(new CXXFunctionSummaryFormat(
-  stl_summary_flags, lldb_private::formatters::LibcxxStringSummaryProvider,
+  stl_summary_flags,
+  lldb_private::formatters::LibcxxStringSummaryProviderASCII,
   "std::string summary provider"));
+  lldb::TypeSummaryImplSP std_stringu16_summary_sp(new CXXFunctionSummaryFormat(
+  stl_summary_flags,
+  lldb_private::formatters::LibcxxStringSummaryProviderUTF16,
+  "std::u16string summary provider"));
+  lldb::TypeSummaryImplSP std_stringu32_summary_sp(new CXXFunctionSummaryFormat(
+  stl_summary_flags,
+  lldb_private::formatters::LibcxxStringSummaryProviderUTF32,
+  "std::u32string summary provider"));
   lldb::TypeSummaryImplSP std_wstring_summary_sp(new CXXFunctionSummaryFormat(
   stl_summary_flags, lldb_private::formatters::LibcxxWStringSummaryProvider,

[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1405
 
-llvm::StringRef version_part;
-
-if (sdk_name.startswith(sdk_strings[(int)desired_type])) {
-  version_part =
-  sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
-} else {
+if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
   return false;

Could we avoid C-style casts, and in this case use `static_cast` although in 
this case it is obvious, using explicit casts avoids bugs in the long-term due 
to changes in type and cv-qualifiers etc... and makes your intent clear.

I personally would advocate for -Wold-style-cast being a hard error in C++ code.



Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1408
+auto version_part =
+sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+version_part.consume_back(".sdk");

Same comment here.


https://reviews.llvm.org/D53677



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

[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

LGTM


https://reviews.llvm.org/D53709



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

[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345402: [DataFormatters] Adding formatters for libc++ 
std::u16string and std::u32string (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53656?vs=170921&id=171313#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53656

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h

Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile
@@ -4,4 +4,4 @@
 
 USE_LIBCPP := 1
 include $(LEVEL)/Makefile.rules
-CXXFLAGS += -O0
+CXXFLAGS += -std=c++11 -O0
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -10,6 +10,8 @@
 std::string TheVeryLongOne("1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890someText123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901

[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Are we refactoring in the right place? Why not just refactor 
`FileSpec::GetByteSize()` why is `FileSpec` the wrong place?




Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:718
+  (FileSystem::Instance().GetByteSize(file) - file_offset) /
+  1024);
 

`1024u` just for consistency, I don't expect this is practically a problem.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53788



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, davide, teemperor.
Herald added a subscriber: JDevlieghere.

Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove 
redundant parameter which can be calculated from other parameter.

Both calling sites of the sites were incorrectly calculating the qualtype as 
the underlying type unconditionally irrespective of whether the enum was 
unscoped or scoped


https://reviews.llvm.org/D54003

Files:
  include/lldb/Symbol/ClangASTContext.h
  packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
  
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
  packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8848,43 +8848,53 @@
 }
 
 clang::EnumConstantDecl *ClangASTContext::AddEnumerationValueToEnumerationType(
-lldb::opaque_compiler_type_t type,
-const CompilerType &enumerator_clang_type, const Declaration &decl,
-const char *name, int64_t enum_value, uint32_t enum_value_bit_size) {
-  if (type && enumerator_clang_type.IsValid() && name && name[0]) {
-clang::QualType enum_qual_type(GetCanonicalQualType(type));
-
-bool is_signed = false;
-enumerator_clang_type.IsIntegerType(is_signed);
-const clang::Type *clang_type = enum_qual_type.getTypePtr();
-if (clang_type) {
-  const clang::EnumType *enutype =
-  llvm::dyn_cast(clang_type);
-
-  if (enutype) {
-llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
-enum_llvm_apsint = enum_value;
-clang::EnumConstantDecl *enumerator_decl =
-clang::EnumConstantDecl::Create(
-*getASTContext(), enutype->getDecl(), clang::SourceLocation(),
-name ? &getASTContext()->Idents.get(name)
- : nullptr, // Identifier
-ClangUtil::GetQualType(enumerator_clang_type),
-nullptr, enum_llvm_apsint);
-
-if (enumerator_decl) {
-  enutype->getDecl()->addDecl(enumerator_decl);
+const CompilerType enum_type, const Declaration &decl, const char *name,
+int64_t enum_value, uint32_t enum_value_bit_size) {
+
+  if (!enum_type || ConstString(name).IsEmpty())
+return nullptr;
+
+  lldb::opaque_compiler_type_t enum_opaque_compiler_type =
+  enum_type.GetOpaqueQualType();
+
+  if (!enum_opaque_compiler_type)
+return nullptr;
+
+  CompilerType underlying_type =
+  GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
+
+  clang::QualType enum_qual_type(
+  GetCanonicalQualType(enum_opaque_compiler_type));
+
+  bool is_signed = false;
+  underlying_type.IsIntegerType(is_signed);
+  const clang::Type *clang_type = enum_qual_type.getTypePtr();
+
+  if (!clang_type)
+return nullptr;
+
+  const clang::EnumType *enutype = llvm::dyn_cast(clang_type);
+
+  if (!enutype)
+return nullptr;
+
+  llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
+  enum_llvm_apsint = enum_value;
+  clang::EnumConstantDecl *enumerator_decl = clang::EnumConstantDecl::Create(
+  *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
+  name ? &getASTContext()->Idents.get(name) : nullptr, // Identifier
+  clang::QualType(enutype, 0), nullptr, enum_llvm_apsint);
+
+  if (!enumerator_decl)
+return nullptr;
+
+  enutype->getDecl()->addDecl(enumerator_decl);
 
 #ifdef LLDB_CONFIGURATION_DEBUG
-  VerifyDecl(enumerator_decl);
+  VerifyDecl(enumerator_decl);
 #endif
 
-  return enumerator_decl;
-}
-  }
-}
-  }
-  return nullptr;
+  return enumerator_decl;
 }
 
 CompilerType
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1086,8 +1086,7 @@
   uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
   ClangUtil::GetQualType(underlying_type));
   auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType(
-  enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(),
-  raw_value, byte_size * 8);
+  enum_type, decl, name.c_str(), raw_value, byte_size * 8);
   if (!enum_constant_decl)
 return false;
 
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2501,9 +2501,7 @@
 
 if (name && name[0] && got_value) {
   m_ast.AddEnumerationValueToEnumeratio

[Lldb-commits] [PATCH] D54074: CPlusPlusLanguage: Use new demangler API to implement type substitution

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:336
+  llvm::itanium_demangle::Node *parseType() {
+if (llvm::StringRef(First, Last - First).startswith(Search)) {
+  Result += llvm::StringRef(Written, First - Written);

Can you just use `numLeft()` here? It would be less opaque.



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:337
+if (llvm::StringRef(First, Last - First).startswith(Search)) {
+  Result += llvm::StringRef(Written, First - Written);
+  Result += Replace;

also `First - Written` is used twice but it is not obvious what the means, 
naming it via a member function might be helpful.


https://reviews.llvm.org/D54074



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

[Lldb-commits] [PATCH] D54074: CPlusPlusLanguage: Use new demangler API to implement type substitution

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Much better, thank you!


Repository:
  rL LLVM

https://reviews.llvm.org/D54074



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@zturner I don't see `AddEnumerationValueToEnumerationType` being called in 
`SymbolFile/NativePDB.cpp` 
https://github.com/llvm-mirror/lldb/search?q=AddEnumerationValueToEnumerationType&unscoped_q=AddEnumerationValueToEnumerationType


https://reviews.llvm.org/D54003



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@zturner I was out of date when I posted this diff but I have that file updated 
locally and it will show up when I update the diff.


https://reviews.llvm.org/D54003



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 173083.
shafik marked 8 inline comments as done.
shafik added a comment.

Made changes based on comments.


https://reviews.llvm.org/D54003

Files:
  include/lldb/Symbol/ClangASTContext.h
  packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
  
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
  packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8876,43 +8876,55 @@
 }
 
 clang::EnumConstantDecl *ClangASTContext::AddEnumerationValueToEnumerationType(
-lldb::opaque_compiler_type_t type,
-const CompilerType &enumerator_clang_type, const Declaration &decl,
-const char *name, int64_t enum_value, uint32_t enum_value_bit_size) {
-  if (type && enumerator_clang_type.IsValid() && name && name[0]) {
-clang::QualType enum_qual_type(GetCanonicalQualType(type));
-
-bool is_signed = false;
-enumerator_clang_type.IsIntegerType(is_signed);
-const clang::Type *clang_type = enum_qual_type.getTypePtr();
-if (clang_type) {
-  const clang::EnumType *enutype =
-  llvm::dyn_cast(clang_type);
-
-  if (enutype) {
-llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
-enum_llvm_apsint = enum_value;
-clang::EnumConstantDecl *enumerator_decl =
-clang::EnumConstantDecl::Create(
-*getASTContext(), enutype->getDecl(), clang::SourceLocation(),
-name ? &getASTContext()->Idents.get(name)
- : nullptr, // Identifier
-ClangUtil::GetQualType(enumerator_clang_type),
-nullptr, enum_llvm_apsint);
-
-if (enumerator_decl) {
-  enutype->getDecl()->addDecl(enumerator_decl);
+const CompilerType &enum_type, const Declaration &decl, const char *name,
+int64_t enum_value, uint32_t enum_value_bit_size) {
+
+  if (!enum_type || ConstString(name).IsEmpty())
+return nullptr;
+
+  lldbassert(enum_type.GetTypeSystem() == static_cast(this));
+
+  lldb::opaque_compiler_type_t enum_opaque_compiler_type =
+  enum_type.GetOpaqueQualType();
+
+  if (!enum_opaque_compiler_type)
+return nullptr;
+
+  CompilerType underlying_type =
+  GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
+
+  clang::QualType enum_qual_type(
+  GetCanonicalQualType(enum_opaque_compiler_type));
+
+  bool is_signed = false;
+  underlying_type.IsIntegerType(is_signed);
+  const clang::Type *clang_type = enum_qual_type.getTypePtr();
+
+  if (!clang_type)
+return nullptr;
+
+  const clang::EnumType *enutype = llvm::dyn_cast(clang_type);
+
+  if (!enutype)
+return nullptr;
+
+  llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
+  enum_llvm_apsint = enum_value;
+  clang::EnumConstantDecl *enumerator_decl = clang::EnumConstantDecl::Create(
+  *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
+  name ? &getASTContext()->Idents.get(name) : nullptr, // Identifier
+  clang::QualType(enutype, 0), nullptr, enum_llvm_apsint);
+
+  if (!enumerator_decl)
+return nullptr;
+
+  enutype->getDecl()->addDecl(enumerator_decl);
 
 #ifdef LLDB_CONFIGURATION_DEBUG
-  VerifyDecl(enumerator_decl);
+  VerifyDecl(enumerator_decl);
 #endif
 
-  return enumerator_decl;
-}
-  }
-}
-  }
-  return nullptr;
+  return enumerator_decl;
 }
 
 CompilerType
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1122,8 +1122,7 @@
   uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
   ClangUtil::GetQualType(underlying_type));
   auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType(
-  enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(),
-  raw_value, byte_size * 8);
+  enum_type, decl, name.c_str(), raw_value, byte_size * 8);
   if (!enum_constant_decl)
 return false;
 
Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -159,13 +159,9 @@
   TypeSP underlying_type =
   m_symbol_file.GetOrCreateType(m_cvr.er.getUnderlyingType());
 
-  lldb::opaque_compiler_type_t enum_qt = m_derived_ct.GetOpaqueQualType();
-
-  CompilerType enumerator_type = clang.GetEnumerationIntegerType(enum_qt);
   

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: include/lldb/Symbol/ClangASTContext.h:909
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
-  lldb::opaque_compiler_type_t type,
-  const CompilerType &enumerator_qual_type, const Declaration &decl,
-  const char *name, int64_t enum_value, uint32_t enum_value_bit_size);
+  const CompilerType enum_type, const Declaration &decl, const char *name,
+  int64_t enum_value, uint32_t enum_value_bit_size);

jingham wrote:
> clayborg wrote:
> > teemperor wrote:
> > > Can we pass `enum_type` as const ref?
> > CompilerType instances are two pointers. Pretty cheap to copy.
> We're not entirely consistent, but a quick glance through headers show us 
> almost always choosing to pass "const CompilerType &" over "const 
> CompilerType".
That was a good catch, thank you!


https://reviews.llvm.org/D54003



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham @zturner @teemperor @clayborg  I believe I have addressed all the 
comments.


https://reviews.llvm.org/D54003



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

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346428: Refactor 
ClangASTContext::AddEnumerationValueToEnumerationType() to remove… (authored by 
shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54003?vs=173083&id=173197#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54003

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -8876,43 +8876,55 @@
 }
 
 clang::EnumConstantDecl *ClangASTContext::AddEnumerationValueToEnumerationType(
-lldb::opaque_compiler_type_t type,
-const CompilerType &enumerator_clang_type, const Declaration &decl,
-const char *name, int64_t enum_value, uint32_t enum_value_bit_size) {
-  if (type && enumerator_clang_type.IsValid() && name && name[0]) {
-clang::QualType enum_qual_type(GetCanonicalQualType(type));
-
-bool is_signed = false;
-enumerator_clang_type.IsIntegerType(is_signed);
-const clang::Type *clang_type = enum_qual_type.getTypePtr();
-if (clang_type) {
-  const clang::EnumType *enutype =
-  llvm::dyn_cast(clang_type);
+const CompilerType &enum_type, const Declaration &decl, const char *name,
+int64_t enum_value, uint32_t enum_value_bit_size) {
+
+  if (!enum_type || ConstString(name).IsEmpty())
+return nullptr;
+
+  lldbassert(enum_type.GetTypeSystem() == static_cast(this));
+
+  lldb::opaque_compiler_type_t enum_opaque_compiler_type =
+  enum_type.GetOpaqueQualType();
+
+  if (!enum_opaque_compiler_type)
+return nullptr;
+
+  CompilerType underlying_type =
+  GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
+
+  clang::QualType enum_qual_type(
+  GetCanonicalQualType(enum_opaque_compiler_type));
+
+  bool is_signed = false;
+  underlying_type.IsIntegerType(is_signed);
+  const clang::Type *clang_type = enum_qual_type.getTypePtr();
 
-  if (enutype) {
-llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
-enum_llvm_apsint = enum_value;
-clang::EnumConstantDecl *enumerator_decl =
-clang::EnumConstantDecl::Create(
-*getASTContext(), enutype->getDecl(), clang::SourceLocation(),
-name ? &getASTContext()->Idents.get(name)
- : nullptr, // Identifier
-ClangUtil::GetQualType(enumerator_clang_type),
-nullptr, enum_llvm_apsint);
+  if (!clang_type)
+return nullptr;
+
+  const clang::EnumType *enutype = llvm::dyn_cast(clang_type);
+
+  if (!enutype)
+return nullptr;
+
+  llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
+  enum_llvm_apsint = enum_value;
+  clang::EnumConstantDecl *enumerator_decl = clang::EnumConstantDecl::Create(
+  *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
+  name ? &getASTContext()->Idents.get(name) : nullptr, // Identifier
+  clang::QualType(enutype, 0), nullptr, enum_llvm_apsint);
+
+  if (!enumerator_decl)
+return nullptr;
 
-if (enumerator_decl) {
-  enutype->getDecl()->addDecl(enumerator_decl);
+  enutype->getDecl()->addDecl(enumerator_decl);
 
 #ifdef LLDB_CONFIGURATION_DEBUG
-  VerifyDecl(enumerator_decl);
+  VerifyDecl(enumerator_decl);
 #endif
 
-  return enumerator_decl;
-}
-  }
-}
-  }
-  return nullptr;
+  return enumerator_decl;
 }
 
 CompilerType
Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1122,8 +1122,7 @@
   uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
   ClangUtil::GetQualType(underlying_type));
   auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType(
-  enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(),
-  raw_value, byte_size * 8);
+  enum_type, decl, name.c_str(), raw_value, byte_size * 8);
   if (!enum_constant_decl)
 return false;
 
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@zturner I would not be against discussing using pass by value for small 
objects going forward. I don't currently have a good feeling for at what 
sizes/data types the right trade-off is at though.


Repository:
  rL LLVM

https://reviews.llvm.org/D54003



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

[Lldb-commits] [PATCH] D54452: [NativePDB] Add support for handling S_CONSTANT records

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:329
 
+std::pair GetIntegralTypeInfo(TypeIndex ti, TpiStream &tpi) {
+  if (ti.isSimple()) {

lemo wrote:
> Just a suggestion: I'm not a big fan of returning std::pair<>. I'd use a 
> simple struct instead.
I have to concur on this, although the use of `std::tie` helps alleviate the 
pain i.e. no `second` and `first` it does feel more verbose on the calling side 
to declare two variables and then do the tie and it relies on the calling to 
choose meaningful names which can't be relied on if the use spreads and people 
are not as careful.


https://reviews.llvm.org/D54452



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

[Lldb-commits] [PATCH] D54528: [lldb] NFC: Remove the extra ';'

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for fixing this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54528



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

[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added subscribers: stella.stamenova, shafik.
shafik added a comment.

Hello,

This PR broke the green dragon bots, see the following log file:

http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/13655/consoleFull#-15796076f80f5c9c-2aaa-47fb-b15d-be39b7128d72

I was about to revert it but it looks @stella.stamenova landed a fix that looks 
good:

http://llvm.org/viewvc/llvm-project?view=revision&revision=348542

Please watch the bots after you land a change.


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

https://reviews.llvm.org/D53094



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

[Lldb-commits] [PATCH] D55584: Simplify boolean expressions

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I find the `static_cast` to be a bit too clever, I don't think it helps 
readability.

This is also too large to digest in a way I would feel satisfied that I did not 
miss anything.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55584



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

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision as: shafik.
shafik added a comment.

LGTM after comment are addressed.




Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5894
+if (!success) {
+  offset = MachHeaderSizeFromMagic(m_header.magic);
+  for (uint32_t i = 0; !success && i < m_header.ncmds; ++i) {

This looks like a big chunk of unrelated changes.



Comment at: 
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1725
 // not support qHostInfo or qWatchpointSupportInfo packets.
-if (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
-atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el ||
-atype == llvm::Triple::ppc64le)
-  after = false;
-else
-  after = true;
+after =
+!(atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||

Not sure if I like this one, the previous form feels more readable.



Comment at: source/Plugins/Process/mach-core/ProcessMachCore.cpp:305
   std::string corefile_identifier = core_objfile->GetIdentifierString();
-  if (found_main_binary_definitively == false 
-  && corefile_identifier.find("Darwin Kernel") != std::string::npos) {
-  UUID uuid;
-  addr_t addr = LLDB_INVALID_ADDRESS;
-  if (corefile_identifier.find("UUID=") != std::string::npos) {
-  size_t p = corefile_identifier.find("UUID=") + strlen("UUID=");
-  std::string uuid_str = corefile_identifier.substr(p, 36);
-  uuid.SetFromStringRef(uuid_str);
-  }
-  if (corefile_identifier.find("stext=") != std::string::npos) {
-  size_t p = corefile_identifier.find("stext=") + strlen("stext=");
-  if (corefile_identifier[p] == '0' && corefile_identifier[p + 1] == 
'x') {
-  errno = 0;
-  addr = ::strtoul(corefile_identifier.c_str() + p, NULL, 16);
-  if (errno != 0 || addr == 0)
-  addr = LLDB_INVALID_ADDRESS;
-  }
-  }
-  if (uuid.IsValid() && addr != LLDB_INVALID_ADDRESS) {
-  m_mach_kernel_addr = addr;
-  found_main_binary_definitively = true;
-  if (log)
-log->Printf("ProcessMachCore::DoLoadCore: Using the kernel address 
0x%" PRIx64
-" from LC_IDENT/LC_NOTE 'kern ver str' string: '%s'", 
addr, corefile_identifier.c_str());
+  if (!found_main_binary_definitively &&
+  corefile_identifier.find("Darwin Kernel") != std::string::npos) {

Another big chunk of unrelated changes.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:1117
   if ((distance(range.first, range.second) == 1) &&
-  range.first->end_sequence == true) {
+  static_cast(range.first->end_sequence)) {
 *range.first = state;

The `static_cast` feels less readable here.



Comment at: source/Symbol/LineTable.cpp:217
 if (prev_pos->file_addr == search_entry.file_addr &&
-prev_pos->is_terminal_entry == false)
+!static_cast(prev_pos->is_terminal_entry))
   --pos;

static_cast



Comment at: source/Symbol/LineTable.cpp:236
 // terminating entry for a previous line...
-if (pos != end_pos && pos->is_terminal_entry == false) {
+if (pos != end_pos && !static_cast(pos->is_terminal_entry)) {
   uint32_t match_idx = std::distance(begin_pos, pos);

static_cast



Comment at: source/Symbol/Type.cpp:749
 bool TypeAndOrName::IsEmpty() const {
-  if ((bool)m_type_name || (bool)m_type_pair)
-return false;
-  else
-return true;
+  return !((bool)m_type_name || (bool)m_type_pair);
 }

Oh, even worse C-style casts ... can we remove these. I am assuming they are 
triggering a member conversion function.



Comment at: source/Utility/RegisterValue.cpp:478
   if (this == &rhs)
-return rhs.m_type == eTypeInvalid ? false : true;
+return rhs.m_type != eTypeInvalid;
 

Nice one.


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

https://reviews.llvm.org/D55584



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, friss, jasonmolenda.
Herald added subscribers: christof, mgorny.
Herald added a reviewer: EricWF.

Add formattors for libc++ std::optional and tests to verify the new formattors.


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,91 @@
+//===-- LibCxxOptional.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd: public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_elements.size(); }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  std::vector m_elements;
+  ValueObjectSP m_base_sp;
+};
+}
+
+bool OptionalFrontEnd::Update() {
+  m_elements.clear();
+
+  ValueObjectSP engaged_sp( m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp) {
+return false;
+  }
+
+  uint64_t num_elements = engaged_sp->GetValueAsSigned(0) ;
+
+  m_elements.assign(num_elements, ValueObjectSP());
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_elements.size()) {
+return ValueObjectSP();
+  }
+
+  //ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__val_"), true));
+  ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__engaged_") , true)->GetParent()->GetChildAtIndex(0,true)->GetChildMemberWithName(ConstString("__val_"), true) );
+
+  if (!val_sp) {
+  fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();
+  }
+
+  if (m_elements[idx]) {
+return m_elements[idx];
+  }
+
+  CompilerType holder_type =
+val_sp->GetCompilerType() ;
+
+  if (!holder_type) {
+  fprintf( stderr, "no holder_type!\n" ) ;
+return ValueObjectSP();
+  }
+
+m_elements[idx] =
+val_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str()));
+
+  return m_elements[idx];
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+   lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,12 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions
+&options); // libc++ std::optional<>
+
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +139,9 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) ;
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,25 @@
 using namespace lldb_private;
 using nam

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 155508.
shafik marked 7 inline comments as done.
shafik edited the summary of this revision.
shafik added a comment.

Address comments

- Applying clang-formt
- Refactoring OptionalFrontEnd to produce out that makes the underlying data 
look like an array
- Removing commented out code and left in debugging
- Using StringRef instead of const char *


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  m_size = engaged_sp->GetValueAsSigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+return f

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@davide @jingham Thank you for the review, those were good catches and comments.

I have addressed them except for refactoring the test to use lldbInline which I 
will be working on now.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:14-38
+optional_int number ;
+
+printf( "%d\n", number.has_value() ) ; // break here
+
+number = 42 ;
+
+printf( "%d\n", *number ) ; // break here

davide wrote:
> You don't really need to se all these breakpoints.
> You just need set one on the return and print all the types.
> Also, an `lldbInline` python test here should suffice and it's probably more 
> readable (grep for lldbInline).
This looks like a good idea, I will try it out and let you know.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63
+  if (!val_sp) {
+  fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();

davide wrote:
> jingham wrote:
> > 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.
> if you want to log diagnostics, you might consider using the `LOG` facility 
> and then add these to the `data formatters` channel.
Thank you for catching, I meant to remove this.


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 155618.
shafik added a comment.

Removing unrelated changes due to applying clang-format to patch file with 
context.


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  m_size = engaged_sp->GetValueAsSigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+return false;
+
+  ValueObjectSP engaged_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  llvm::StringRef engaged_as_cstring(
+  engaged_sp->GetValueAsUnsigned

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 155758.
shafik added a comment.

Refactoring test to use lldbinline


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  m_size = engaged_sp->GetValueAsSigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+return false;
+
+  ValueObjectSP engaged_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  llvm::StringRef engaged_as_cstring(
+  engaged_sp->GetValueAsUnsigned(0) == 1 ? "true" : "false");
+
+  stream.Printf("

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 5 inline comments as done and 3 inline comments as done.
shafik added a comment.

@jingham @davide I believe I have addressed all your comments


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 156886.
shafik marked 4 inline comments as done.
shafik added a comment.

Addressing comments

- -O0 is not needed in Makefile
- engaged is not friendly terminology so switching to "Has Value"
- Switching test away from lldbinline style due to bug w/ .categories files 
which does not work on these tests


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  m_size = engaged_sp->GetValueAsSigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+return false;
+
+  ValueObjectSP engaged_sp(
+  v

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 6 inline comments as done.
shafik added a comment.

@davide One more pass




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__, globals(), [no_debug_info_test])

jingham wrote:
> davide wrote:
> > I do wonder if you need a decorator to indicate that this is a libcxx only 
> > test (and skip everywhere the library isn't supported).
> Yes, you do need to mark this or you will get spurious failures at least on 
> Windows, which turns off the libcxx category.
Switched back to the old style tests due to the .categories bug we discussed 
earlier.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:42-51
+  ValueObjectSP engaged_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  llvm::StringRef engaged_as_cstring(

davide wrote:
> This is really obscure to somebody who doesn't understand the type 
> internally. Can you add a comment explaining the structure? (here or in the 
> synthetic)
That is a good point, I am looking at the cppreference page for it and I think 
maybe has_value might be an improvement. 


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 157337.
shafik added a comment.

Adding additional comments


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,28 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ 

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 158121.
shafik added a comment.

Adjust test to filter on clang version and libc++ version to precent build bots 
from failing due to lack of C++17 or lack of optional support


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,28 @@
 using namespace lldb_pri

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@davide @labath I believe I have addressed both the C++17 filter and the libc++ 
filter. Please let me know if you have further comments.


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 158342.
shafik marked 2 inline comments as done.
shafik added a comment.

- Adding comments
- Changing from exit to self.skipTest
- Adding skip for gcc


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,28 @@
 using namespace lldb_private;
 using namespace l

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@add_test_categories(["libc++"])
+@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '5.0'])
+

labath wrote:
> Could you add another line for `gcc` here? The -std=c++17 flag seems to be 
> supported starting with gcc-5.1.
> 
> Also a comment that this is due to the -std flag would be helpful to people 
> looking at this in the future.
Adding a comment makes sense.

So `@add_test_categories(["libc++"])` won't skip for gcc bots then?


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 158362.
shafik added a comment.

Fixing gcc skipIf to check for version as well


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,28 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptiona

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 158662.
shafik added a comment.

Simplifying initialization of has_optional in test.


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,28 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOp

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 4 inline comments as done.
shafik added a comment.

@labath Addressed comment, thank you for helping out.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@add_test_categories(["libc++"])
+@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '5.0'])
+

labath wrote:
> shafik wrote:
> > labath wrote:
> > > Could you add another line for `gcc` here? The -std=c++17 flag seems to 
> > > be supported starting with gcc-5.1.
> > > 
> > > Also a comment that this is due to the -std flag would be helpful to 
> > > people looking at this in the future.
> > Adding a comment makes sense.
> > 
> > So `@add_test_categories(["libc++"])` won't skip for gcc bots then?
> It won't because it doesn't make sense to do that. Gcc is perfectly capable 
> of using libc++. The only tricky thing is that it doesn't have the magic 
> `-stdlib` argument, so you have to specify the include paths and libraries 
> explicitly (which our makefiles do).
Good point I did not realize this before! 



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:17-20
+bool has_optional = false ;
+#if HAVE_OPTIONAL == 1
+has_optional = true ;
+#endif

labath wrote:
> This could be simplified to `bool has_optional = HAVE_OPTIONAL;`
Good catch!


https://reviews.llvm.org/D49271



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

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, davide.
Herald added a reviewer: EricWF.
Herald added a subscriber: christof.

Adding formatter summary for std::function.

  
  - Added LibcxxFunctionSummaryProvider
  - Removed LibcxxFunctionFrontEnd
  - Modified data formatter tests to test new summary functionality


https://reviews.llvm.org/D50864

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h

Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -32,6 +32,10 @@
 const TypeSummaryOptions
 &options); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
+bool LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::function<>
+
 SyntheticChildrenFrontEnd *
 LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *,
  lldb::ValueObjectSP);
@@ -124,9 +128,6 @@
 LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
-SyntheticChildrenFrontEnd *LibcxxFunctionFrontEndCreator(CXXSyntheticChildren *,
- lldb::ValueObjectSP);
-
 SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -23,6 +23,7 @@
 #include "lldb/DataFormatters/VectorIterator.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/ProcessStructReader.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +34,176 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+
+  if (!valobj_sp)
+return false;
+
+  // Member __f_ has type __base*, the contents of which will either directly
+  // hold a pointer to the callable object or vtable entry which will hold the
+  // type information need to discover the lambda being called.
+  ValueObjectSP member__f_(
+  valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  // We may need to increment pointers into __f_ so we need to know the size
+  uint32_t address_size = process->GetAddressByteSize();
+
+  if (process != nullptr) {
+Status status;
+
+// First item pointed to by __f_ should be the pointer to the vtable for
+// a __base object.
+lldb::addr_t vtable_address =
+process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+if (status.Fail()) {
+  return false;
+}
+
+lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+lldb::addr_t possible_function_address =
+process->ReadPointerFromMemory(address_after_vtable, status);
+
+Target &target = process->GetTarget();
+
+if (!target.GetSectionLoadList().IsEmpty()) {
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (target.GetSectionLoadList().ResolveLoadAddress(
+  vtable_address, vtable_addr_resolved)) {
+
+target.GetImages().ResolveSymbolContextForAddress(
+vtable_addr_resolved, eSymbolContextEverything, sc);
+
+symbol = sc.symbol;
+if (symbol != NULL) {
+  llvm::StringRef vtable_name(symbol->GetName().GetCString());
+  bool found_expected_start_string = vtable_name.startswith(
+  "vtable for std::__1::__function::__func<");
+
+  if (found_expected_start_string) {
+// Given a vtable name, we are want to extract the first template
+// parameter
+//
+//  ... __func ...
+// ^
+//
+// We do this by find the first < and , and extracting in
+// between.
+//
+// This 

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 161837.
shafik marked 12 inline comments as done.
shafik added a comment.

Fixes based on comments

- Switched to early exits
- Added a lot more comments to explain all the cases being dealt with and 
noting which cases different sections are dealing with


https://reviews.llvm.org/D50864

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h

Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -32,6 +32,10 @@
 const TypeSummaryOptions
 &options); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
+bool LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::function<>
+
 SyntheticChildrenFrontEnd *
 LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *,
  lldb::ValueObjectSP);
@@ -124,9 +128,6 @@
 LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
-SyntheticChildrenFrontEnd *LibcxxFunctionFrontEndCreator(CXXSyntheticChildren *,
- lldb::ValueObjectSP);
-
 SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -12,6 +12,7 @@
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "llvm/ADT/ScopeExit.h"
 // Project includes
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
@@ -23,6 +24,7 @@
 #include "lldb/DataFormatters/VectorIterator.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/ProcessStructReader.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +35,225 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+
+  if (!valobj_sp)
+return false;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x7ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x7ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__funcGetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+return false;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+  process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+return false;
+
+  bool found_wrapped_function = false;
+
+  // Using scoped exit so we can use early return and still execute the default
+  // action in case we don't find the wrapper function. Otherwise we can't use
+  // early exit without duplicating code.
+  auto default_print_on_exit = llvm::make_scope_exit(
+  [&found_wrapped_function, &stream, &member__f_pointer_value]() {
+if (!found_wrapped_function)
+  stream.Printf(" __f_ = %llu", member__f_pointer_value);
+  });
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+  process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+return false;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+return false;
+
+  Ad

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham @vsk I believe I have addressed most of your comments




Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58
+
+  if (process != nullptr) {
+Status status;

jingham wrote:
> vsk wrote:
> > Please use an early exit here, and everywhere else in the function where 
> > it's possible to do so. If the process has really disappeared, there's no 
> > need to print anything. In general there's no need to present an incomplete 
> > result from this formatter.
> Data formatters can get called in the absence of a process.  For simple C 
> structs, for instance, they might be used to look at an initialized static 
> object in a DATA section.  For instance, I could do something like:
>  
> #include 
> auto g_func = [](int x, int y) {return x + y; };
> int
> main(int argc, char ** argv)
> {
>   int ret = g_func(argc, 100);
>   return ret;
> }
> 
> and I should be able to see g_func using "target var" before running the 
> program.  I think the only place you care about the process here is whether 
> you should resolve a load address or not.  Target->ReadMemory will read from 
> a running process if that target has one (but preferring data from the 
> underlying files if that's faster.)  So I think this formatter should be able 
> to do its job with or without a process.
> 
> But I agree with Vedant that in cases where you can't get useful information 
> (for instance if you can't get the vtable address), then you should return 
> false immediately and not try to present a summary.  That way instead of 
> showing an incomplete summary, lldb will show the raw printing of the type, 
> which is more useful than a place-holder summary string.
> 
> You already do that for the case where you can't read the process memory.  
> Using early return when you discover an error will make it easier to ensure 
> you are doing that right every time you get an error.
AFAIT we can't derive useful information for the static case.

`std::function` does not have constexpr constructors so it can't be initialized 
during constant initialization. I confirmed in lldb that all the fields are 
uninitialized therefore we have no pointers to examine.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:136-140
+// Given:
+//
+//main::$_1::__invoke(...)
+//
+// We want to slice off __invoke(...) and append operator()()

jingham wrote:
> In the first bit of code, you put the explaining comment before the code that 
> handles the condition.  This time it's in the middle of the code.  Maybe move 
> this comment before "if (symbol_name.contains(...)"
I updated the commenting a big, hopefully it clears things up.


https://reviews.llvm.org/D50864



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

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 161997.
shafik added a comment.

Updating comment


https://reviews.llvm.org/D50864

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h

Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -32,6 +32,10 @@
 const TypeSummaryOptions
 &options); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
+bool LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::function<>
+
 SyntheticChildrenFrontEnd *
 LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *,
  lldb::ValueObjectSP);
@@ -124,9 +128,6 @@
 LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
-SyntheticChildrenFrontEnd *LibcxxFunctionFrontEndCreator(CXXSyntheticChildren *,
- lldb::ValueObjectSP);
-
 SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -12,6 +12,7 @@
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "llvm/ADT/ScopeExit.h"
 // Project includes
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
@@ -23,6 +24,7 @@
 #include "lldb/DataFormatters/VectorIterator.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/ProcessStructReader.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +35,226 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxFunctionSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+
+  if (!valobj_sp)
+return false;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x7ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x7ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__funcGetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+return false;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+  process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+return false;
+
+  bool found_wrapped_function = false;
+
+  // Using scoped exit so we can use early return and still execute the default
+  // action in case we don't find the wrapper function. Otherwise we can't use
+  // early exit without duplicating code.
+  auto default_print_on_exit = llvm::make_scope_exit(
+  [&found_wrapped_function, &stream, &member__f_pointer_value]() {
+if (!found_wrapped_function)
+  stream.Printf(" __f_ = %llu", member__f_pointer_value);
+  });
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+  process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+return false;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+return false;
+
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
+  vtable_add

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: include/lldb/Symbol/ClangASTContext.h:284
+ ((bool)pack_name == (bool)packed_args) &&
+ (!packed_args || !packed_args->args.empty());
 }

Is this saying that an empty parameter pack is invalid? We can have an empty 
parameter pack

https://godbolt.org/z/8zCFz9



Comment at: include/lldb/Symbol/ClangASTContext.h:792
+  GetTemplateArgumentKind(lldb::opaque_compiler_type_t type, size_t idx,
+  bool expand_pack) override;
   CompilerType GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,

Why not default to `false` here?



Comment at: include/lldb/Symbol/ClangASTContext.h:794
   CompilerType GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,
-   size_t idx) override;
+   size_t idx, bool expand_pack) override;
   llvm::Optional

Why not default to `false` here?



Comment at: include/lldb/Symbol/ClangASTContext.h:797
+  GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type, size_t idx,
+  bool expand_pack) override;
 

Why not default to `false` here?



Comment at: source/Symbol/ClangASTContext.cpp:7569
+  if (expand_pack) {
+auto &pack = template_arg_list[num_args - 1];
+if (pack.getKind() == clang::TemplateArgument::Pack)

Do we need to check `num_args != 0`



Comment at: source/Symbol/ClangASTContext.cpp:7664
+
+  assert(args.size() &&
+ "We shouldn't have a template specialization without any args");

The asset before we do math making this assumption i.e. `args.size() - 1`


https://reviews.llvm.org/D51387



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

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, aprantl, davide.

lldb::StateType is an enum without an explicit underlying type.

According to [dcl.enum]p8 http://eel.is/c++draft/dcl.enum#8 and 
[expr.static.cast]p10  http://eel.is/c++draft/expr.static.cast#10 we are 
limited to setting StateType to values bound with a value range based on the 
values of the enumerators. Currently we have a test and SWIG interface that 
either explicitly set StateType to a value outside of the allowed range or in 
the case of SWIG does not prevent it.

In both cases using UBSan generates a runtime error when we load a value 
outside of the range allowed by the standard.

This change refactors StateType to keep track of the largest value and range 
checking to the SWIG interface and removes the invalid test case.


https://reviews.llvm.org/D51445

Files:
  include/lldb/lldb-enumerations.h
  scripts/Python/python-typemaps.swig
  source/Interpreter/CommandObject.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  source/Target/Process.cpp
  source/Utility/State.cpp
  unittests/Utility/StateTest.cpp

Index: unittests/Utility/StateTest.cpp
===
--- unittests/Utility/StateTest.cpp
+++ unittests/Utility/StateTest.cpp
@@ -15,7 +15,18 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", kNumStateType).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: source/Utility/State.cpp
===
--- source/Utility/State.cpp
+++ source/Utility/State.cpp
@@ -18,6 +18,7 @@
 
 const char *lldb_private::StateAsCString(StateType state) {
   switch (state) {
+  case kNumStateType:
   case eStateInvalid:
 return "invalid";
   case eStateUnloaded:
@@ -78,6 +79,7 @@
   case eStateStepping:
 return true;
 
+  case kNumStateType:
   case eStateConnected:
   case eStateDetached:
   case eStateInvalid:
@@ -93,6 +95,7 @@
 
 bool lldb_private::StateIsStoppedState(StateType state, bool must_exist) {
   switch (state) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateConnected:
   case eStateAttaching:
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -837,6 +837,7 @@
 Destroy(false);
 break;
 
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateDetached:
@@ -1054,6 +1055,7 @@
 return false;
 
   switch (event_state) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateAttaching:
@@ -1854,6 +1856,7 @@
 
   bool show_error = true;
   switch (GetState()) {
+  case kNumStateType:
   case eStateInvalid:
   case eStateUnloaded:
   case eStateConnected:
@@ -3582,6 +3585,7 @@
 // always report them.
 return_value = true;
 break;
+  case kNumStateType:
   case eStateInvalid:
 // We stopped for no apparent reason, don't report it.
 return_value = false;
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -776,6 +776,7 @@
   DEBUG_PRINTF("DynamicLoaderDarwin::%s(%s)\n", __FUNCTION__,
StateAsCString(state));
   switch (state) {
+  case kNumStateType:
   case eStateConnected:
   case eStateAttaching:
   case eStateLaunching:
Index: source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -1475,6 +1475,7 @@
   DEBUG_PRINTF("DynamicLoaderDarwinKernel::%s(%s)\n", _

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Very nice! Do we have a way of verifying the performance gain?




Comment at: source/Breakpoint/BreakpointResolver.cpp:183
 llvm::StringRef log_ident) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-
-  while (sc_list.GetSize() > 0) {
-SymbolContextList tmp_sc_list;
-unsigned current_idx = 0;
-SymbolContext sc;
-bool first_entry = true;
-
-FileSpec match_file_spec;
-FileSpec match_original_file_spec;
-uint32_t closest_line_number = UINT32_MAX;
-
-// Pull out the first entry, and all the others that match its file spec,
-// and stuff them in the tmp list.
-while (current_idx < sc_list.GetSize()) {
-  bool matches;
-
-  sc_list.GetContextAtIndex(current_idx, sc);
-  if (first_entry) {
-match_file_spec = sc.line_entry.file;
-match_original_file_spec = sc.line_entry.original_file;
-matches = true;
-first_entry = false;
-  } else
-matches = ((sc.line_entry.file == match_file_spec) ||
-   (sc.line_entry.original_file == match_original_file_spec));
-
-  if (matches) {
-tmp_sc_list.Append(sc);
-sc_list.RemoveContextAtIndex(current_idx);
-
-// ResolveSymbolContext will always return a number that is >= the line
-// number you pass in. So the smaller line number is always better.
-if (sc.line_entry.line < closest_line_number)
-  closest_line_number = sc.line_entry.line;
-  } else
-current_idx++;
+  llvm::SmallVector all_scs;
+  for (unsigned i = 0; i < sc_list.GetSize(); ++i)

The number `16` and `8` below where do they come from?



Comment at: source/Breakpoint/BreakpointResolver.cpp:184
+  llvm::SmallVector all_scs;
+  for (unsigned i = 0; i < sc_list.GetSize(); ++i)
+all_scs.push_back(sc_list[i]);

`GetSize()` returns `uint32_t` we should match the type.



Comment at: source/Breakpoint/BreakpointResolver.cpp:214
+std::sort(worklist_begin, worklist_end,
+  [&](const SymbolContext &a, const SymbolContext &b) {
+return a.line_entry.range.GetBaseAddress().GetFileAddress() <

I don't think you need a capture here.



Comment at: source/Breakpoint/BreakpointResolver.cpp:228
+  std::remove_if(std::next(first), worklist_end,
+ [&](const SymbolContext &sc) {
+   return blocks_with_breakpoints.count(sc.block);

Although in the other lambdas being explicit with the capture list might reduce 
readability here we are only capturing `blocks_with_breakpoints`


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51453



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

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: jingham, davide.
Herald added subscribers: christof, mgorny.
Herald added a reviewer: EricWF.

Adding data formatter for libc++ std::variant and appropriate tests


https://reviews.llvm.org/D51520

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,32 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *
+LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h_
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -0,0 +1,282 @@
+//===-- LibCxxVariant.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxxVariant.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/ScopeExit.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// libc++ variant implementation contains two members that we care about both
+// are contained in the __impl member.
+// - __index which tells us which of the variadic template types is the active
+//   type for the variant
+// - __data is a variadic union which recursively contains itself as member
+//   which refers to the tailing variadic types.
+//   - __head which refers to the leading non pack type
+// - __value refers to the actual value contained
+//   - __tail which refers to the remaining pack types and
+//
+// e.g. given std::variant v1
+//
+// (lldb) frame var -R v1.__impl.__data
+//(... __union<... 0, int, double, char>) v1.__impl.__data = {
+// ...
+//  __head = {
+//__value = ...
+//  }
+//  __tail = {
+//  ...
+//__head = {
+//  __value = ...
+//}
+//__tail = {
+//...
+//  __head = {
+//__value = ...
+//  ...
+//
+// So given
+// - __index equal to 0 the active value is contained in
+//
+// __data.__head.__value
+//
+// - __index equal to 1 the active value is contained in
+//
+// __data.__tail.__head.__value
+//
+// - __index equal to 2 the active value is contained in
+//
+//  __data.__tail.__tail.__head.__value
+//
+
+namespace {
+// libc++ std::variant index could have one of three states
+// 1) VALID, we can obtain it and its not variant_npos
+// 2) INVALID, we can't obtian it or it is not a type we expect
+// 3) NPOS, its value is variant_npos which means the variant has no value
+enum class LibcxxVariantIndexValidity {
+ VALID,
+ INVALID,
+ NPOS
+};
+
+LibcxxVariantIndexValidity LibcxxVariantGetIndexValidity( ValueObjectSP &impl_sp ) {
+ValueObjectSP index_sp(
+   impl_sp->GetChildMemberWithName(ConstString("__index"), true));
+
+if (!index_sp)
+return LibcxxVariantIndexValidity::INVALID ;
+
+uint64_t index_value = index_sp->GetValueAsUnsigned(0) ;
+
+CompilerType index_compiler_type = index_sp->GetCompilerType() ;
+
+// We are expecting to layers of typedefs before we obtain the b

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 3 inline comments as done.
shafik added a comment.

@jingham I am switching to the @aprantl suggestions which feels cleaner and 
removes this issue.




Comment at: include/lldb/lldb-enumerations.h:60
///< or threads get the chance to run.
+  kNumStateType
 };

aprantl wrote:
> I think we usually do something like eLastsState = eStateSuspended. This 
> avoid having to add the case to all the switches.
This approach seems like a much better idiom, I will switch to that.


https://reviews.llvm.org/D51445



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

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 163450.
shafik marked an inline comment as done.
shafik added a comment.

Switching enum guard to kLastStateType which references the last valid enum 
which lead to cleaner code instead of inventing a new value which does not have 
a good meaning in many cases.


https://reviews.llvm.org/D51445

Files:
  include/lldb/lldb-enumerations.h
  scripts/Python/python-typemaps.swig
  unittests/Utility/StateTest.cpp


Index: unittests/Utility/StateTest.cpp
===
--- unittests/Utility/StateTest.cpp
+++ unittests/Utility/StateTest.cpp
@@ -15,7 +15,17 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: scripts/Python/python-typemaps.swig
===
--- scripts/Python/python-typemaps.swig
+++ scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+PythonInteger py_int(PyRefType::Borrowed, $input);
+int64_t state_type_value = py_int.GetInteger() ;
+
+if (state_type_value > lldb::StateType::kLastStateType) {
+  PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
+  return nullptr;
+}
+$1 = static_cast(state_type_value);
+  }
+  else
+  {
+PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+return nullptr;
+  }
+}
+
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
Index: include/lldb/lldb-enumerations.h
===
--- include/lldb/lldb-enumerations.h
+++ include/lldb/lldb-enumerations.h
@@ -54,9 +54,10 @@
   eStateCrashed,   ///< Process or thread has crashed and can be examined.
   eStateDetached,  ///< Process has been detached and can't be examined.
   eStateExited,///< Process has exited and can't be examined.
-  eStateSuspended  ///< Process or thread is in a suspended state as far
+  eStateSuspended, ///< Process or thread is in a suspended state as far
///< as the debugger is concerned while other processes
///< or threads get the chance to run.
+  kLastStateType = eStateSuspended  
 };
 
 //--


Index: unittests/Utility/StateTest.cpp
===
--- unittests/Utility/StateTest.cpp
+++ unittests/Utility/StateTest.cpp
@@ -15,7 +15,17 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: scripts/Python/python-typemaps.swig
===
--- scripts/Python/python-typemaps.swig
+++ scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+PythonInteger py_int(PyRefType::Borrowed, $input);
+int6

[Lldb-commits] [PATCH] D51557: Replace uses of LazyBool with LazyBool template

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: include/lldb/DataFormatters/ValueObjectPrinter.h:146
+
+#define LLDB_VOP ValueObjectPrinter
+  LazyBoolMember m_should_print;

davide wrote:
> can you typedef?
I feel like using ... = is cleaner


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51557



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

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added a reviewer: jingham.

Moving the core functionality of std::function formatter into 
CPPLanguageRuntime in preparation for future changes to allow us to step into 
the wrapped callable of std::function.

This will prevent code duplication since both functionalities require the same 
core pieces of information. This refactor also improves the readability of the 
existing functionality.


https://reviews.llvm.org/D51896

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -14,9 +14,20 @@
 
 #include "llvm/ADT/StringRef.h"
 
+#include "lldb/API/SBValue.h"
+#include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/VariableList.h"
+
+#include "lldb/API/SBFrame.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/SectionLoadList.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/ThreadPlanRunToAddress.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -40,3 +51,216 @@
   // C++ has no generic way to do this.
   return false;
 }
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
+lldb::ValueObjectSP &valobj_sp) {
+  LibCppStdFunctionCallableInfo optional_info;
+
+  if (!valobj_sp)
+return optional_info;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x7ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x7ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__funcGetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  optional_info.member__f_pointer_value = member__f_pointer_value;
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+return optional_info;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+  process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+return optional_info;
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+  process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+return optional_info;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+return optional_info;
+
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
+  vtable_addr_resolved))
+return optional_info;
+
+  target.GetImages().ResolveSymbolContextForAddress(
+  vtable_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return optional_info;
+
+  llvm::StringRef vtable_name(symbol->GetName().GetCString());
+  bool found_expected_start_string =
+  vtable_name.startswith("vtable for std::__1::__function::__func<");
+
+  if (!found_expected_start_string)
+return optional_info;
+
+  // Given case 1 or 3 we have a vtable name, we are want to extract the first
+  // template parameter
+  //
+  //  ... __func ...
+  // ^
+  //
+  // We do this by find the first < and , and extracting in between.
+  //
+  // This covers the case of the lambda known at compile time.
+  //
+  size_t first_open_angle_bracket = 

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341957: Remove undefined behavior around the use of 
StateType (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51445?vs=163450&id=164905#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51445

Files:
  lldb/trunk/include/lldb/lldb-enumerations.h
  lldb/trunk/scripts/Python/python-typemaps.swig
  lldb/trunk/unittests/Utility/StateTest.cpp


Index: lldb/trunk/unittests/Utility/StateTest.cpp
===
--- lldb/trunk/unittests/Utility/StateTest.cpp
+++ lldb/trunk/unittests/Utility/StateTest.cpp
@@ -15,7 +15,17 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: lldb/trunk/scripts/Python/python-typemaps.swig
===
--- lldb/trunk/scripts/Python/python-typemaps.swig
+++ lldb/trunk/scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+PythonInteger py_int(PyRefType::Borrowed, $input);
+int64_t state_type_value = py_int.GetInteger() ;
+
+if (state_type_value > lldb::StateType::kLastStateType) {
+  PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
+  return nullptr;
+}
+$1 = static_cast(state_type_value);
+  }
+  else
+  {
+PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+return nullptr;
+  }
+}
+
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
Index: lldb/trunk/include/lldb/lldb-enumerations.h
===
--- lldb/trunk/include/lldb/lldb-enumerations.h
+++ lldb/trunk/include/lldb/lldb-enumerations.h
@@ -54,9 +54,10 @@
   eStateCrashed,   ///< Process or thread has crashed and can be examined.
   eStateDetached,  ///< Process has been detached and can't be examined.
   eStateExited,///< Process has exited and can't be examined.
-  eStateSuspended  ///< Process or thread is in a suspended state as far
+  eStateSuspended, ///< Process or thread is in a suspended state as far
///< as the debugger is concerned while other processes
///< or threads get the chance to run.
+  kLastStateType = eStateSuspended  
 };
 
 //--


Index: lldb/trunk/unittests/Utility/StateTest.cpp
===
--- lldb/trunk/unittests/Utility/StateTest.cpp
+++ lldb/trunk/unittests/Utility/StateTest.cpp
@@ -15,7 +15,17 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }
Index: lldb/trunk/scripts/Python/python-typemaps.swig
===
--- lldb/trunk/scripts/Python/python-typemaps.swig
+++ lldb/trun

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB341957: Remove undefined behavior around the use of 
StateType (authored by shafik, committed by ).
Herald added a subscriber: abidh.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51445

Files:
  include/lldb/lldb-enumerations.h
  scripts/Python/python-typemaps.swig
  unittests/Utility/StateTest.cpp


Index: scripts/Python/python-typemaps.swig
===
--- scripts/Python/python-typemaps.swig
+++ scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+PythonInteger py_int(PyRefType::Borrowed, $input);
+int64_t state_type_value = py_int.GetInteger() ;
+
+if (state_type_value > lldb::StateType::kLastStateType) {
+  PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
+  return nullptr;
+}
+$1 = static_cast(state_type_value);
+  }
+  else
+  {
+PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+return nullptr;
+  }
+}
+
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
Index: include/lldb/lldb-enumerations.h
===
--- include/lldb/lldb-enumerations.h
+++ include/lldb/lldb-enumerations.h
@@ -54,9 +54,10 @@
   eStateCrashed,   ///< Process or thread has crashed and can be examined.
   eStateDetached,  ///< Process has been detached and can't be examined.
   eStateExited,///< Process has exited and can't be examined.
-  eStateSuspended  ///< Process or thread is in a suspended state as far
+  eStateSuspended, ///< Process or thread is in a suspended state as far
///< as the debugger is concerned while other processes
///< or threads get the chance to run.
+  kLastStateType = eStateSuspended  
 };
 
 //--
Index: unittests/Utility/StateTest.cpp
===
--- unittests/Utility/StateTest.cpp
+++ unittests/Utility/StateTest.cpp
@@ -15,7 +15,17 @@
 using namespace lldb_private;
 
 TEST(StateTest, Formatv) {
-  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("invalid", llvm::formatv("{0}", eStateInvalid).str());
+  EXPECT_EQ("unloaded", llvm::formatv("{0}", eStateUnloaded).str());
+  EXPECT_EQ("connected", llvm::formatv("{0}", eStateConnected).str());
+  EXPECT_EQ("attaching", llvm::formatv("{0}", eStateAttaching).str());
+  EXPECT_EQ("launching", llvm::formatv("{0}", eStateLaunching).str());
   EXPECT_EQ("stopped", llvm::formatv("{0}", eStateStopped).str());
-  EXPECT_EQ("unknown", llvm::formatv("{0}", StateType(-1)).str());
+  EXPECT_EQ("running", llvm::formatv("{0}", eStateRunning).str());
+  EXPECT_EQ("stepping", llvm::formatv("{0}", eStateStepping).str());
+  EXPECT_EQ("crashed", llvm::formatv("{0}", eStateCrashed).str());
+  EXPECT_EQ("detached", llvm::formatv("{0}", eStateDetached).str());
+  EXPECT_EQ("exited", llvm::formatv("{0}", eStateExited).str());
+  EXPECT_EQ("suspended", llvm::formatv("{0}", eStateSuspended).str());
+  
 }


Index: scripts/Python/python-typemaps.swig
===
--- scripts/Python/python-typemaps.swig
+++ scripts/Python/python-typemaps.swig
@@ -77,6 +77,26 @@
   }
 }
 
+%typemap(in) lldb::StateType {
+  using namespace lldb_private;
+  if (PythonInteger::Check($input))
+  {
+PythonInteger py_int(PyRefType::Borrowed, $input);
+int64_t state_type_value = py_int.GetInteger() ;
+
+if (state_type_value > lldb::StateType::kLastStateType) {
+  PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
+  return nullptr;
+}
+$1 = static_cast(state_type_value);
+  }
+  else
+  {
+PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+return nullptr;
+  }
+}
+
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
Index: include/lldb/lldb-enumerations.h
===
--- include/lldb/lldb-enumerations.h
+++ include/lldb/lldb-enumerations.h
@@ -54,9 +54,10 @@
   eStateCrashed,   ///< Process or thread has crashed and can be examined.
   eStateDetached,  ///< Process has been detached and can't be examined.
   eStateExited,///< Process has exited and can't be examined.
-  eStateSuspended  ///< Process or thread is in a suspended state as far
+  eStateSuspended, ///< Process or thread is in a suspended state as far
///< as the debugger is concerned while other processes
///< or threads get the chance to run.
+  kLastStateType = eStateSuspended  
 };
 
 //

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 164962.
shafik added a comment.

Removing change that was meant for the next PR


https://reviews.llvm.org/D51896

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Target/CPPLanguageRuntime.cpp

Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -14,9 +14,20 @@
 
 #include "llvm/ADT/StringRef.h"
 
+#include "lldb/API/SBValue.h"
+#include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/VariableList.h"
+
+#include "lldb/API/SBFrame.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/SectionLoadList.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/ThreadPlanRunToAddress.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -40,3 +51,216 @@
   // C++ has no generic way to do this.
   return false;
 }
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
+lldb::ValueObjectSP &valobj_sp) {
+  LibCppStdFunctionCallableInfo optional_info;
+
+  if (!valobj_sp)
+return optional_info;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x7ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x7ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__funcGetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  optional_info.member__f_pointer_value = member__f_pointer_value;
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+return optional_info;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+  process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+return optional_info;
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+  process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+return optional_info;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+return optional_info;
+
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
+  vtable_addr_resolved))
+return optional_info;
+
+  target.GetImages().ResolveSymbolContextForAddress(
+  vtable_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return optional_info;
+
+  llvm::StringRef vtable_name(symbol->GetName().GetCString());
+  bool found_expected_start_string =
+  vtable_name.startswith("vtable for std::__1::__function::__func<");
+
+  if (!found_expected_start_string)
+return optional_info;
+
+  // Given case 1 or 3 we have a vtable name, we are want to extract the first
+  // template parameter
+  //
+  //  ... __func ...
+  // ^
+  //
+  // We do this by find the first < and , and extracting in between.
+  //
+  // This covers the case of the lambda known at compile time.
+  //
+  size_t first_open_angle_bracket = vtable_name.find('<') + 1;
+  size_t first_comma = vtable_name.find_first_of(',');
+
+  llvm::StringRef first_template_parameter =
+  vtable_name.slice(first_open_angle_bracket, first_comma);
+
+  Address function_address_resolved;
+
+  // Setup for cases 2, 4 and 5 we have a pointer to a function after the
+  // vtable. We will use a process of elimination to drop through each case
+  // and obtain the data we need.
+  if (target.GetSectionLoadList().ResolveLoadAddress(
+  possible_function_address, function_address_resolved)) {
+target.GetImages().ResolveSymbolContextForAddress(
+function_address_resolved, eSymbolContextEverything, sc);
+symbol = sc.symbol;
+  }
+
+  auto get_name = [&first_template_parameter, &symbol]() {
+// Given case 1:
+//
+//main::$_0
+//
+// we want to append ::op

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341991: Refactoring std::function formatter to move core 
functionality into… (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51896?vs=164962&id=164968#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51896

Files:
  lldb/trunk/include/lldb/Target/CPPLanguageRuntime.h
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/trunk/source/Target/CPPLanguageRuntime.cpp

Index: lldb/trunk/source/Target/CPPLanguageRuntime.cpp
===
--- lldb/trunk/source/Target/CPPLanguageRuntime.cpp
+++ lldb/trunk/source/Target/CPPLanguageRuntime.cpp
@@ -14,9 +14,20 @@
 
 #include "llvm/ADT/StringRef.h"
 
+#include "lldb/API/SBValue.h"
+#include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/VariableList.h"
+
+#include "lldb/API/SBFrame.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/SectionLoadList.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/ThreadPlanRunToAddress.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -40,3 +51,216 @@
   // C++ has no generic way to do this.
   return false;
 }
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
+lldb::ValueObjectSP &valobj_sp) {
+  LibCppStdFunctionCallableInfo optional_info;
+
+  if (!valobj_sp)
+return optional_info;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x7ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x7ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__funcGetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  optional_info.member__f_pointer_value = member__f_pointer_value;
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+return optional_info;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+  process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+return optional_info;
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+  process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+return optional_info;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+return optional_info;
+
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
+  vtable_addr_resolved))
+return optional_info;
+
+  target.GetImages().ResolveSymbolContextForAddress(
+  vtable_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return optional_info;
+
+  llvm::StringRef vtable_name(symbol->GetName().GetCString());
+  bool found_expected_start_string =
+  vtable_name.startswith("vtable for std::__1::__function::__func<");
+
+  if (!found_expected_start_string)
+return optional_info;
+
+  // Given case 1 or 3 we have a vtable name, we are want to extract the first
+  // template parameter
+  //
+  //  ... __func ...
+  // ^
+  //
+  // We do this by find the first < and , and extracting in between.
+  //
+  // This covers the case of the lambda known at compile time.
+  //
+  size_t first_open_angle_bracket = vtable_name.find('<') + 1;
+  size_t first_comma = vtable_name.find_first_of(',');
+
+  llvm::StringRef first_template_parameter =
+  vtable_name.slice(first_open_angle_bracket, first_comma);
+
+  Address function_address_resolved;
+
+  // Setup for cases 2, 4 and 5 we have a pointer to a function after the
+  // vtable. We will use a process of elimination to drop through each case
+  // and obtain the data we need.
+  if (target.GetSectionLoadList().ResolveLoadAddress(
+  possible_function_address, function_address_resolved)) {

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 165201.
shafik marked 9 inline comments as done.
shafik added a comment.

Address comments on std::variant formatter

- Adding tests for reference to variant and variant of variant
- Refactoring test to be more efficient
- Refactoring unsafe code that assumed sizes of types between local machine and 
target machine were the same
  - Used to check to variant_npos which is max unsigned or -1


https://reviews.llvm.org/D51520

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,32 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *
+LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h_
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -0,0 +1,274 @@
+//===-- LibCxxVariant.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxxVariant.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/ScopeExit.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// libc++ variant implementation contains two members that we care about both
+// are contained in the __impl member.
+// - __index which tells us which of the variadic template types is the active
+//   type for the variant
+// - __data is a variadic union which recursively contains itself as member
+//   which refers to the tailing variadic types.
+//   - __head which refers to the leading non pack type
+// - __value refers to the actual value contained
+//   - __tail which refers to the remaining pack types
+//
+// e.g. given std::variant v1
+//
+// (lldb) frame var -R v1.__impl.__data
+//(... __union<... 0, int, double, char>) v1.__impl.__data = {
+// ...
+//  __head = {
+//__value = ...
+//  }
+//  __tail = {
+//  ...
+//__head = {
+//  __value = ...
+//}
+//__tail = {
+//...
+//  __head = {
+//__value = ...
+//  ...
+//
+// So given
+// - __index equal to 0 the active value is contained in
+//
+// __data.__head.__value
+//
+// - __index equal to 1 the active value is contained in
+//
+// __data.__tail.__head.__value
+//
+// - __index equal to 2 the active value is contained in
+//
+//  __data.__tail.__tail.__head.__value
+//
+
+namespace {
+// libc++ std::variant index could have one of three states
+// 1) VALID, we can obtain it and its not variant_npos
+// 2) INVALID, we can't obtain it or it is not a type we expect
+// 3) NPOS, its value is variant_npos which means the variant has no value
+enum class LibcxxVariantIndexValidity {
+ VALID,
+ INVALID,
+ NPOS
+};
+
+LibcxxVariantIndexValidity LibcxxVariantGetIndexValidity( ValueObjectSP &impl_sp ) {
+ValueObjectSP index_sp(
+   impl_sp->GetChildMemberWithName(ConstString("__index"), true));
+
+if (!index_sp)
+return LibcxxVariantIndexValidity::INVALID;

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@vsk @jingham I believe I have addressed your comments, please review again.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp:29
+std::variant v3;
+std::variant v_no_value;
+

vsk wrote:
> Does a std::variant containing a std::variant work?
Yes, adding test.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp:97-105
+if ( index_basic_type == eBasicTypeUnsignedInt ) {
+if( index_value == static_cast(-1))
+return LibcxxVariantIndexValidity::NPOS ;
+} else if ( index_basic_type == eBasicTypeUnsignedLong ) {
+if( index_value == static_cast(-1))
+return LibcxxVariantIndexValidity::NPOS ;
+} else if ( index_basic_type == eBasicTypeUnsignedLongLong ) {

jingham wrote:
> I don't think this comparison is a safe thing to do.  For instance, you are 
> comparing the unsigned value that you get from the target (index_value) with 
> lldb's host "unsigned int" value.  If the target has a 32 bit unsigned int, 
> but the host has a 64 bit unsigned int (or vice versa) then the two values 
> won't be the same.  
I believe I applied the change as we discussed earlier, let me know if not.


https://reviews.llvm.org/D51520



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

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@vsk Interesting I ran git clang-format before generating the diff and it made 
changes, so I am not sure what happened


https://reviews.llvm.org/D51520



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

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Commands/CommandObjectTarget.cpp:153
+static OptionDefinition g_dependents_options[1] = {
+{LLDB_OPT_SET_1, false, "no-dependents", 'd',
+ OptionParser::eOptionalArgument, nullptr, g_dependents_enumaration, 0,

Should "no-dependents" be "default"?



Comment at: source/Commands/CommandObjectTarget.cpp:181
+const int short_option = g_dependents_options[option_idx].short_option;
+switch (short_option) {
+case 'd': {

Wouldn't an if/else make more sense here? I had to reread the description up 
top before this made sense that `option_value` is doing the work here.


https://reviews.llvm.org/D51934



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

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 165536.
shafik added a comment.

Applying clang-format


https://reviews.llvm.org/D51520

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,31 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h_
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -0,0 +1,273 @@
+//===-- LibCxxVariant.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxxVariant.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/ScopeExit.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// libc++ variant implementation contains two members that we care about both
+// are contained in the __impl member.
+// - __index which tells us which of the variadic template types is the active
+//   type for the variant
+// - __data is a variadic union which recursively contains itself as member
+//   which refers to the tailing variadic types.
+//   - __head which refers to the leading non pack type
+// - __value refers to the actual value contained
+//   - __tail which refers to the remaining pack types
+//
+// e.g. given std::variant v1
+//
+// (lldb) frame var -R v1.__impl.__data
+//(... __union<... 0, int, double, char>) v1.__impl.__data = {
+// ...
+//  __head = {
+//__value = ...
+//  }
+//  __tail = {
+//  ...
+//__head = {
+//  __value = ...
+//}
+//__tail = {
+//...
+//  __head = {
+//__value = ...
+//  ...
+//
+// So given
+// - __index equal to 0 the active value is contained in
+//
+// __data.__head.__value
+//
+// - __index equal to 1 the active value is contained in
+//
+// __data.__tail.__head.__value
+//
+// - __index equal to 2 the active value is contained in
+//
+//  __data.__tail.__tail.__head.__value
+//
+
+namespace {
+// libc++ std::variant index could have one of three states
+// 1) VALID, we can obtain it and its not variant_npos
+// 2) INVALID, we can't obtain it or it is not a type we expect
+// 3) NPOS, its value is variant_npos which means the variant has no value
+enum class LibcxxVariantIndexValidity { VALID, INVALID, NPOS };
+
+LibcxxVariantIndexValidity
+LibcxxVariantGetIndexValidity(ValueObjectSP &impl_sp) {
+  ValueObjectSP index_sp(
+  impl_sp->GetChildMemberWithName(ConstString("__index"), true));
+
+  if (!index_sp)
+return LibcxxVariantIndexValidity::INVALID;
+
+  int64_t index_value = index_sp->GetValueAsSigned(0);
+
+  CompilerType index_compiler_type = index_sp->GetCompilerType();
+
+  // We are expecting two layers of typedefs before we obtain the basic type
+  // The next two checks verify this.
+  if (!index_compiler_type.IsTypedefType())
+return LibcxxVariantIndexValidity::INVALID;
+
+  if (!index_c

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@teemperor Thanks for the catch!


https://reviews.llvm.org/D51520



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

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 165540.
shafik added a comment.

Applying clang-format to files I missed earlier


https://reviews.llvm.org/D51520

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,31 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h_
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -0,0 +1,273 @@
+//===-- LibCxxVariant.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxxVariant.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/ScopeExit.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// libc++ variant implementation contains two members that we care about both
+// are contained in the __impl member.
+// - __index which tells us which of the variadic template types is the active
+//   type for the variant
+// - __data is a variadic union which recursively contains itself as member
+//   which refers to the tailing variadic types.
+//   - __head which refers to the leading non pack type
+// - __value refers to the actual value contained
+//   - __tail which refers to the remaining pack types
+//
+// e.g. given std::variant v1
+//
+// (lldb) frame var -R v1.__impl.__data
+//(... __union<... 0, int, double, char>) v1.__impl.__data = {
+// ...
+//  __head = {
+//__value = ...
+//  }
+//  __tail = {
+//  ...
+//__head = {
+//  __value = ...
+//}
+//__tail = {
+//...
+//  __head = {
+//__value = ...
+//  ...
+//
+// So given
+// - __index equal to 0 the active value is contained in
+//
+// __data.__head.__value
+//
+// - __index equal to 1 the active value is contained in
+//
+// __data.__tail.__head.__value
+//
+// - __index equal to 2 the active value is contained in
+//
+//  __data.__tail.__tail.__head.__value
+//
+
+namespace {
+// libc++ std::variant index could have one of three states
+// 1) VALID, we can obtain it and its not variant_npos
+// 2) INVALID, we can't obtain it or it is not a type we expect
+// 3) NPOS, its value is variant_npos which means the variant has no value
+enum class LibcxxVariantIndexValidity { VALID, INVALID, NPOS };
+
+LibcxxVariantIndexValidity
+LibcxxVariantGetIndexValidity(ValueObjectSP &impl_sp) {
+  ValueObjectSP index_sp(
+  impl_sp->GetChildMemberWithName(ConstString("__index"), true));
+
+  if (!index_sp)
+return LibcxxVariantIndexValidity::INVALID;
+
+  int64_t index_value = index_sp->GetValueAsSigned(0);
+
+  CompilerType index_compiler_type = index_sp->GetCompilerType();
+
+  // We are expecting two layers of typedefs before we obtain the basic type
+  // The next two checks verify this.
+  if (!index_compiler_type.IsTypedefType())
+return LibcxxVariantIndexValidity::INVALID;
+
+  if (!index_compiler_type.GetTyped

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342421: [DataFormatters] Add formatter for C++17 
std::variant (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51520?vs=165540&id=165840#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51520

Files:
  lldb/trunk/lldb.xcodeproj/project.pbxproj
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -39,6 +39,7 @@
 #include "CxxStringTypes.h"
 #include "LibCxx.h"
 #include "LibCxxAtomic.h"
+#include "LibCxxVariant.h"
 #include "LibStdcpp.h"
 
 using namespace lldb;
@@ -516,6 +517,10 @@
   "libc++ std::optional synthetic children",
   ConstString("^std::__(ndk)?1::optional<.+>(( )?&)?$"),
   stl_synth_flags, true);
+  AddCXXSynthetic(cpp_category_sp, LibcxxVariantFrontEndCreator,
+  "libc++ std::variant synthetic children",
+  ConstString("^std::__(ndk)?1::variant<.+>(( )?&)?$"),
+  stl_synth_flags, true);
   AddCXXSynthetic(
   cpp_category_sp,
   lldb_private::formatters::LibcxxAtomicSyntheticFrontEndCreator,
@@ -617,6 +622,11 @@
 "libc++ std::optional summary provider",
 ConstString("^std::__(ndk)?1::optional<.+>(( )?&)?$"),
 stl_summary_flags, true);
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxVariantSummaryProvider,
+"libc++ std::variant summary provider",
+ConstString("^std::__(ndk)?1::variant<.+>(( )?&)?$"),
+stl_summary_flags, true);
 
   stl_summary_flags.SetSkipPointers(true);
 
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -142,6 +142,10 @@
 LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP valobj_sp);
 
+SyntheticChildrenFrontEnd *
+LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
@@ -13,6 +13,7 @@
   LibCxxQueue.cpp
   LibCxxTuple.cpp
   LibCxxUnorderedMap.cpp
+  LibCxxVariant.cpp
   LibCxxVector.cpp
   LibStdcpp.cpp
   LibStdcppTuple.cpp
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.h
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,31 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added a reviewer: jingham.

SBFrame should be a thin wrapper around the core functionality in StackFrame. 
Currently FindVariable() core functionality is implemented in SBFrame and this 
will move that into StackFrame.

This is step two in enabling stepping into std::function.


https://reviews.llvm.org/D52247

Files:
  include/lldb/Target/StackFrame.h
  source/API/SBFrame.cpp
  source/Target/StackFrame.cpp


Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
 GetFrameCodeAddress());
 }
 
+lldb::ValueObjectSP StackFrame::FindVariable(const char *name) {
+  ValueObjectSP value_sp;
+
+  if (name == nullptr || name[0] == '\0')
+return value_sp;
+
+  TargetSP target_sp = CalculateTarget();
+  ProcessSP process_sp = CalculateProcess();
+
+  if (!target_sp && !process_sp)
+return value_sp;
+
+  VariableList variable_list;
+  VariableSP var_sp;
+  SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+  if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, 
stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(ConstString(name));
+}
+
+if (var_sp)
+  value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+  }
+
+  return value_sp;
+}
+
 TargetSP StackFrame::CalculateTarget() {
   TargetSP target_sp;
   ThreadSP thread_sp(GetThread());
Index: source/API/SBFrame.cpp
===
--- source/API/SBFrame.cpp
+++ source/API/SBFrame.cpp
@@ -666,28 +666,10 @@
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
-  const bool can_create = true;
-  const bool get_parent_variables = true;
-  const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(name);
 
-  if (sc.block->AppendVariables(
-  can_create, get_parent_variables,
-  stop_if_block_is_inlined_function,
-  [frame](Variable *v) { return v->IsInScope(frame); },
-  &variable_list)) {
-var_sp = variable_list.FindVariable(ConstString(name));
-  }
-}
-
-if (var_sp) {
-  value_sp =
-  frame->GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+if (value_sp)
   sb_value.SetSP(value_sp, use_dynamic);
-}
   } else {
 if (log)
   log->Printf("SBFrame::FindVariable () => error: could not "
Index: include/lldb/Target/StackFrame.h
===
--- include/lldb/Target/StackFrame.h
+++ include/lldb/Target/StackFrame.h
@@ -503,6 +503,8 @@
   lldb::ValueObjectSP GuessValueForRegisterAndOffset(ConstString reg,
  int64_t offset);
 
+  lldb::ValueObjectSP FindVariable(const char *name);
+
   //--
   // lldb::ExecutionContextScope pure virtual functions
   //--


Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
 GetFrameCodeAddress());
 }
 
+lldb::ValueObjectSP StackFrame::FindVariable(const char *name) {
+  ValueObjectSP value_sp;
+
+  if (name == nullptr || name[0] == '\0')
+return value_sp;
+
+  TargetSP target_sp = CalculateTarget();
+  ProcessSP process_sp = CalculateProcess();
+
+  if (!target_sp && !process_sp)
+return value_sp;
+
+  VariableList variable_list;
+  VariableSP var_sp;
+  SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+  if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(ConstString(name));
+}
+
+if (var_sp)
+  value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+  }
+
+  return value_sp;
+}
+
 TargetSP StackFrame::CalculateTarget() {
   TargetSP 

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 166144.
shafik added a comment.

Updating LibcxxVariantGetIndexValidity() to no longer do type check of __index. 
It was left over from the old method of checking for an empty variant and was 
also breaking clang 5.


https://reviews.llvm.org/D51520

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.h

Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.h
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.h
@@ -0,0 +1,31 @@
+//===-- LibCxxVariant.h ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_LibCxxVariant_h_
+#define liblldb_LibCxxVariant_h_
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/DataFormatters/TypeSynthetic.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+bool LibcxxVariantSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::variant<>
+
+SyntheticChildrenFrontEnd *LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
+lldb::ValueObjectSP);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // liblldb_LibCxxVariant_h_
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -0,0 +1,256 @@
+//===-- LibCxxVariant.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxxVariant.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/ScopeExit.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// libc++ variant implementation contains two members that we care about both
+// are contained in the __impl member.
+// - __index which tells us which of the variadic template types is the active
+//   type for the variant
+// - __data is a variadic union which recursively contains itself as member
+//   which refers to the tailing variadic types.
+//   - __head which refers to the leading non pack type
+// - __value refers to the actual value contained
+//   - __tail which refers to the remaining pack types
+//
+// e.g. given std::variant v1
+//
+// (lldb) frame var -R v1.__impl.__data
+//(... __union<... 0, int, double, char>) v1.__impl.__data = {
+// ...
+//  __head = {
+//__value = ...
+//  }
+//  __tail = {
+//  ...
+//__head = {
+//  __value = ...
+//}
+//__tail = {
+//...
+//  __head = {
+//__value = ...
+//  ...
+//
+// So given
+// - __index equal to 0 the active value is contained in
+//
+// __data.__head.__value
+//
+// - __index equal to 1 the active value is contained in
+//
+// __data.__tail.__head.__value
+//
+// - __index equal to 2 the active value is contained in
+//
+//  __data.__tail.__tail.__head.__value
+//
+
+namespace {
+// libc++ std::variant index could have one of three states
+// 1) VALID, we can obtain it and its not variant_npos
+// 2) INVALID, we can't obtain it or it is not a type we expect
+// 3) NPOS, its value is variant_npos which means the variant has no value
+enum class LibcxxVariantIndexValidity { VALID, INVALID, NPOS };
+
+LibcxxVariantIndexValidity
+LibcxxVariantGetIndexValidity(ValueObjectSP &impl_sp) {
+  ValueObjectSP index_sp(
+  impl_sp->GetChildMemberWithName(ConstString("__index"), true));
+
+  if (!index_sp)
+return LibcxxVariantIndexValidity::INVALID;
+
+  int64_t index_value = index_sp->GetValueAsSigned(0);
+
+  if (index_value == -1)
+return LibcxxVariantIndexValidity::NPOS;
+
+  return LibcxxVariantIndexValidity::VALID;
+}
+
+llvm::Optional LibcxxVariantIndexValue(ValueObjectSP &impl

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 166194.
shafik marked 6 inline comments as done.
shafik added a comment.

Addressing comments:

  
  - Adding documentation to FindVariable()
  - Using ConstString instead of const char *


https://reviews.llvm.org/D52247

Files:
  include/lldb/Target/StackFrame.h
  source/API/SBFrame.cpp
  source/Target/StackFrame.cpp

Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
 GetFrameCodeAddress());
 }
 
+lldb::ValueObjectSP StackFrame::FindVariable(ConstString name) {
+  ValueObjectSP value_sp;
+
+  if (!name)
+return value_sp;
+
+  TargetSP target_sp = CalculateTarget();
+  ProcessSP process_sp = CalculateProcess();
+
+  if (!target_sp && !process_sp)
+return value_sp;
+
+  VariableList variable_list;
+  VariableSP var_sp;
+  SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+  if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(name);
+}
+
+if (var_sp)
+  value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+  }
+
+  return value_sp;
+}
+
 TargetSP StackFrame::CalculateTarget() {
   TargetSP target_sp;
   ThreadSP thread_sp(GetThread());
Index: source/API/SBFrame.cpp
===
--- source/API/SBFrame.cpp
+++ source/API/SBFrame.cpp
@@ -666,28 +666,10 @@
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
-  const bool can_create = true;
-  const bool get_parent_variables = true;
-  const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(ConstString(name));
 
-  if (sc.block->AppendVariables(
-  can_create, get_parent_variables,
-  stop_if_block_is_inlined_function,
-  [frame](Variable *v) { return v->IsInScope(frame); },
-  &variable_list)) {
-var_sp = variable_list.FindVariable(ConstString(name));
-  }
-}
-
-if (var_sp) {
-  value_sp =
-  frame->GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+if (value_sp)
   sb_value.SetSP(value_sp, use_dynamic);
-}
   } else {
 if (log)
   log->Printf("SBFrame::FindVariable () => error: could not "
Index: include/lldb/Target/StackFrame.h
===
--- include/lldb/Target/StackFrame.h
+++ include/lldb/Target/StackFrame.h
@@ -503,6 +503,18 @@
   lldb::ValueObjectSP GuessValueForRegisterAndOffset(ConstString reg,
  int64_t offset);
 
+  //--
+  /// Attempt to reconstruct the ValueObject for a variable with a given \a name
+  /// from within the current StackFrame, within the current block.
+  ///
+  /// @params [in] name
+  ///   The name of the variable.
+  ///
+  /// @return
+  ///   The ValueObject if found.
+  //--
+  lldb::ValueObjectSP FindVariable(ConstString name);
+
   //--
   // lldb::ExecutionContextScope pure virtual functions
   //--
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham @clayborg @davide addressed comments w/ the exception of the lambda 
one which I politely disagree with.




Comment at: source/Target/StackFrame.cpp:1733-1738
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, 
stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(ConstString(name));
+}

davide wrote:
> This is fairly unreadable IMHO. If I were you, I would hoist the lambda out.
This is exactly the case lambda were meant to address, moving it out would just 
add boilerplate code :-(


https://reviews.llvm.org/D52247



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

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 166308.
shafik added a comment.

Adjusting documentation based on feedback


https://reviews.llvm.org/D52247

Files:
  include/lldb/Target/StackFrame.h
  source/API/SBFrame.cpp
  source/Target/StackFrame.cpp

Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
 GetFrameCodeAddress());
 }
 
+lldb::ValueObjectSP StackFrame::FindVariable(ConstString name) {
+  ValueObjectSP value_sp;
+
+  if (!name)
+return value_sp;
+
+  TargetSP target_sp = CalculateTarget();
+  ProcessSP process_sp = CalculateProcess();
+
+  if (!target_sp && !process_sp)
+return value_sp;
+
+  VariableList variable_list;
+  VariableSP var_sp;
+  SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+  if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(name);
+}
+
+if (var_sp)
+  value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+  }
+
+  return value_sp;
+}
+
 TargetSP StackFrame::CalculateTarget() {
   TargetSP target_sp;
   ThreadSP thread_sp(GetThread());
Index: source/API/SBFrame.cpp
===
--- source/API/SBFrame.cpp
+++ source/API/SBFrame.cpp
@@ -666,28 +666,10 @@
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
-  const bool can_create = true;
-  const bool get_parent_variables = true;
-  const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(ConstString(name));
 
-  if (sc.block->AppendVariables(
-  can_create, get_parent_variables,
-  stop_if_block_is_inlined_function,
-  [frame](Variable *v) { return v->IsInScope(frame); },
-  &variable_list)) {
-var_sp = variable_list.FindVariable(ConstString(name));
-  }
-}
-
-if (var_sp) {
-  value_sp =
-  frame->GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+if (value_sp)
   sb_value.SetSP(value_sp, use_dynamic);
-}
   } else {
 if (log)
   log->Printf("SBFrame::FindVariable () => error: could not "
Index: include/lldb/Target/StackFrame.h
===
--- include/lldb/Target/StackFrame.h
+++ include/lldb/Target/StackFrame.h
@@ -503,6 +503,21 @@
   lldb::ValueObjectSP GuessValueForRegisterAndOffset(ConstString reg,
  int64_t offset);
 
+  //--
+  /// Attempt to reconstruct the ValueObject for a variable with a given \a name
+  /// from within the current StackFrame, within the current block. The search
+  /// for the variable starts in the deepest block corresponding to the current
+  /// PC in the stack frame and traverse through all parent blocks stopping at
+  /// inlined function boundaries.
+  ///
+  /// @params [in] name
+  ///   The name of the variable.
+  ///
+  /// @return
+  ///   The ValueObject if found.
+  //--
+  lldb::ValueObjectSP FindVariable(ConstString name);
+
   //--
   // lldb::ExecutionContextScope pure virtual functions
   //--
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342663: Refactor FindVariable() core functionality into 
StackFrame out of SBFrame (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52247?vs=166308&id=166322#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52247

Files:
  lldb/trunk/include/lldb/Target/StackFrame.h
  lldb/trunk/source/API/SBFrame.cpp
  lldb/trunk/source/Target/StackFrame.cpp

Index: lldb/trunk/source/Target/StackFrame.cpp
===
--- lldb/trunk/source/Target/StackFrame.cpp
+++ lldb/trunk/source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
 GetFrameCodeAddress());
 }
 
+lldb::ValueObjectSP StackFrame::FindVariable(ConstString name) {
+  ValueObjectSP value_sp;
+
+  if (!name)
+return value_sp;
+
+  TargetSP target_sp = CalculateTarget();
+  ProcessSP process_sp = CalculateProcess();
+
+  if (!target_sp && !process_sp)
+return value_sp;
+
+  VariableList variable_list;
+  VariableSP var_sp;
+  SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+  if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(name);
+}
+
+if (var_sp)
+  value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+  }
+
+  return value_sp;
+}
+
 TargetSP StackFrame::CalculateTarget() {
   TargetSP target_sp;
   ThreadSP thread_sp(GetThread());
Index: lldb/trunk/source/API/SBFrame.cpp
===
--- lldb/trunk/source/API/SBFrame.cpp
+++ lldb/trunk/source/API/SBFrame.cpp
@@ -666,28 +666,10 @@
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
-  const bool can_create = true;
-  const bool get_parent_variables = true;
-  const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(ConstString(name));
 
-  if (sc.block->AppendVariables(
-  can_create, get_parent_variables,
-  stop_if_block_is_inlined_function,
-  [frame](Variable *v) { return v->IsInScope(frame); },
-  &variable_list)) {
-var_sp = variable_list.FindVariable(ConstString(name));
-  }
-}
-
-if (var_sp) {
-  value_sp =
-  frame->GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+if (value_sp)
   sb_value.SetSP(value_sp, use_dynamic);
-}
   } else {
 if (log)
   log->Printf("SBFrame::FindVariable () => error: could not "
Index: lldb/trunk/include/lldb/Target/StackFrame.h
===
--- lldb/trunk/include/lldb/Target/StackFrame.h
+++ lldb/trunk/include/lldb/Target/StackFrame.h
@@ -504,6 +504,21 @@
  int64_t offset);
 
   //--
+  /// Attempt to reconstruct the ValueObject for a variable with a given \a name
+  /// from within the current StackFrame, within the current block. The search
+  /// for the variable starts in the deepest block corresponding to the current
+  /// PC in the stack frame and traverse through all parent blocks stopping at
+  /// inlined function boundaries.
+  ///
+  /// @params [in] name
+  ///   The name of the variable.
+  ///
+  /// @return
+  ///   The ValueObject if found.
+  //--
+  lldb::ValueObjectSP FindVariable(ConstString name);
+
+  //--
   // lldb::ExecutionContextScope pure virtual functions
   //--
   lldb::TargetSP CalculateTarget() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Sorry for the late review




Comment at: lldb/trunk/include/lldb/Target/ProcessStructReader.h:70
 }
-size_t total_size = struct_type.GetByteSize(nullptr);
-lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+auto total_size = struct_type.GetByteSize(nullptr);
+if (!total_size)

I will also second the sentiment on the `auto`, it does not improve readability.



Comment at: lldb/trunk/source/API/SBType.cpp:106
 uint64_t SBType::GetByteSize() {
-  if (!IsValid())
-return 0;
-
-  return m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr);
+  if (IsValid())
+if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))

The early exit is more readable here and also has a lower mental load.



Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:647
+  if (!size) {
+result.AppendError("can't get size of type");
+return false;

This does not appear to be NFC



Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:1069
+if (!size)
+  return false;
+switch (*size) {

NFC?



Comment at: lldb/trunk/source/Core/Value.cpp:227
 if (ast_type.IsValid())
-  byte_size = ast_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+  if (auto size = ast_type.GetByteSize(
+  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))

This might read better as two steps, get the value and then check it.



Comment at: lldb/trunk/source/Expression/Materializer.cpp:50
+  if (auto size = type.GetByteSize(nullptr))
+m_size = *size;
 

What value does `m_size` have otherwise?



Comment at: lldb/trunk/source/Expression/Materializer.cpp:800
+  if (!byte_size) {
+err.SetErrorString("can't get size of type");
+return;

NFC?



Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp:2372
   lldb::offset_t offset = 0;
-  if (byte_size == sizeof(float)) {
+  if (*byte_size == sizeof(float)) {
 value.GetScalar() = data.GetFloat(&offset);

switch?



Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp:827
 CompilerType compiler_type(value->GetCompilerType());
-if (compiler_type) {
+auto bit_size = compiler_type.GetBitSize(&thread);
+if (bit_size) {

Looks like you chopped out the `if (compiler_type) ` check



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1720
   if (homogeneous_count > 0 && homogeneous_count <= 4) {
+auto base_byte_size = base_type.GetByteSize(nullptr);
 if (base_type.IsVectorType(nullptr, nullptr)) {

There are a lot of checks for `base_byte_size` so it is not clear what value 
`vfp_byte_size` will have if `base_byte_size` is empty.

The flow looks a lot different but hard to track what will be set and not set.



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1788
 
-DataBufferSP data_sp(new DataBufferHeap(byte_size, 0));
+DataBufferSP data_sp(new DataBufferHeap(*byte_size, 0));
 uint32_t data_offset = 0;

`make_shared`?



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp:2343
   lldb::offset_t offset = 0;
-  if (byte_size == sizeof(float)) {
+  if (*byte_size == sizeof(float)) {
 value.GetScalar() = data.GetFloat(&offset);

switch? 



Comment at: lldb/trunk/source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp:379
 CompilerType compiler_type = value->GetCompilerType();
-if (!compiler_type)
+auto bit_size = compiler_type.GetBitSize(&thread);
+if (!bit_size)

Looks the `if (!compiler_type)` was removed.



Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:228
   }
-  CompilerType 
pair_type(__i_->GetCompilerType().GetTypeTemplateArgument(0));
-  std::string name; uint64_t bit_offset_ptr; uint32_t 
bitfield_bit_size_ptr; bool is_bitfield_ptr;
-  pair_type = pair_type.GetFieldAtIndex(0, name, &bit_offset_ptr, 
&bitfield_bit_size_ptr, &is_bitfield_ptr);
+  CompilerType pair_type(
+  __i_->GetCompilerType().GetTypeTemplateArgument(0));

Unrelated formatting changes?



Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:243
   m_pair_ptr = nullptr;
-  if (addr && addr!=LLDB_INVALID_ADDRESS) {
-ClangASTContext *ast_ctx = 
llvm::dyn_cast_or_null(pair_type.GetTypeSystem());
+  if (addr && addr != LLDB

[Lldb-commits] [PATCH] D57222: Simplify LangOpts initalization in ClangExpressionParser [NFC]

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Nice improvement in readability!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57222



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

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, jingham, teemperor, martong.
Herald added a subscriber: rnkovacs.
Herald added a reviewer: serge-sans-paille.

When we are creating a ClassTemplateSpecializationDecl in 
ParseTypeFromDWARF(...) we are not handling the case where variadic pack is 
empty in the specialization. This patch handles that case and adds a test to 
prevent future regressions.


https://reviews.llvm.org/D57363

Files:
  packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile
  
packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py
  packages/Python/lldbsuite/test/expression_command/radar_47565290/main.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1549,25 +1549,31 @@
 }
   }
 
-  if (template_param_infos.packed_args &&
-  template_param_infos.packed_args->args.size()) {
+  if (template_param_infos.packed_args) {
 IdentifierInfo *identifier_info = nullptr;
 if (template_param_infos.pack_name && template_param_infos.pack_name[0])
   identifier_info = &ast->Idents.get(template_param_infos.pack_name);
 const bool parameter_pack_true = true;
-if (IsValueParam(template_param_infos.packed_args->args[0])) {
-  template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
-  template_param_infos.packed_args->args[0].getIntegralType(),
-  parameter_pack_true, nullptr));
-} else {
+
+if (template_param_infos.packed_args->args.size()) {
+
+  if (IsValueParam(template_param_infos.packed_args->args[0])) {
+template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
+*ast, decl_context, SourceLocation(), SourceLocation(), depth,
+num_template_params, identifier_info,
+template_param_infos.packed_args->args[0].getIntegralType(),
+parameter_pack_true, nullptr));
+  } else {
+template_param_decls.push_back(TemplateTypeParmDecl::Create(
+*ast, decl_context, SourceLocation(), SourceLocation(), depth,
+num_template_params, identifier_info, is_typename,
+parameter_pack_true));
+  }
+} else if (identifier_info) {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
-  is_typename, parameter_pack_true));
+  *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+  num_template_params, identifier_info, is_typename,
+  parameter_pack_true));
 }
   }
   clang::Expr *const requires_clause = nullptr; // TODO: Concepts
Index: packages/Python/lldbsuite/test/expression_command/radar_47565290/main.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_47565290/main.cpp
@@ -0,0 +1,26 @@
+template 
+class A {
+
+};
+
+class BB;
+
+template <>
+class A {
+
+};
+
+class Value{};
+
+class BB final : public Value,
+ public A {
+public:
+int foo() { return 1;}
+};
+
+
+int main() {
+  BB b;
+
+  return b.foo() - 1; // break here
+}
Index: packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py
@@ -0,0 +1,26 @@
+"""
+Test Expression Parser code gen for ClassTemplateSpecializationDecl. This is a fix for rdar://problem/47564499
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestClassTemplateSpecializationParametersHandling(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_with_run_command(self):
+"""Test that that file and class static variables display correctly."""
+self.build()
+
+(self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("expr -u 0 -- b.foo()", substrs=['$0 = 1'])
Index: packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 184173.
shafik marked 19 inline comments as done.
shafik added a comment.

Addressed comments, including

- Renamed test
- Reduced test size
- Stream-lined code in CreateTemplateParameterList(...)


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

https://reviews.llvm.org/D57363

Files:
  
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
  
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
  
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
  source/Symbol/ClangASTContext.cpp


Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1549,25 +1549,24 @@
 }
   }
 
-  if (template_param_infos.packed_args &&
-  template_param_infos.packed_args->args.size()) {
+  if (template_param_infos.packed_args) {
 IdentifierInfo *identifier_info = nullptr;
 if (template_param_infos.pack_name && template_param_infos.pack_name[0])
   identifier_info = &ast->Idents.get(template_param_infos.pack_name);
 const bool parameter_pack_true = true;
-if (IsValueParam(template_param_infos.packed_args->args[0])) {
+
+if (!template_param_infos.packed_args->args.empty() &&
+IsValueParam(template_param_infos.packed_args->args[0])) {
   template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
+  *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+  num_template_params, identifier_info,
   template_param_infos.packed_args->args[0].getIntegralType(),
   parameter_pack_true, nullptr));
 } else {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
-  is_typename, parameter_pack_true));
+  *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+  num_template_params, identifier_info, is_typename,
+  parameter_pack_true));
 }
   }
   clang::Expr *const requires_clause = nullptr; // TODO: Concepts
Index: 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
@@ -0,0 +1,9 @@
+template 
+struct A {
+int foo() { return 1;}
+};
+
+int main() {
+  A b;
+  return b.foo(); // break here
+}
Index: 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
@@ -0,0 +1,23 @@
+"""
+Test Expression Parser code gen for ClassTemplateSpecializationDecl to insure
+that we generate a TemplateTypeParmDecl in the TemplateParameterList for empty
+variadic packs.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestClassTemplateSpecializationParametersHandling(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_class_template_specialization(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("expr -u 0 -- b.foo()", substrs=['$0 = 1'])
Index: 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules


Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1549,25 +1549,24 @@
 }
   }
 
-  if (template_param_infos.packed_args &&
-  template_param_infos.packed_args->args.size()) {
+  if (template_param_infos.packed_args) {
 IdentifierInfo *identifier_info = nullptr;
 if (template_param_infos.pack_name && template_param_infos.pack_name[0])
   identifier_info = &ast->Idents.get(template_param_infos.pack_name);
 const bool

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@aprantl @teemperor @davide thank you for the review, some great catches. I 
believe I have addressed the comments.




Comment at: 
packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile:1
+LEVEL = ../../make
+

aprantl wrote:
> Could you give the directory a more descriptive name instead of 
> `radar_47565290`? It's fine to mention a rdar:// link in the commit message, 
> but for most people radar numbers are completely opaque.
Makes sense, there are a bunch of tests like this. I did not check how old they 
were though.



Comment at: 
packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py:20
+def test_with_run_command(self):
+"""Test that that file and class static variables display correctly."""
+self.build()

aprantl wrote:
> This needs to be updated, too.
I will just remove it since the top comment should be sufficient.



Comment at: 
packages/Python/lldbsuite/test/expression_command/radar_47565290/main.cpp:1
+template 
+class A {

teemperor wrote:
> Maybe I miss something, but this could be simpler I think? E.g. like this:
> ```
> template 
> struct A {
> int foo() { return 1;}
> };
> 
> int main() {
>   A b;
>   return b.foo(); // break here
> }
> ```
Good catch, I was modeling the code that was the original source of the 
problem. I had simplified it a lot but yes I could have kept going it appears.



Comment at: source/Symbol/ClangASTContext.cpp:1562
+template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
+*ast, decl_context, SourceLocation(), SourceLocation(), depth,
+num_template_params, identifier_info,

teemperor wrote:
> aprantl wrote:
> > does this get more or less readable if we replace `SourceLocation()` with 
> > `{}`?
> We have `SourceLocation()` everywhere in clang, so it is at least more 
> consistent this way IMHO.
I also think using `{}` here would obscure more then help, using `{}` in return 
statements they make much more sense usually though.



Comment at: source/Symbol/ClangASTContext.cpp:1572
+  }
+} else if (identifier_info) {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(

davide wrote:
> aprantl wrote:
> > Looks like this is the same code as above. Could this be organized 
> > differently to avoid duplicating the code? (I didn't look whether these are 
> > all different constructors, so maybe this is already as good as it gets)
> Why this code block became conditional of `if (identifier_info)` ?
> I understand the size check above, but can you elaborate on the need for this?
I don't think I need it, I thought I did but looking at it over again I don't 
think that was justified. 



Comment at: source/Symbol/ClangASTContext.cpp:1572
+  }
+} else if (identifier_info) {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(

shafik wrote:
> davide wrote:
> > aprantl wrote:
> > > Looks like this is the same code as above. Could this be organized 
> > > differently to avoid duplicating the code? (I didn't look whether these 
> > > are all different constructors, so maybe this is already as good as it 
> > > gets)
> > Why this code block became conditional of `if (identifier_info)` ?
> > I understand the size check above, but can you elaborate on the need for 
> > this?
> I don't think I need it, I thought I did but looking at it over again I don't 
> think that was justified. 
It was not obvious at first but I was able to simplify a bit.


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

https://reviews.llvm.org/D57363



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

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:789-792
+  assert(encodedBits.repr.unused == 0);
+  decodedBits.repr.sign = encodedBits.repr.sign;
+  decodedBits.repr.fraction = encodedBits.repr.fraction;
+  decodedBits.repr.exponent = decodeExponent(encodedBits.repr.exponent);

davide wrote:
> This bit is fine,  and it's the only one I can comment on, because I wrote it.
Your using the inactive member of union which is UB in C++.


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

https://reviews.llvm.org/D57413



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

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D57413#1376280 , @davide wrote:

> To clarify, I think we ought to fix the UB regardless, but Zachary's change 
> can go in anyway as it doesn't make the situation worse than it was before.


It does not, I was more comment on the "it is fine" part.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57413



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

[Lldb-commits] [PATCH] D57467: Fix use of non-existing variable in crashlog.py

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, jingham.
Herald added a reviewer: serge-sans-paille.

The method find_matching_slice(self) uses `uuid_str` on one of the paths but 
the variable does not exist and so this results in a NameError exception if we 
take that path.


https://reviews.llvm.org/D57467

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -260,7 +260,7 @@
 if not self.resolved_path:
 self.unavailable = True
 print("error\nerror: unable to locate '%s' with UUID %s"
-  % (self.path, uuid_str))
+  % (self.path, self.get_normalized_uuid_string()))
 return False
 
 def locate_module_and_debug_symbols(self):


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -260,7 +260,7 @@
 if not self.resolved_path:
 self.unavailable = True
 print("error\nerror: unable to locate '%s' with UUID %s"
-  % (self.path, uuid_str))
+  % (self.path, self.get_normalized_uuid_string()))
 return False
 
 def locate_module_and_debug_symbols(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352677: Fix handling of CreateTemplateParameterList when 
there is an empty pack (authored by shafik, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57363?vs=184173&id=184361#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57363

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/main.cpp
@@ -0,0 +1,9 @@
+template 
+struct A {
+int foo() { return 1;}
+};
+
+int main() {
+  A b;
+  return b.foo(); // break here
+}
Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
@@ -0,0 +1,23 @@
+"""
+Test Expression Parser code gen for ClassTemplateSpecializationDecl to insure
+that we generate a TemplateTypeParmDecl in the TemplateParameterList for empty
+variadic packs.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestClassTemplateSpecializationParametersHandling(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_class_template_specialization(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("expr -u 0 -- b.foo()", substrs=['$0 = 1'])
Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -1549,25 +1549,24 @@
 }
   }
 
-  if (template_param_infos.packed_args &&
-  template_param_infos.packed_args->args.size()) {
+  if (template_param_infos.packed_args) {
 IdentifierInfo *identifier_info = nullptr;
 if (template_param_infos.pack_name && template_param_infos.pack_name[0])
   identifier_info = &ast->Idents.get(template_param_infos.pack_name);
 const bool parameter_pack_true = true;
-if (IsValueParam(template_param_infos.packed_args->args[0])) {
+
+if (!template_param_infos.packed_args->args.empty() &&
+IsValueParam(template_param_infos.packed_args->args[0])) {
   template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
+  *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+  num_template_params, identifier_info,
   template_param_infos.packed_args->args[0].getIntegralType(),
   parameter_pack_true, nullptr));
 } else {
   template_param_decls.push_back(TemplateTypeParmDecl::Create(
-  *ast, decl_context,
-  SourceLocation(), SourceLocation(), depth, num_template_params,
-  identifier_info,
-  is_typename, parameter_pack_true));
+  *ast, decl_context, SourceLocation(), SourceLocation(), depth,
+  num_template_params, identifier_info, is_typename,
+  parameter_pack_true));
 }
   }
   clang::Expr *const requires_cl

[Lldb-commits] [PATCH] D57467: Fix use of non-existing variable in crashlog.py

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB352772: Fix use of non-existing variable in crashlog.py 
(authored by shafik, committed by ).

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57467

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -260,7 +260,7 @@
 if not self.resolved_path:
 self.unavailable = True
 print("error\nerror: unable to locate '%s' with UUID %s"
-  % (self.path, uuid_str))
+  % (self.path, self.get_normalized_uuid_string()))
 return False
 
 def locate_module_and_debug_symbols(self):


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -260,7 +260,7 @@
 if not self.resolved_path:
 self.unavailable = True
 print("error\nerror: unable to locate '%s' with UUID %s"
-  % (self.path, uuid_str))
+  % (self.path, self.get_normalized_uuid_string()))
 return False
 
 def locate_module_and_debug_symbols(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58090: [WIP] Deserialize Clang module search path from DWARF

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:266
 
-  for (size_t ci = 1; ci < path.size(); ++ci) {
-llvm::StringRef component = path[ci].GetStringRef();
+  for (size_t ci = 1; ci < module.path.size(); ++ci) {
+llvm::StringRef component = module.path[ci].GetStringRef();

JDevlieghere wrote:
> Can we make this `i`?
Or perhaps `path_index` 


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

https://reviews.llvm.org/D58090



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

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/basic/main.cpp:1
+#include 
+

Looks like you are relying on `iostream` to bring in `cstdlib` which is not 
guaranteed. 



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:444
+  switch (language) {
+  case lldb::eLanguageTypeC_plus_plus:
+  case lldb::eLanguageTypeC_plus_plus_11:

`eLanguageTypeC_plus_plus_03`?


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

https://reviews.llvm.org/D58125



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

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Looks like a great fix!




Comment at: MIDataTypes.h:67
+template 
+class MIDereferenceable {
+public:

Can we use `llvm::Optional` instead? Then we can use `getValueOr()`.



Comment at: MIDataTypes.h:71
+
+  T *Or(T &alt_value) { return m_ptr ? m_ptr : &alt_value; }
+private:

If we can not use `llvm::Optional` then why not just take `T *`?


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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D58273: Fix TestDataFormatterLibcxxListLoop.py test

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Thanks for fixing this.


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

https://reviews.llvm.org/D58273



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

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: teemperor, aprantl, jingham, martong.
Herald added subscribers: jdoerfert, rnkovacs.
Herald added a reviewer: serge-sans-paille.

This tests a fix in the ASTImpoter.cpp to ensure that we import built-in 
correctly, see differential:

https://reviews.llvm.org/D58743

Once this change is merged this test should pass and should catch regressions 
in this feature.


https://reviews.llvm.org/D58790

Files:
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
  packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m


Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,26 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SoureLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int 
(*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void 
*)0x00);", 
+substrs=['$0 = 0'])
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa


Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,26 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SoureLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int (*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void *)0x00);", 
+substrs=['$0 = 0'])
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 188780.
shafik marked 5 inline comments as done.
shafik added a comment.

Addressing comments


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

https://reviews.llvm.org/D58790

Files:
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
  packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m


Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,27 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SourceLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int 
(*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void 
*)0x00);", 
+substrs=['$0 = 0'])
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa


Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,27 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SourceLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int (*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void *)0x00);", 
+substrs=['$0 = 0'])
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58792: Add "operator bool" to SB APIs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

It stood out to me that some of the conversions were not `const` and I can see 
that `IsValid` is not consistently `const` across the API but after talking to 
@jingham it is unfortunately something we can't change.


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

https://reviews.llvm.org/D58792



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

[Lldb-commits] [PATCH] D58792: Add "operator bool" to SB APIs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D58792#1415390 , @clayborg wrote:

> In D58792#1414964 , @labath wrote:
>
> > In D58792#1414191 , @shafik wrote:
> >
> > > It stood out to me that some of the conversions were not `const` and I 
> > > can see that `IsValid` is not consistently `const` across the API but 
> > > after talking to @jingham it is unfortunately something we can't change.
> >
> >
> > Yes, that is unfortunate. I can think of three things that we could do 
> > differently though:
> >
> > 1. add a `const` version of `IsValid` where it is missing, and have and 
> > always-const `operator bool` which uses that
> > 2. give up on constness and just have a non-const `operator bool` everywhere
> > 3. add a const `operator bool` everywhere, and have `IsValid` (const or 
> > non-const) call into that
>
>
> "const" is useless when your class contains a shared pointer or a unique 
> pointer. It also changes the API mangling which we can't do. So I vote for 
> give up on const as you can call non const methods in any shared and unique 
> pointers because const is only protecting the pointer from changing.
>
> > Each option has different tradeoffs, and it's not really clear to me which 
> > one is better. I am happy to implement whichever you think is best.


The goal of `const` is to communicate to the user that it does not mutate the 
underlying object, which in general is good practice to maintain if possible.

In this case I don't see any good trade-offs here. I don't feel like adding a 
`const` version along w/ a non-`const` version really helps since it still 
muddies the waters.


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

https://reviews.llvm.org/D58792



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

[Lldb-commits] [PATCH] D59004: [lldb] Fix DW_OP_addrx uses.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2910
 //--
 // OPCODE: DW_OP_GNU_addr_index
 // OPERANDS: 1

Should this comment be updated?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59004



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

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB355525: Adding test to cover the correct import of 
SourceLocation pertaining to a built… (authored by shafik, committed by ).
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58790

Files:
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
  
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
  packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m


Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa
Index: 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
+++ 
packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,27 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SourceLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int 
(*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void 
*)0x00);", 
+substrs=['$0 = 0'])


Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
===
--- packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/main.m
@@ -0,0 +1,6 @@
+#import 
+
+int main(int argc, const char * argv[]) {
+
+return 0; // break here
+}
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
===
--- packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
+LDFLAGS += -framework Cocoa
Index: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
===
--- packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
+++ packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py
@@ -0,0 +1,27 @@
+"""
+They may be cases where an expression will import SourceLocation and if the
+SourceLocation ends up with a FileID that is a built-in we need to copy that
+buffer over correctly.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestImportBuiltinFileID(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@add_test_categories(["gmodules"])
+def test_import_builtin_fileid(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.m", False))
+
+self.expect("expr int (*DBG_CGImageGetRenderingIntent)(void *) = ((int (*)(void *))CGImageGetRenderingIntent); DBG_CGImageGetRenderingIntent((void *)0x00);", 

[Lldb-commits] [PATCH] D59040: Move ExpressionSourceCode.cpp -> ClangExpressionSourceCode.cpp

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM although there are some good comments by @aprantl


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59040



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

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.
Herald added a subscriber: ormris.

It looks like this commit introduced undefined behavior via a misaligned load 
see this log from lldb sanitized bot on green dragon

http://lab.llvm.org:8080/green/view/LLDB/job/lldb-sanitized/2050/testReport/junit/lldb-Suite/functionalities_postmortem_minidump-new/TestMiniDumpUUID_py/

Summary:

/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/lib/Support/ConvertUTF.cpp:259:14:
 runtime error: load of misaligned address 0x61700018f671 for type 'const 
llvm::UTF16' (aka 'const unsigned short'), which requires 2 byte alignment
0x61700018f671: note: pointer points here

  0c 00 00  00 2f 00 74 00 6d 00 70  00 2f 00 62 00 52 53 44  53 0a 14 1e 28 32 
3c 46  50 5a 64 6e 78
   ^ 

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/lib/Support/ConvertUTF.cpp:259:14
 in


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59433



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