> On Jan 17, 2020, at 2:24 PM, Raphael Isemann via Phabricator 
> <revi...@reviews.llvm.org> wrote:
> 
> teemperor added a comment.
> 
> (Just some quick comments, will review this properly during normal working 
> hours)
> 
> Without this fix debugging Clang with LLDB is essentially impossible, so I'm 
> in favour of landing this with as few pre-commit refactorings as possible to 
> make backporting easier and getting this in ASAP. We probably also want to 
> ping Hans to get this into the 10 release branch (which was created 2 days 
> ago, so that's just a simple cherry-pick).

If you want to make back porting easier, you might not want to use expect_expr 
in the tests, since that would require back porting that as well as this patch?

Jim


> 
> Also you might want to check out the recently added `expect_expr` command 
> instead of calling `expect` (see the inline comment for an example). The API 
> currently doesn't support the whole fuzzy substr matching (which is on 
> purpose) so you might not be able to use it everywhere (at least the current 
> version).
> 
> 
> 
> ================
> Comment at: 
> lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py:207
> +
> +        self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
> +                    substrs=['unsigned int', '2'])
> ----------------
> We can do this since last week: `self.expect_expr("(lba.a)", 
> result_type="unsigned int", result_value="2")` which is a much more safer and 
> convenient way of doing this.
> 
> 
> ================
> Comment at: lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c:128
> +      uint64_t HasNonTrivialToPrimitiveDestructCUnion : 1;
> +      uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
> +    };
> ----------------
> I would prefer generic names as otherwise Clang folks randomly see the test 
> when they grep for usages of these variables by name in the LLVM repo.
> 
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D72953/new/
> 
> https://reviews.llvm.org/D72953
> 
> 
> 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to