[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.


================
Comment at: source/API/SBValue.cpp:1367-1371
+  if (log)
+    log->Printf("SBValue(%p)::EvaluateExpression (expr=\"%s\") => SBValue(%p) "
+                "(execution result=%d)",
+                static_cast<void *>(value_sp.get()), expr,
+                static_cast<void *>(res_val_sp.get()), expr_res);
----------------
zturner wrote:
> If you decide to make that change, note that the macros call `Format`, not `Printf`, so in this case you would have to change your format string to something like:
> 
> ```
> LLDB_LOG(log, "SBValue({0}::EvaluateExpression (expr=\"{1}\") => SBValue({2}) (execution result = {3})",
>     value_sp.get(), expr, res_val_sp.get(), expr_res);
> ```
> 
> BTW, I would discourage logging pointer values, as it makes log output non-deterministic and doesn't often help when reading log files.  That said, it wouldn't be the first time we logged pointer values.
Logging the pointer value of something long-lived can often be really helpful when you are actually debugging lldb, since you can notice some oddity by looking at the log and then get directly to the object that was logged.  Logging the values of short-lived pointers is not so useful.  I think this is more of the latter case, and maybe logging the incoming value_sp's name and whether the result was successful might be more useful.


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

https://reviews.llvm.org/D55318