[PATCH] D103502: Bug 41152 - [DebugInfo] Better dumping of empty location expression

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 7 02:11:10 PDT 2021


jhenderson added a comment.

Mostly looks good to me, but would be nice if one of the earlier reviewers gives thumbs up before we commit this patch.

If I'd written this patch myself, I'd have probably also added a new dedicated test that explicitly checks the output for empty location ranges. It looks like the existing test is producing an empty range as a side-effect of the behaviour under test, and so it would be quite possible that the test coverage would be lost if this beahviour changed/the testing method changed for it. What do others think?

> Bug 41152 - [DebugInfo] Better dumping of empty location expression

As a tip for this summary - we usually put tags at the start of the summary, so I'd relabel this as "[DebugInfo] Bug 41152 - Improve dumping of empty location expressions". Note also the slight change of wording too ("improve" is a verb, making the summary a full imperative sentence).

FYI for the other reviewers: as I believe this is @sohamdixit's first LLVM patch to land, I'm happy to push it for him once approval has been given.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103502



More information about the llvm-commits mailing list