[PATCH] D82545: [Debugify] Make the debugify aware of the original (-g) Debug Info

Djordje Todorovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 29 07:46:02 PDT 2020


djtodoro added inline comments.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:230
+
+* `original` - collects and checks the preservation of real debug info
+  metadata
----------------
vsk wrote:
> djtodoro wrote:
> > aprantl wrote:
> > > I think that this is a useful feature, but from the user's perspective the naming is confusing.
> > > 
> > > The name `debugify` implies that that the tool is taking input and then adds something debug-related to it. That clashes with preserving original information.
> > > 
> > > My suggestion is to either rename debugify to something more generic that encompasses both use-cases, or to share the implementation in one class, but give it two separate user-facing names. Maybe separating out the debugify action from the check action would also work (`--enable-debugify --enable-debuginfo-(preservation-)verification`).
> > Sure, I'll try to find a name for the pass(es) that will cover both use-cases. Thanks!
> @aprantl @djtodoro -- let's not change any names in this patch, please.
> 
> The 'opt -debugify' name has been referenced in written communication since 2017 (D40512). Changing it would be disruptive (e.g., we've already recorded a talk that references this name extensively).
> 
> If you strongly feel that the old name should be changed, it should at least happen in a separate patch. This patch is already large, and it's difficult to determine what needs review. The name change exacerbates this issue.
> 
> 
OK, I'll return back the name, and we can defer the discussion about the name changing for later. That will make the review process easier.


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

https://reviews.llvm.org/D82545



More information about the llvm-commits mailing list