[Lldb-commits] [PATCH] D13073: Add an expression parser for Go

[Zachary Turner via lldb-commits]

On Tue, Oct 20, 2015 at 7:40 PM Jim Ingham via lldb-commits <
lldb-commits at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151021/1cd347ba/attachment.html>