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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55318/new/
https://reviews.llvm.org/D55318
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
