[PATCH] D86998: [llvm-dwarfdump] Warn user when it encounters no null terminated strings.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:533
+    Error Err = Error::success();
+    cantFail(std::move(Err));
+
----------------
Higuoxing wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > Higuoxing wrote:
> > > > MaskRay wrote:
> > > > > `operator bool` sets the checked state if Error is in a success state.
> > > > > 
> > > > > So you can just run: `(void)!Err`
> > > > > 
> > > > > If you want to ensure an Error in a success state is also checked (not in this case), `ErrorAsOutParameter ...(&Err)`
> > > > Thanks. Actually I'm not sure about it. I didn't find anything about creating a checked error in [LLVM Programmer's manual](https://llvm.org/docs/ProgrammersManual.html).
> > > > 
> > > > I usually do it using
> > > > 
> > > > ```
> > > > Error Err = Error::success();
> > > > cantFail(std::move(Err));
> > > > ```
> > > > 
> > > > Can we record it in programmer's manual?
> > > Do you actually need this here? It seems to me a `cantFail(std::move(Err));` after the loop might be more appropriate. The `getCStr` function handles the `Error` that gets passed in, so that it doesn't need checking beforehand.
> > I'm not sure whether to put it after the loop or put it after `Error::success()` but I'm sure we need it. If we remove the `cantFail(std::move(Err))` or `(void)!Err`, it will crash due to an unhandled error when parsing the following object file.
> > 
> > ```
> > ...
> > Sections:
> >   - Name: .debug_str
> >     Type: SHT_PROGBITS
> >     Size: 0
> > ```
> Sorry, I missed your point. We can remove the `cantFail(std::move(Err))`.
Actually that wasn't my point, but the solution is much better this way, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86998