[PATCH] D149058: [BPF][DebugInfo] Use .BPF.ext for line info when DWARF is not available

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 10 14:37:50 PDT 2023


nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

In D149058#4460285 <https://reviews.llvm.org/D149058#4460285>, @eddyz87 wrote:

> Could you please take a second look?

Sorry for the delays; I've been on vacation a lot lately!

> A silly question, who has to check the "Done" box for sub-discussions, author or reviewer?

Anyone can, but generally a reviewer makes a comment/request, then the author of the patch marks them done as they are addressed, which denotes that the requested points have been addressed.



================
Comment at: llvm/lib/DebugInfo/BTF/BTFContext.cpp:77
+                   std::function<void(Error)> ErrorHandler) {
+  auto Ctx = std::unique_ptr<BTFContext>(new BTFContext());
+  if (auto E = Ctx->BTF.parse(Obj))
----------------
eddyz87 wrote:
> MaskRay wrote:
> > make_unqiue
> I defined `BTFContext` constructor as private and clang disallows me to call private constructor here:
> 
> ```
> unique_ptr.h:1065:34: error: calling a private constructor of class 'llvm::BTFContext'
>     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
> ```
> 
> If you strictly disprove usage of `unique_ptr()` I can make the constructor public.
right, std::make_unique requires the constructor to be private, unfortunately. I think the code LGTM as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149058



More information about the llvm-commits mailing list