slinder1 wrote: > > > This looks like a reasonable path forward - though could you explain the > > > motivation for the unit test coverage, rather than adding more (or the > > > same/porting from the previous patch) lit style test coverage? > > > > > > These tests are essentially ports of the test from #221615, and I believe > > (?) they are lit style. Did I put them in the wrong directory? > > The changes to/introduction of CGDebugInfoTests.cpp - unit tests, rather than > the other test changes that are in .cpp files (limited-ctor.cpp). (oh, I see > in the old patch you had some unit test coverage in DeclTest.cpp and now > that's moved to a separate file - I guess, yeah, same question there: Why is > some test coverage in a unit test, and some in a lit (.cpp) test? Likely it > should all be in .cpp/lit tests (the absence of an existing > CGDebugINfoTests.cpp is a bit of a hint that we haven't needed/used unit > testing for this sort of functionality so far))
The `isForwardDecl` check is easy, but `isCompleteType` seems like it would require a `-NOT` check which is more fragile. Or I guess the `update_test_checks` route works with a comment that the meaningful bit is the fowardDecl-ness of the composite types? The unit version gets to actually name the checks which seems like a plus. I guess comments could do the same in the lit version, though. https://github.com/llvm/llvm-project/pull/224439 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
