[PATCH] D95998: [Debug-Info] [NFC] use emitDwarfUnitLength to handle debug line section unit length field

Wed Feb 17 05:14:38 PST 2021

shchenz added a comment.

In D95998#2568193 <https://reviews.llvm.org/D95998#2568193>, @ikudrin wrote:

> In D95998#2567599 <https://reviews.llvm.org/D95998#2567599>, @dblaikie wrote:
>
>> What if the function created and returned Hi rather than taking it as an argument?
>
> Hmm, I can't say I'm a vivid fan of this change. All methods of `MCStremer` instruct it to do something, i.e. it roles as just a performer. With this change, it now requests a caller to do some action. It looks like inverting customer vs. performer roles for this particular case.

Hmm, a good point again. But I am a little favor of the current design. The function name is `emitDwarfUnitLength`, so maybe the role of the `Hi` and `Lo` symbol should be the same? If we have only one of them as the function parameter, like only having `Hi`, without checking the function implementation or good comments, we may be a little strange about why the length only has one bound `Hi`? If we pass both of them, then that's ok, only from the function prototype, we know we pass the two bounds from the caller side; if we pass none of them, that is easy to understand too, callee will treat the two bound inside.

I am ok with both solutions, I will let this decision be made by debug-info experts like you @dblaikie @ikudrin

Thanks for your review!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95998/new/

https://reviews.llvm.org/D95998