Execution contexts in the OptionValue subsystem?

Subprojects LLDB
1

There’s a function in OptionValueProperties ( line 2

const Property *OptionValueProperties::GetPropertyAtIndex(
const ExecutionContext *exe_ctx, bool will_modify, uint32_t idx) const {
return ProtectedGetPropertyAtIndex(idx);
}

Its callers go to some trouble to collect and pass around the ExecutionContext (e.g., GetSubValue passes it around everywhere, GetPropertyAtrIndexAs* has to keep it everywhere, the Dump mechanism passes around ExecutionContexts, etc.)

Aside from calling this function with completely ignores the ExecutionContext, I don’t see the execution contexts getting used anywhere. Is this a remnant from old code?

Sean

2

There's a function in OptionValueProperties (http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueProperties.cpp?view=markup line 234):

const Property *OptionValueProperties::GetPropertyAtIndex(
    const ExecutionContext *exe_ctx, bool will_modify, uint32_t idx) const {
  return ProtectedGetPropertyAtIndex(idx);
}

Its callers go to some trouble to collect and pass around the ExecutionContext (e.g., GetSubValue passes it around everywhere, GetPropertyAtrIndexAs* has to keep it everywhere, the Dump mechanism passes around ExecutionContexts, etc.)

Aside from calling this function with completely ignores the ExecutionContext, I don't see the execution contexts getting used anywhere. Is this a remnant from old code?

For instance (from Process.cpp):

class ProcessOptionValueProperties : public OptionValueProperties {
public:
  ProcessOptionValueProperties(const ConstString &name)
      : OptionValueProperties(name) {}

  // This constructor is used when creating ProcessOptionValueProperties when it
  // is part of a new lldb_private::Process instance. It will copy all current
  // global property values as needed
  ProcessOptionValueProperties(ProcessProperties *global_properties)
      : OptionValueProperties(*global_properties->GetValueProperties()) {}

  const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
                                     bool will_modify,
                                     uint32_t idx) const override {
    // When getting the value for a key from the process options, we will always
    // try and grab the setting from the current process if there is one. Else
    // we just
    // use the one from this instance.
    if (exe_ctx) {
      Process *process = exe_ctx->GetProcessPtr();
      if (process) {
        ProcessOptionValueProperties *instance_properties =
            static_cast<ProcessOptionValueProperties *>(
                process->GetValueProperties().get());
        if (this != instance_properties)
          return instance_properties->ProtectedGetPropertyAtIndex(idx);
      }
    }
    return ProtectedGetPropertyAtIndex(idx);
  }
};

That's what tells you whether to use the global process property, or this process specific one. Ditto for Thread properties.

Jim

3

Was this just curiosity, or was this getting in your way somehow?

Jim

4

I was evaluating whether to provide one in a target setting, and I decided not to.

Then I was looking to see why anyone wouild, and I couldn’t find anyone actually using it.

It’s a little painful to see it passed around everywhere rather than stored by Process or Target, the two things that care… but it’s not getting in the way of my work.

Sean

5

I was evaluating whether to provide one in a target setting, and I decided not to.

Then I was looking to see why anyone wouild, and I couldn't find anyone actually using it.

It's a little painful to see it passed around everywhere rather than stored by Process or Target, the two things that care... but it's not getting in the way of my work.

Not sure what you mean by that. "settings set target.process.whatever something" has to convey to the process property setter it calls down to what the current process is - if any. That's not something you can store in the process. The process property setter could check the "selected process" but that would be racy, and not a good idea.

Maybe I'm missing something, however.

Jim

6

If you don't pass an exe_ctx to the Target property setter, it will always set the global property. If that's what you want that's fine, but be aware that's what you'll get...

Jim