[Lldb-commits] [PATCH] D65932: [lldb] Fix dynamic_cast by not treating type info pointers as variables.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

> What are the circumstances under which we don't find metadata for a global variable and we definitely *do* want to report this as an error?

The metadata just maps the variables back to the original Clang decls. So if LLDB or Clang fail to register the metadata for any reason, then we currently report this back to the user and point out the specific variable that failed. Without this check we will just assume everything is fine but then probably fail later with another error that is less specific ("failed to run expression" or so). I'm fine with deleting this as we can still see the warning in the log file and it's obvious to the user that something went wrong if they see any error (even if it's less specific).

>   all of the tests pass

That piece of code has 0 test coverage <https://teemperor.de/lldb-coverage/coverage/Users/vsk/src/llvm.org-lldbsan/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp.html#L1338> and was checked in without any tests, so that's expected :(. But I tried for an hour now to trigger a reasonable error after removing this code, so I think this is as safe as it gets.


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

https://reviews.llvm.org/D65932