labath added a comment.

Although this is technically correct and pretty consistent with our other 
"plugins", I can't help but feel that it's incredibly wasteful. Each of the 
XXXSignals.cpp files is less than a 100 lines long (with the licence header and 
all bolierplate) and it's unlikely to ever grow beyond that. And essentially, 
all these files do is define a single enum. The reason they are this long is 
because the UnixSignals class is already over-engineered (e.g. I don't see why 
LinuxSignals needs to be a separate class, or why it needs to repeat the 
descriptions/default stop values for each of the signals). Making this a plugin 
would just add another chunk of boilerplate on top of that.

I don't know about others, but I'd rather us move in a direction which reduces 
the amount of boilerplate instead of adding more of it. In my ideal world, each 
of these signal definitions would just be a bunch of (number, name) pairs. This 
doesn't have/need to be a class or a plugin; a single constexpr variable would 
suffice for that. Then we'd just cross-reference this mapping with another one 
which gives the default stop values and descriptions for each of the signals, 
and that's it.

I know I am repeating myself, but each time I say this, it's because I find 
another reason for it: I think we should start a new library which I would 
roughly define as "utilities for describing and manipulating various low-level 
aspects of processes, but which is agnostic of any actual process class". The 
idea would be that we can shove here all classes which are shared between 
lldb-server liblldb. UnixSignals would be one candidate for it. AuxVector, 
MemoryRegionInfo are others. `Plugins/Process/Utility` (where most of the 
signal classes live) would be a pretty good place for it already, were it not 
for the "Plugins" part (it would be weird for non-plugin code to depend on 
something called a "plugin"). However, maybe we could just create a new 
top-level library called "ProcessUtil" (or whatever name we come up with) and 
move the relevant stuff there.

Anyway, TL;DR: I think this should be handled differently. However, if others 
are fine with this approach, then feel free to ignore me.


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