[Lldb-commits] [PATCH] D97586: [mlir][lldb] Fix several gcc warnings in mlir and lldb

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectTrace.cpp:119
       lldb::TraceSP trace_sp = traceOrErr.get();
-      if (m_options.m_verbose)
+      if (m_options.m_verbose && trace_sp)
         result.AppendMessageWithFormat("loading trace with plugin %s\n",
----------------
teemperor wrote:
> Do you still have that GCC warning around (or the GCC version that produced that warning)? I'm kinda curious what GCC is assuming here. The interface never returns a "null" shared_ptr, the shared_ptr either contains a valid object or llvm::Error is returned. IIRC the Expected<ptr> return is the "new" version of how the Plugin interface should work (the old was just a ptr with `nullptr` being an error), so if some GCC version doesn't understand the Expected<ptr> pattern then we should maybe have some more centralized workaround instead of adding all the unnecessary nullptr checks.
> 
> But that shouldn't block this patch, so feel free to land.
The gcc version is:

`gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0`

The warning is:

```
/mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp: In member function ‘virtual bool CommandObjectTraceLoad::DoExecute(lldb_private::Args&, lldb_private::CommandReturnObject&)’:
/mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp:120:39: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
  120 |         result.AppendMessageWithFormat("loading trace with plugin %s\n",
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  121 |                                        trace_sp->GetPluginName().AsCString());
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp:120:39: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
```

Interestingly, this is the only place in all of llvm (well, llvm, lld, clang and lldb) that produces this warning.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97586