Index: include/lldb/API/SBDebugger.h =================================================================== --- include/lldb/API/SBDebugger.h +++ include/lldb/API/SBDebugger.h @@ -228,7 +228,7 @@ const char *GetReproducerPath() const; - void SetReproducerPath(const char *reproducer); + lldb::SBError ReplayReproducer(const char *path); lldb::ScriptLanguage GetScriptLanguage() const; Index: include/lldb/Utility/Reproducer.h =================================================================== --- include/lldb/Utility/Reproducer.h +++ include/lldb/Utility/Reproducer.h @@ -116,7 +116,6 @@ private: friend Reproducer; - void SetEnabled(bool enabled) { m_enabled = enabled; } Provider *Register(std::unique_ptr provider); /// Builds and index with provider info. @@ -129,10 +128,6 @@ /// The reproducer root directory. FileSpec m_root; - /// Flag for controlling whether we generate a reproducer when Keep is - /// called. - bool m_enabled; - /// Flag to ensure that we never call both keep and discard. bool m_done; }; @@ -165,8 +160,8 @@ const Generator *GetGenerator() const; const Loader *GetLoader() const; - llvm::Error SetCapture(bool value, FileSpec root = {}); - llvm::Error SetReplay(bool value, FileSpec root = {}); + llvm::Error SetCapture(llvm::Optional root); + llvm::Error SetReplay(llvm::Optional root); FileSpec GetReproducerPath() const; Index: scripts/interface/SBDebugger.i =================================================================== --- scripts/interface/SBDebugger.i +++ scripts/interface/SBDebugger.i @@ -376,8 +376,8 @@ const char * GetReproducerPath() const; - void - SetReproducerPath (const char *path); + lldb::SBError + ReplayReproducer (const char *path); lldb::ScriptLanguage GetScriptLanguage() const; Index: source/API/SBDebugger.cpp =================================================================== --- source/API/SBDebugger.cpp +++ source/API/SBDebugger.cpp @@ -1057,12 +1057,15 @@ : nullptr); } -void SBDebugger::SetReproducerPath(const char *p) { +SBError SBDebugger::ReplayReproducer(const char *p) { + SBError sb_error; if (m_opaque_sp) { auto error = m_opaque_sp->SetReproducerReplay(llvm::StringRef::withNullAsEmpty(p)); - llvm::consumeError(std::move(error)); + std::string error_str = llvm::toString(std::move(error)); + sb_error.ref().SetErrorString(error_str); } + return sb_error; } ScriptLanguage SBDebugger::GetScriptLanguage() const { Index: source/Commands/CommandObjectReproducer.cpp =================================================================== --- source/Commands/CommandObjectReproducer.cpp +++ source/Commands/CommandObjectReproducer.cpp @@ -144,7 +144,7 @@ auto &r = repro::Reproducer::Instance(); const char *repro_path = command.GetArgumentAtIndex(0); - if (auto e = r.SetReplay(true, FileSpec(repro_path))) { + if (auto e = r.SetReplay(FileSpec(repro_path))) { std::string error_str = llvm::toString(std::move(e)); result.AppendErrorWithFormat("%s", error_str.c_str()); return false; Index: source/Core/Debugger.cpp =================================================================== --- source/Core/Debugger.cpp +++ source/Core/Debugger.cpp @@ -419,17 +419,26 @@ } llvm::Error Debugger::SetReproducerReplay(llvm::StringRef p) { - auto &r = repro::Reproducer::Instance(); - if (auto e = r.SetReplay(true, FileSpec(p))) + llvm::Optional arg = llvm::None; + + if (!p.empty()) + arg = FileSpec(p); + + if (auto e = repro::Reproducer::Instance().SetReplay(arg)) return e; + return llvm::Error::success(); } llvm::Error Debugger::SetReproducerCapture(bool b) { - auto &r = repro::Reproducer::Instance(); - auto root = HostInfo::GetReproducerTempDir(); - if (auto e = r.SetCapture(b, root)) + llvm::Optional arg = llvm::None; + + if (b) + arg = HostInfo::GetReproducerTempDir(); + + if (auto e = repro::Reproducer::Instance().SetCapture(arg)) return e; + return llvm::Error::success(); } Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3437,8 +3437,8 @@ return Status("Provider for gdb-remote contains no files."); // Construct replay history path. - FileSpec history_file = (loader->GetRoot()); - history_file.AppendPathComponent(provider_info->files.front()); + FileSpec history_file = loader->GetRoot().CopyByAppendingPathComponent( + provider_info->files.front()); // Enable replay mode. m_replay_mode = true; Index: source/Utility/Reproducer.cpp =================================================================== --- source/Utility/Reproducer.cpp +++ source/Utility/Reproducer.cpp @@ -51,34 +51,32 @@ return nullptr; } -llvm::Error Reproducer::SetCapture(bool value, FileSpec root) { +llvm::Error Reproducer::SetCapture(llvm::Optional root) { std::lock_guard guard(m_mutex); - if (value && m_loader) + if (root && m_loader) return make_error( "cannot generate a reproducer when replay one", inconvertibleErrorCode()); - if (value) { - m_generator.emplace(root); - m_generator->SetEnabled(value); - } else { + if (root) + m_generator.emplace(*root); + else m_generator.reset(); - } return Error::success(); } -llvm::Error Reproducer::SetReplay(bool value, FileSpec root) { +llvm::Error Reproducer::SetReplay(llvm::Optional root) { std::lock_guard guard(m_mutex); - if (value && m_generator) + if (root && m_generator) return make_error( "cannot replay a reproducer when generating one", inconvertibleErrorCode()); - if (value) { - m_loader.emplace(root); + if (root) { + m_loader.emplace(*root); if (auto e = m_loader->LoadIndex()) return e; } else { @@ -96,8 +94,7 @@ return {}; } -Generator::Generator(const FileSpec &root) - : m_root(root), m_enabled(false), m_done(false) {} +Generator::Generator(const FileSpec &root) : m_root(root), m_done(false) {} Generator::~Generator() {} @@ -114,9 +111,6 @@ assert(!m_done); m_done = true; - if (!m_enabled) - return; - for (auto &provider : m_providers) provider.second->Keep(); @@ -127,9 +121,6 @@ assert(!m_done); m_done = true; - if (!m_enabled) - return; - for (auto &provider : m_providers) provider.second->Discard(); Index: tools/driver/Driver.cpp =================================================================== --- tools/driver/Driver.cpp +++ tools/driver/Driver.cpp @@ -734,7 +734,9 @@ case 'z': { SBFileSpec file(optarg); if (file.Exists()) { - m_debugger.SetReproducerPath(optarg); + SBError repro_error = m_debugger.ReplayReproducer(optarg); + if (repro_error.Fail()) + error = repro_error; } else error.SetErrorStringWithFormat("file specified in --reproducer " "(-z) option doesn't exist: '%s'", Index: unittests/Utility/ReproducerTest.cpp =================================================================== --- unittests/Utility/ReproducerTest.cpp +++ unittests/Utility/ReproducerTest.cpp @@ -33,22 +33,24 @@ // Enable capture and check that means we have a generator. { - auto error = reproducer.SetCapture(true, FileSpec("/bogus/path")); - EXPECT_FALSE(static_cast(error)); + llvm::cantFail(reproducer.SetCapture(FileSpec("/bogus/path"))); EXPECT_NE(nullptr, reproducer.GetGenerator()); - - // Make sure the bogus path is correctly set. EXPECT_EQ(FileSpec("/bogus/path"), reproducer.GetGenerator()->GetRoot()); EXPECT_EQ(FileSpec("/bogus/path"), reproducer.GetReproducerPath()); } // Ensure that we cannot enable replay. { - auto error = reproducer.SetReplay(true); + auto error = reproducer.SetReplay(FileSpec("/bogus/path")); EXPECT_TRUE(static_cast(error)); llvm::consumeError(std::move(error)); EXPECT_EQ(nullptr, reproducer.GetLoader()); } + + // Ensure we can disable the generator again. + llvm::cantFail(reproducer.SetCapture(llvm::None)); + EXPECT_EQ(nullptr, reproducer.GetGenerator()); + EXPECT_EQ(nullptr, reproducer.GetLoader()); } TEST(ReproducerTest, SetReplay) { @@ -60,7 +62,7 @@ // Enable capture and check that means we have a generator. { - auto error = reproducer.SetReplay(true, FileSpec("/bogus/path")); + auto error = reproducer.SetReplay(FileSpec("/bogus/path")); // Expected to fail because we can't load the index. EXPECT_TRUE(static_cast(error)); llvm::consumeError(std::move(error)); @@ -74,7 +76,7 @@ // Ensure that we cannot enable replay. { - auto error = reproducer.SetCapture(true); + auto error = reproducer.SetCapture(FileSpec("/bogus/path")); EXPECT_TRUE(static_cast(error)); llvm::consumeError(std::move(error)); EXPECT_EQ(nullptr, reproducer.GetGenerator()); @@ -84,8 +86,7 @@ TEST(GeneratorTest, ChangeRoot) { Reproducer reproducer; - auto error = reproducer.SetCapture(true, FileSpec("/bogus/path")); - EXPECT_FALSE(static_cast(error)); + llvm::cantFail(reproducer.SetCapture(FileSpec("/bogus/path"))); auto &generator = *reproducer.GetGenerator(); // As long as we haven't registered any providers this should work. @@ -99,8 +100,7 @@ TEST(GeneratorTest, Create) { Reproducer reproducer; - auto error = reproducer.SetCapture(true, FileSpec("/bogus/path")); - EXPECT_FALSE(static_cast(error)); + llvm::cantFail(reproducer.SetCapture(FileSpec("/bogus/path"))); auto &generator = *reproducer.GetGenerator(); auto *provider = generator.Create(); @@ -114,8 +114,7 @@ TEST(GeneratorTest, Get) { Reproducer reproducer; - auto error = reproducer.SetCapture(true, FileSpec("/bogus/path")); - EXPECT_FALSE(static_cast(error)); + llvm::cantFail(reproducer.SetCapture(FileSpec("/bogus/path"))); auto &generator = *reproducer.GetGenerator(); auto *provider = generator.Create(); @@ -128,8 +127,7 @@ TEST(GeneratorTest, GetOrCreate) { Reproducer reproducer; - auto error = reproducer.SetCapture(true, FileSpec("/bogus/path")); - EXPECT_FALSE(static_cast(error)); + llvm::cantFail(reproducer.SetCapture(FileSpec("/bogus/path"))); auto &generator = *reproducer.GetGenerator(); auto *provider = generator.GetOrCreate();