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

[Davide Italiano via Phabricator via lldb-commits]

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` ?


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

https://reviews.llvm.org/D61805