D87008 added creating a test AsmPrinter to the CodeGen unit tests, so it is now possible to fix the DIEHashTest.MemberBlock so that it provides an instance to the tested class.

In D87021#2267059, @dblaikie wrote:

The test sources were based on some simple code, but then they were cleared as much as I could.

You mean the IR was modified after compilation? Or that the original C source was simplified as much as possible?

In D87009#2266648, @dblaikie wrote:

Let's take, for example, DIEExpr. As far as I can find, with DW_FORM_sec_offset it is used only in the DebugInfo/DWARF unit tests, in dwarfgen::DIE::addStrOffsetsBaseAttribute(). If we took the approach with end-to-end functional testing, this class would not be updated. And the path is not simple to track, by the way.

API only intended for unit tests would be tested/updated with unit tests.

Ping (for this and other not yet accepted revisions in the stack).

• Relaxed checking in the test. The line is a bit longer than I suggested because the test has to validate emitting the abbr_offset field.

• Updated the test as suggested. Thanks!

In D87009#2255174, @dblaikie wrote:

In D87009#2253973, @ikudrin wrote:

That would require to update a particular method several times and potentially would leave it in a partially updated state because some code paths are hard to reach.

Could you explain this in more detail?

• Added reporting a fatal error if the generated debug info exceeds the limits supported by the format.

• Updated the test as suggested.

• Extended the test. Note that this change required rearranging the patches in the set so that this patch comes after the patches for DWARF units.

• Updated the test because the patch is being moved before fixing emitters for pre-v5 name lookup tables.

• Update to reflect changes in the parent commits.

• Updated to follow changes in the parent commits.

In D87025#2253890, @dblaikie wrote:

Guess this is untestable without creating giant output files?

In D87021#2253899, @dblaikie wrote:

General feedback: If you can include the original C source code and whatever compile command was needed to generate the IR in any tests (this includes other patches I've already approved, if possible - not vital, many of the tests have very small IR that's pretty easy to read, but it's a "nice to have") that'd be handy! Makes it easier to eyeball the dump output and understand what it's expecting.

In D87011#2252628, @dblaikie wrote:

Worth testing that the length field or other parts of the line table were written correctly per the format? Or is that already/going to be tested elsewhere? (if this is the first patch that makes debug_line able to be written in DWARF64, then I'd expect it to be testing any parts of the line table that are different/noteworthy in DWARF64 - possibly using assembly testing, but if llvm-dwarfdump's DWARF64 parsing support is well tested independently, then I guess it's good to just use it here too, like for any other tests)

In D87010#2252619, @dblaikie wrote:

Might be too fine-grained, may be better off rolling this into whatever patch uses this functionality & testing it end-to-end there, rather than with a unit test? (unless there are cases that are especially hard to reach via the usual functional tests)

In D87009#2252622, @dblaikie wrote:

Might be too fine-grained and better off rolled into whatever patch exercises this functionality when emitting assembly.

• Changed AP->isDwarf64() ? 8 : 4 to AP->getDwarfOffsetByteSize().

In D87008#2252576, @dblaikie wrote:

It's a fair bit of unit test infrastructure - what's the tradeoff between API/unit testing versus functional testing with the command line tools as most of the testing is done? (FileChecking the resulting assembly)