labath wrote: > To answer your last question: because the plugins (which is really a > misnomer), as I understand it, are there to provide abstraction: the caller > shouldn't have to know what concrete instance an `ObjectFile` is. The > no-plugins-dependency enforces that at compile/link time.
This is my problem with that. You're saying the "caller shouldn't have to *know*" -- which is a statement about his state of mind. We can't enforce the state of mind through syntactic restrictions. We can prevent the caller from *saying* he expects a specific subclass, but there's not way to prevent code from expecting that implicitly (perhaps even without it knowing). Functions like `SupportsDebugMap` make it worse because they encourage writing code like that and that makes the plugin system (the abstractions created by it) less useful as a whole. Like, if I now wanted to create a new symbol file plugin that works only with a specific object file, do I add `SupportsPsym` to every object file plugin? Or we don't have to look at new plugins, we have two good examples already: SymbolFileBreakpad and SymbolFileJSON. Both of these currently depend on the their respective object file plugin. Should that be replaced by something else (and would the result be better if we did)? Nonetheless, I agree that plugins should create abstractions, but I think it's important to think about who are those abstractions directed at. The core code definitely shouldn't need to "look behind the curtain" (ideally not even through calls like `SupportsDebugMap`), but I don't think that means that noone else isn't allowed to do that. This situation is really simple, so it doesn't demonstrate the full scope of the problem, which is why I want to go back to the ProcessCore->ObjectFile example. ProcessElfCore needs to extract a lot of detailed information from ObjectFileELF. Same goes for ProcessMachCore. At some level, the information these two pairs are exchanging is the same (threads, registers, ...), so one could imagine a generic interface to pass this -- but would that help anything? I say "no", because you still want to have the two plugins to work in pair -- if you didn't, then you wouldn't need separate process core plugins. This setup would make sense if we had a generic ProcessCore class (not a plugin) which works off of information provided by the object plugins -- but that's not the situation we're in now. You can make the same case for dynamic loaders -- should MacOSX-DYLD work with ObjectFilePECOFF? > TL;DR: My stance is that if there's a way to enforce the DAG, I'm totally > fine with allowing inter-plugin dependencies where they make sense. Barring > that I think the current rule, albeit overly strict, is the easiest way to > enforce we don't regress inter-plugin dependencies in general. Enforcement is tricky, because "where it makes sense" is obviously a judgement call. It's also complicated by the fact that current graph is not a DAG. Linux linkers actually kind of enforce the graph implicitly, but since it's not a DAG now, we have a [workaround](https://github.com/llvm/llvm-project/blob/5c7bc6a0e6a5093812e6aa9719f7d98d14bb0015/lldb/source/Core/CMakeLists.txt#L92) and that prevents us catching any new bad deps. I guess it would be possible to do something similar to `NO_PLUGIN_DEPENDENCIES` in cmake, which only allows dependencies that we consider acceptable, though it's not foolproof: for example SymbolFileJSON doesn't list ObjectFileJSON in its cmake dependencies even though it definitely needs it. I think the best we can do is some cmake check plus some documentation about which dependencies we consider "okay". I don't think this can be foolproof though. Going by the examples above, I would say that we should permit dependencies on object file plugins from SymbolFiles, DynamicLoaders and Processes -- but there are still exceptions to that. For example, ProcessGdbRemote and DynamicLoaderStatic (and *maybe* SymbolFileDWARF) probably shouldn't depend on any specific ObjectFile plugin since they're meant to work with more than one (I guess that could be a general rule -- but it's not enforcable one). A rule like "there shall be no inter-plugin dependencies" is easy to enforce and that makes it appealing, but I think it will be very hard for us to come into compliance with that (and I don't think we'd like the result even if we did). If we say some deps are okay, then a lot of things become automatically compliant, and we can focus on the high-value high-complexity deps like SymbolFile<->TypeSystem. https://github.com/llvm/llvm-project/pull/139170 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
