On Tue, Oct 20, 2015 at 7:40 PM Jim Ingham via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> jingham added a comment.
>
> The generic parts of this change look fine to me, with a few inlined style
> comments.
>
> 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:
>
> Foo::Foo () :
>
> m_ivar1
> , m_ivar2
>
> etc. looks very odd, and is not the way we do it anywhere else in lldb.
> Please don't write it that way.
>
> 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.
>
> With those style changes this is okay by me.
>
>
> ================
> Comment at: source/Expression/LLVMUserExpression.cpp:43-60
> @@ +42,20 @@
> +
> +LLVMUserExpression::LLVMUserExpression(ExecutionContextScope &exe_scope,
> const char *expr, const char *expr_prefix,
> + lldb::LanguageType language,
> ResultType desired_type)
> + : UserExpression(exe_scope, expr, expr_prefix, language, desired_type)
> + , m_stack_frame_bottom(LLDB_INVALID_ADDRESS)
> + , m_stack_frame_top(LLDB_INVALID_ADDRESS)
> + , m_transformed_text()
> + , m_execution_unit_sp()
> + , m_materializer_ap()
> + , m_jit_module_wp()
> + , m_enforce_valid_object(true)
> + , m_in_cplusplus_method(false)
> + , m_in_objectivec_method(false)
> + , m_in_static_method(false)
> + , m_needs_object_ptr(false)
> + , m_const_object(false)
> + , m_target(NULL)
> + , m_can_interpret(false)
> + , m_materialized_address(LLDB_INVALID_ADDRESS)
> +{
> ----------------
> 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...
>
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.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
