<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 20, 2015 at 7:40 PM Jim Ingham via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">jingham added a comment.<br>
<br>
The generic parts of this change look fine to me, with a few inlined style comments.<br>
<br>
I didn't read the Go specific parts of this in detail, I assume you're going to get those right.  I mentioned a couple of style things inline, though I didn't mark everywhere that they occurred.  Particularly, the construct:<br>
<br>
Foo::Foo () :<br>
<br>
  m_ivar1<br>
  , m_ivar2<br>
<br>
etc. looks very odd, and is not the way we do it anywhere else in lldb.  Please don't write it that way.<br>
<br>
Also, we try to always call shared pointers foo_sp and unique pointers foo_up.  The life cycle of these guys is enough different from pointers that it is good to be able to see what they are in code without having to trace them back to the definition.<br>
<br>
With those style changes this is okay by me.<br>
<br>
<br>
================<br>
Comment at: source/Expression/LLVMUserExpression.cpp:43-60<br>
@@ +42,20 @@<br>
+<br>
+LLVMUserExpression::LLVMUserExpression(ExecutionContextScope &exe_scope, const char *expr, const char *expr_prefix,<br>
+                                       lldb::LanguageType language, ResultType desired_type)<br>
+    : UserExpression(exe_scope, expr, expr_prefix, language, desired_type)<br>
+    , m_stack_frame_bottom(LLDB_INVALID_ADDRESS)<br>
+    , m_stack_frame_top(LLDB_INVALID_ADDRESS)<br>
+    , m_transformed_text()<br>
+    , m_execution_unit_sp()<br>
+    , m_materializer_ap()<br>
+    , m_jit_module_wp()<br>
+    , m_enforce_valid_object(true)<br>
+    , m_in_cplusplus_method(false)<br>
+    , m_in_objectivec_method(false)<br>
+    , m_in_static_method(false)<br>
+    , m_needs_object_ptr(false)<br>
+    , m_const_object(false)<br>
+    , m_target(NULL)<br>
+    , m_can_interpret(false)<br>
+    , m_materialized_address(LLDB_INVALID_ADDRESS)<br>
+{<br>
----------------<br>
Don't write initializers with the commas in front like this.  We don't do it this way anywhere else in lldb, and it looks really weird...<br></blockquote><div><br></div><div>Sorry, this is really clang-format's fault.  Sigh, I really need to get off my you-know-what and get this fixed in clang-format because it's like one of the only things left in clang-format that doesn't do what LLDB needs.</div></div></div>