forked from OSchip/llvm-project
Fix the .experimental. settings feature so that we don't return an error
if an experimental setting has been removed/is missing. Add tests for the .experimental. settings behaviors -- that they correctly forward through to the real setting if it has become a real setting, that they don't generate errors when a settig has been removed. As Pavel notes in https://reviews.llvm.org/D45348, the way I'm suppressing errors in the setting is not completely correct - if any of the setting path components include "experimental", a missing setting would be declared a non-error. So settings set target.experimental.setting-that-does-not-exist true would not generate an error, which is correct. But as Pavel notes, settings set setting-does-not-exist.experimental.run-stopped true should generate an error because the unknown name occurs before the "experimental". The amount of change to do this correctly hasn't thrilled me, so I'm leaving this as-is for now. <rdar://problem/39223054> Differential Revision: https://reviews.llvm.org/D45348 llvm-svn: 331315
This commit is contained in:
parent
e2dfe8a853
commit
b81afd6598
|
@ -524,3 +524,41 @@ class SettingsCommandTestCase(TestBase):
|
|||
"target.process.extra-startup-command",
|
||||
"target.process.thread.step-avoid-regexp",
|
||||
"target.process.thread.trace-thread"])
|
||||
|
||||
# settings under an ".experimental" domain should have two properties:
|
||||
# 1. If the name does not exist with "experimental" in the name path,
|
||||
# the name lookup should try to find it without "experimental". So
|
||||
# a previously-experimental setting that has been promoted to a
|
||||
# "real" setting will still be set by the original name.
|
||||
# 2. Changing a setting with .experimental., name, where the setting
|
||||
# does not exist either with ".experimental." or without, should
|
||||
# not generate an error. So if an experimental setting is removed,
|
||||
# people who may have that in their ~/.lldbinit files should not see
|
||||
# any errors.
|
||||
def test_experimental_settings(self):
|
||||
cmdinterp = self.dbg.GetCommandInterpreter()
|
||||
result = lldb.SBCommandReturnObject()
|
||||
|
||||
# Set target.arg0 to a known value, check that we can retrieve it via
|
||||
# the actual name and via .experimental.
|
||||
self.expect('settings set target.arg0 first-value')
|
||||
self.expect('settings show target.arg0', substrs=['first-value'])
|
||||
self.expect('settings show target.experimental.arg0', substrs=['first-value'], error=False)
|
||||
|
||||
# Set target.arg0 to a new value via a target.experimental.arg0 name,
|
||||
# verify that we can read it back via both .experimental., and not.
|
||||
self.expect('settings set target.experimental.arg0 second-value', error=False)
|
||||
self.expect('settings show target.arg0', substrs=['second-value'])
|
||||
self.expect('settings show target.experimental.arg0', substrs=['second-value'], error=False)
|
||||
|
||||
# showing & setting an undefined .experimental. setting should generate no errors.
|
||||
self.expect('settings show target.experimental.setting-which-does-not-exist', patterns=['^\s$'], error=False)
|
||||
self.expect('settings set target.experimental.setting-which-does-not-exist true', error=False)
|
||||
|
||||
# A domain component before .experimental. which does not exist should give an error
|
||||
# But the code does not yet do that.
|
||||
# self.expect('settings set target.setting-which-does-not-exist.experimental.arg0 true', error=True)
|
||||
|
||||
# finally, confirm that trying to set a setting that does not exist still fails.
|
||||
# (SHOWING a setting that does not exist does not currently yield an error.)
|
||||
self.expect('settings set target.setting-which-does-not-exist true', error=True)
|
||||
|
|
|
@ -202,13 +202,24 @@ Status OptionValueProperties::SetSubValue(const ExecutionContext *exe_ctx,
|
|||
llvm::StringRef value) {
|
||||
Status error;
|
||||
const bool will_modify = true;
|
||||
llvm::SmallVector<llvm::StringRef, 8> components;
|
||||
name.split(components, '.');
|
||||
bool name_contains_experimental = false;
|
||||
for (const auto &part : components)
|
||||
if (Properties::IsSettingExperimental(part))
|
||||
name_contains_experimental = true;
|
||||
|
||||
|
||||
lldb::OptionValueSP value_sp(GetSubValue(exe_ctx, name, will_modify, error));
|
||||
if (value_sp)
|
||||
error = value_sp->SetValueFromString(value, op);
|
||||
else {
|
||||
if (error.AsCString() == nullptr)
|
||||
// Don't set an error if the path contained .experimental. - those are
|
||||
// allowed to be missing and should silently fail.
|
||||
if (name_contains_experimental == false && error.AsCString() == nullptr) {
|
||||
error.SetErrorStringWithFormat("invalid value path '%s'", name.str().c_str());
|
||||
}
|
||||
}
|
||||
return error;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue