[lldb-dev] [RFC} [Patch] SBFunction::ExecuteFunction (v3)

[jingham at apple.com]

> On Oct 14, 2014, at 5:50 AM, Carlo Kok <ck at remobjects.com> wrote:
> 
> 
> 
> jingham at apple.com schreef op 10/13/2014 om 8:17 PM:
>> This looks good.  A couple of trivial things,
>> 
>> 1) You make a shared pointer to a ClangFunction:
>> 
>> std::shared_ptr<ClangFunction>& func...
>> 
>> We usually append "_sp" to shared pointers so you can tell that their
>> lifespan is different from ordinary stack objects.
> 
> ke.
> 
>> 
>> 2) Also, when there are errors evaluating the function you return an
>> empty SBValue.  But SBValues can have errors (returned with GetError,
>> though only settable on the ValueObject side... Might be nice to set
>> the error for the SBValue rather than just returning an empty one.
> 
> sounds good.
> 
>> 3) You have RemoveReusableFunction but it doesn't look like you
>> protected against somebody removing a reusable function while
>> somebody else was using it.
> 
> a simple target lock around both should do there?
> 
>> 
>> 4) SBTypeMemberFunction::ExecuteFunction is explicitly ObjC only.
>> But the name doesn't express that (except that you call the parameter
>> for the implicit object "self") and you don't enforce that self is an
>> ObjC object anywhere.  Be good to make that clear.
> 
> Does this actually get returned for anything else? the SBTypeMemberFunction? If it is I'm not sure how to call this for other languages. I'll rename them to ExecuteObjcFunction and put a check in it for the next version of this patch.

Even if only ObjC returns this now, there's nothing anywhere that says that is true, so it's better to be explicit...

And it would be great to add a test case too!

Thanks for working on this, I think it will be useful.

Jim


> 
> -- 
> Carlo Kok
> RemObjects Software