labath added a comment.

In D63363#1546504 <https://reviews.llvm.org/D63363#1546504>, @jfb wrote:

> 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!).


I think there's some misunderstading here. This code has nothing to do with 
signal handlers. All these classes do is describe the set of possible signals 
on a given target (e.g. if remote-debugging a linux mips machine from a darwin 
x86 host, we need to know what number does linux use to represent SIGSEGV on 
mips, etc.)...


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