[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

Wed Sep 21 18:22:18 PDT 2022

clayborg added a comment.

In D134333#3805945 <https://reviews.llvm.org/D134333#3805945>, @labath wrote:

> Do we actually promise that strings returned by the SB API will live forever? That's not something *I* would want to promote. I always though that we're ConstStringifying the return values only when we don't know any better...

Anything that does return a "const char *" should currently live forever, at least that is what we have been doing so far. We don't want anyone thinking they should free the pointer. The issue here is we were previously handing out a "Status::m_string.c_str()" pointer whose lifespan used to be tied to the SBError's lifespan, but we have a lot of APIs that return a SBError instance. So if you did code like:

  value_list.GetError().GetCString()

the SBValueList::GetError() would return a SBError instance and it would immediately destruct itself and the "GetCString()" which was returning a "Status::m_string.c_str()" is now invalid. This patch in fact didn't work without fixing this issue in our public API. It would get an empty string as an error. And made our APIs less useful. So seeing as we have the SBError::GetCString() API, and we want it to work as expected for people, we really should be fixing this. Or we should add a new API like:

  size_t SBError::GetErrorString(char *buffer, size_t buffer_size);

So that the temp objects can be use to safely grab the string. But seeing as we already have the GEtCString() API, I am guessing that are probably latent bugs right now due to the unexpectedly short lifespan of the string.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333