Michael137 added a comment. In D134344#3811143 <https://reviews.llvm.org/D134344#3811143>, @labath wrote:
> In D134344#3805953 <https://reviews.llvm.org/D134344#3805953>, @Michael137 > wrote: > >> that would require an audit of each API test right? > > Not really. I think this could be a purely mechanical change which replaces > `NO_DEBUG_INFO_TESTCASE = False` with something different. Ideally I'd make > that something a "inheriting from a different class". So we could have > something like `APITestCase` and a `DebugInfoTestCase` (inheriting from the > first), and the tests which want debug info replication (one can also imagine > different kinds of replication for some other tests) would inherit from the > latter. > > In D134344#3806509 <https://reviews.llvm.org/D134344#3806509>, @aprantl wrote: > >> But I'm also missing the context as to why this would be desirable, so if >> there's a good reason, let me know! > > I have two reasons for that: > The first is that from a simply engineering perspective, doing it the other > way around seems cleaner, as now we kind of have two ways to avoid > replication. Either you don't inherit from the replicated test base clase > (all lldb-server and lldb-vscode tests do that), or you do, but then mark > yourself as NO_DEBUG_INFO_TESTCASE. > Secondly, it seems to be that no-replication is a better default. We have a > lot of features that don't (or shouldn't) depend on the kind of debug info > we're using, and we're probably forgetting to add this attribute to some of > them. It's possible those tests are adding some marginal debug info coverage, > but it's hard to rely on that, because noone know what that is. So I'd say > that an opt-in is a better default (particularly for the tests we're adding > nowadays), and the replication should be done when you know you're doing > something debug-heavy. > I also think that having a more opt-in mechanism could enable us to do *more* > replication. For example, I think that running the some tests in both DWARF > v4 and v5 modes would be interesting, but I definitely wouldn't want to run > all of them, all the time. > > In D134344#3811091 <https://reviews.llvm.org/D134344#3811091>, @Michael137 > wrote: > >>> (B) Keep the `gmodules` category in the debug_info categories but add an >>> indicator (e.g., by making the `debug_info_categories` a dictionary) that >>> will skip replication if set. That would solve (1). And (2) would work as >>> it does today without changes. >> >> Uploaded alternative diff that implements this option. Seems simpler since >> tests in the `gmodules` category Just Work and we don't need to special-case >> `gmodules` in several places >> >> https://reviews.llvm.org/D134524 > > It's not exactly how I was imagining this, but I like it. :) We could perhaps move the discussion to discourse. I think the points raised are worth exploring further Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134344/new/ https://reviews.llvm.org/D134344 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
