felipepiovezan wrote:

> > I'm confused: why did you drop the non-virtual Clear method that called the 
> > virtual ClearImpl? The current version is easy to misuse.
> 
> I changed it because. `Clear` is not the only abstract function that may race 
> (there is `DumpValue`, `ToJSON` and `Clone`). This means creating the same 
> `FuncImpl` pattern. But some `OptionValue` Type may have extra function with 
> a lock and wonder why the `XXXXImpl` doesn't have one.
> 
> Removed so it is consistent.

I don' think this is a valid argument. Sure, there might be other racy methods, 
but that doesn't mean we should prefer to have 15 calls to Lock instead of a 
single one. The more calls to Lock, the more likely you are to miss you.

It's fine to expose the mutex _and_ to have the Clear/ClearImpl separation. We 
can deal with the other methods when the time comes

https://github.com/llvm/llvm-project/pull/208471
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to