jfb added a comment.

In D63363#1546490 <https://reviews.llvm.org/D63363#1546490>, @xiaobai wrote:

> In D63363#1546464 <https://reviews.llvm.org/D63363#1546464>, @jfb wrote:
>
> > Can you describe what the goal of your plugin architecture is? Maybe you 
> > need an RFC by email before moving stuff around.
> >
> > I want to understand what you're going for because as they are today the 
> > signals mostly work, and aren't really tested (because injecting these 
> > conditions isn't trivial). Anything you change is likely to break some 
> > subtle invariant which will only repro when your change is deployed at 
> > scale.
>
>
> My goal is to remove non-plugin libraries dependencies on plugins. Today, 
> Target depends on the ProcessUtility plugin because of UnixSignals. If 
> Signals were their own plugin that could be accessed through the 
> PluginManager interface, that dependency would go away. As Pavel said, this 
> feels somewhat over engineered and contrived, but it is the simplest path 
> forward. I am willing to drop this patch and go for a different solution if 
> there is a nicer solution agreed upon by the community, so an RFC sounds like 
> a nice idea. :)


Removing the dependency seems fine, but we have to be careful with how signals 
work: they're inherently global state, and you want to register / de-register 
them exactly where they're registered / de-registered right now to keep them 
working exactly the same. If you change this, we need to really think through 
why that's a good idea (it might be!).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63363/new/

https://reviews.llvm.org/D63363



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to