JDevlieghere updated this revision to Diff 165047.
JDevlieghere added a comment.

I totally agree; I've left the argument name unchanged and made the argument 
optional. I had to change the semantics slightly (because of the negation in 
the argument name) but otherwise things will continue working as before (with 
the notable exception of the default case if no -d is specified of course).


https://reviews.llvm.org/D51934

Files:
  packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
  
packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
  packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
  packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
  packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
  source/Commands/CommandObjectTarget.cpp

Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -138,6 +138,75 @@
   return num_targets;
 }
 
+// Note that the negation in the argument name causes a slightly confusing
+// mapping of the enum values,
+static OptionEnumValueElement g_dependents_enumaration[4] = {
+    {eLoadDependentsDefault, "default",
+     "Only load dependents when the target is an executables."},
+    {eLoadDependentsNo, "true",
+     "Load dependents, even if the target is not an executable."},
+    {eLoadDependentsYes, "false",
+     "Don't load dependents, even if the target is an executable."},
+    {0, nullptr, nullptr}};
+
+static OptionDefinition g_dependents_options[1] = {
+    {LLDB_OPT_SET_1, false, "no-dependents", 'd',
+     OptionParser::eOptionalArgument, nullptr, g_dependents_enumaration, 0,
+     eArgTypeValue,
+     "Whether or not to load dependents when creating a target."}};
+
+class OptionGroupDependents : public OptionGroup {
+public:
+  OptionGroupDependents() {}
+
+  ~OptionGroupDependents() override {}
+
+  llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+    return llvm::makeArrayRef(g_dependents_options);
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+                        ExecutionContext *execution_context) override {
+    Status error;
+
+    // For compatibility no value means don't load dependents.
+    if (option_value.empty()) {
+      m_load_dependent_files = eLoadDependentsNo;
+      return error;
+    }
+
+    const int short_option = g_dependents_options[option_idx].short_option;
+    switch (short_option) {
+    case 'd': {
+      LoadDependentFiles tmp_load_dependents;
+      tmp_load_dependents = (LoadDependentFiles)OptionArgParser::ToOptionEnum(
+          option_value, g_dependents_options[option_idx].enum_values, 0, error);
+      if (error.Success()) {
+        m_load_dependent_files = tmp_load_dependents;
+      }
+      break;
+    }
+    default:
+      error.SetErrorStringWithFormat("unrecognized short option '%c'",
+                                     short_option);
+      break;
+    }
+
+    return error;
+  }
+
+  Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+    m_load_dependent_files = eLoadDependentsDefault;
+  }
+
+  LoadDependentFiles m_load_dependent_files;
+
+private:
+  DISALLOW_COPY_AND_ASSIGN(OptionGroupDependents);
+};
+
 #pragma mark CommandObjectTargetCreate
 
 //-------------------------------------------------------------------------
@@ -158,16 +227,14 @@
                         eArgTypePath,
                         "Path to the remote file to use for this target."),
         m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
-                      eArgTypeFilename, "Fullpath to a stand alone debug "
-                                        "symbols file for when debug symbols "
-                                        "are not in the executable."),
+                      eArgTypeFilename,
+                      "Fullpath to a stand alone debug "
+                      "symbols file for when debug symbols "
+                      "are not in the executable."),
         m_remote_file(
             LLDB_OPT_SET_1, false, "remote-file", 'r', 0, eArgTypeFilename,
             "Fullpath to the file on the remote host if debugging remotely."),
-        m_add_dependents(LLDB_OPT_SET_1, false, "no-dependents", 'd',
-                         "Don't load dependent files when creating the target, "
-                         "just add the specified executable.",
-                         true, true) {
+        m_add_dependents() {
     CommandArgumentEntry arg;
     CommandArgumentData file_arg;
 
@@ -259,12 +326,9 @@
 
       TargetSP target_sp;
       llvm::StringRef arch_cstr = m_arch_option.GetArchitectureName();
-      const bool get_dependent_files =
-          m_add_dependents.GetOptionValue().GetCurrentValue();
       Status error(debugger.GetTargetList().CreateTarget(
           debugger, file_path, arch_cstr,
-          get_dependent_files ? eLoadDependentsYes : eLoadDependentsNo, nullptr,
-          target_sp));
+          m_add_dependents.m_load_dependent_files, nullptr, target_sp));
 
       if (target_sp) {
         // Only get the platform after we create the target because we might
@@ -412,7 +476,7 @@
   OptionGroupFile m_platform_path;
   OptionGroupFile m_symbol_file;
   OptionGroupFile m_remote_file;
-  OptionGroupBoolean m_add_dependents;
+  OptionGroupDependents m_add_dependents;
 };
 
 #pragma mark CommandObjectTargetList
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
@@ -0,0 +1,17 @@
+//===-- main.c --------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+extern int a_function ();
+extern int b_function ();
+
+int
+main (int argc, char const *argv[])
+{
+    return a_function();
+}
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
@@ -0,0 +1,9 @@
+LEVEL := ../../make
+
+LIB_PREFIX := load_
+
+DYLIB_NAME := $(LIB_PREFIX)a
+DYLIB_CXX_SOURCES := a.cpp
+DYLIB_ONLY := YES
+
+include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
@@ -0,0 +1,13 @@
+//===-- b.c -----------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+int a_function ()
+{
+    return 500;
+}
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
@@ -0,0 +1,95 @@
+"""
+Test that breakpoint by symbol name works correctly with dynamic libs.
+"""
+
+from __future__ import print_function
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetDependentsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.build()
+
+    def has_exactly_one_image(self, matching, msg=""):
+        self.expect(
+            "image list",
+            "image list should contain at least one image",
+            substrs=['[  0]'])
+        should_match = not matching
+        self.expect(
+            "image list", msg, matching=should_match, substrs=['[  1]'])
+
+    def test_dependents_implicit_default_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create  " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_default_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -ddefault " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_true_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -dtrue " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_false_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -dfalse " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_implicit_false_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create  -d " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_implicit_default_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_default_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -ddefault " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_true_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -dtrue " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_false_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -dfalse " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_implicit_false_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -d " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
@@ -0,0 +1,16 @@
+LEVEL := ../../make
+
+LIB_PREFIX := load_
+
+LD_EXTRAS := -L. -l$(LIB_PREFIX)a
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
+
+a.out: lib_a
+
+lib_%:
+	$(MAKE) VPATH=$(SRCDIR) -I $(SRCDIR) -f $(SRCDIR)/$*.mk
+
+clean::
+	$(MAKE) -f $(SRCDIR)/a.mk clean
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to