[PATCH] D44560: [DWARF] Rework debug line parsing to use llvm::Error and callbacks

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Currently trying to rebase this change following https://reviews.llvm.org/rLLD328284. In that change, LLD was changed to call getLineTableForUnit, instead of getOrParseLineTable. This function is effectively another level up the call stack, which, if I wanted to follow the current pattern, would result in lots of callers having to consume the Expected return value, as well as pass in a callback for the warning. I was discussing this with a colleague, and one suggestion he had was to instead register separate callbacks for warnings and errors in the `DWARFContext`. The parser would then simply call the appropriate callback. Potentially, other parts of the DWARF library could use the same callbacks ultimately. Tools would register their callback with their instance of the `DWARFContext`. Thus, for LLD, it would be to always call the LLD `warn` function (or whatever we choose to replace it - see the discussion on https://reviews.llvm.org/D44562), whilst other consumers would continue to use the default callbacks (currently `handleDebugLineParseError` and `DWARFDebugLine::warn`). This raises the question of how to handle the fatal error case (i.e. where a valid unit length cannot be read). I think the options here are the same as before:

1. As with the existing behaviour, reset the length field, and then have the caller check this.
2. Add a method to `LineTable::Prologue` that states whether the unit length is valid or not somehow. The caller would then be responsible for checking this, if it might want to continue onto the next table (as in the dwarfdump case), but can also ignore it and stop.
3. As in the current version of this diff, provide information in the `Error` returned to the caller, which the caller uses, if it wishes to.
4. Pass additional information into the callback, making the callback signature `Error(bool IsUnitLengthValid, StringRef Msg)` or similar.

I'd appreciate some feedback from people on which approach seems the best, both in terms of registering the callback(s), and in handling errors that prevent further section parsing (as opposed to further table parsing).


Repository:
  rL LLVM

https://reviews.llvm.org/D44560