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