[llvm] [DwarfGenerator] Calculate relative offset according to Dwarf Version (PR #84847)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 14 01:30:55 PDT 2024
jh7370 wrote:
> > > LGTM, not sure how this isn't already causing issues...!
> >
> >
> > I had exactly the same thought. The reason there is no breakage, I think, is that it "mostly" works and if the unit tests are "round tripping" (ie, emit and read), it is "close enough" that it works. In particular, if you use llvm-dwarfdump on the emitted blobs, you get "mostly" good information.
> > But, seeing this code made me have one of those classic programmer moments: "How did this ever work?"
> > > > Looks good. Can we add a unit test for this?
> > >
> > >
> > > This is already code for generating test inputs and nothing else. What exactly would you test?
> >
> >
> > I also wondered that, but was/am willing to brainstorm and have a try at it!
> > Thank you for the review!!
>
> I just wanted to touch base and see whether we want to try to find a way to make a unit test for this change, @clayborg and @jh7370 ? As I said above, I am more than happy to give it a try but I didn't want to do something without asking!
>
> Will
I'm not convinced we should be adding unit tests to test that test helper code is doing the right thing. In theory, at least, there should be tests _of real code_ (i.e. not test code) that make use of each piece of functionality of this code. In this case, the functionality is not really being used by any existing tests - it might be used incidentally, but the behaviour of that functionality must be irrelevant or either the tests would be failing or the code is broken in some way and the test helper code bug is hiding that breakage.
I guess what I'm thinking is, the tests that use the generator _are_ the tests for the generator. It sounds to me like @hawkinsw specifically is writing tests that fail because of the bug in the generator, so those tests become the tests that cover this (assuming these tests are currently in or going to be in upstream LLVM somewhere anyway). This works because unless there are bugs in both the real code and the generator code that cancel each other out, a test will fail. If there are bugs in both, there's not much we can do about it, but this is tangential - we can't go and write a third implementation just to check that our other two are both correct (well, we could, but then we'd need a fourth implementation to test that, which in turn needs a fifth etc).
https://github.com/llvm/llvm-project/pull/84847
More information about the llvm-commits
mailing list