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

Zachary Turner via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Dec 6 16:34:47 PST 2018

zturner added inline comments.

Comment at: include/lldb/API/SBValue.h:310-312
+  lldb::SBValue EvaluateExpression(const char *expr) const;
+  lldb::SBValue EvaluateExpression(const char *expr,
+                                   const SBExpressionOptions &options) const;
Can you use `StringRef` instead of `const char *` here?

Comment at: include/lldb/Expression/UserExpression.h:296
+           lldb::ModuleSP *jit_module_sp_ptr = nullptr,
+           const lldb::ValueObjectSP &ctx_obj = lldb::ValueObjectSP());
A reference to a `shared_ptr` seems odd.  Why don't we just say `const ValueObject* ctx_obj = nullptr`?

Comment at: include/lldb/Symbol/ClangASTContext.h:1057
+                    const EvaluateExpressionOptions &options,
+                    const lldb::ValueObjectSP &ctx_obj) override;
Same thing here.

Comment at: source/API/SBValue.cpp:1325-1327
+    if (log)
+      log->Printf(
+          "SBValue::EvaluateExpression called with an empty expression");
Instead of the lines like `if (log) log->Printf(...)` you can instead write:

LLDB_LOG(log, "SBValue::EvaluateExpression called with an empty expression");

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);
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.



More information about the lldb-commits mailing list