[Lldb-commits] [PATCH] D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo()

Davide Italiano via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri May 10 18:32:06 PDT 2019

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

What's the scenario that's causing this? Adding a nullptr check is an obviously save thing to do, but it would be excellent if we could add a comment explaining why the symbol could be nullptr.
I also do believe that the logic for checking whether the symbol is nullptr can be hoisted to the beginning of the function, see comment inline.

Comment at: source/Target/CPPLanguageRuntime.cpp:217-218
     if (symbol != NULL &&
         symbol->GetName().GetStringRef().contains("__invoke")) {
This should probably be `nullptr`, anyway, my general comment is that this check is scattered all around the function and could be centralized in a single place.

Comment at: source/Target/CPPLanguageRuntime.cpp:262-276
       if (first_template_parameter.contains("$_") ||
           (symbol != nullptr &&
            symbol->GetName().GetStringRef().contains("__invoke"))) {
         // Case 1 and 2
         optional_info.callable_case = LibCppStdFunctionCallableCase::Lambda;
       } else {
         // Case 3
Here in the `if` branch you check whether the symbol is nullptr or not, but later you dereference it unconditionally. Are you always guaranteed that you're not dereferencing `nullptr` ?



More information about the lldb-commits mailing list