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).

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