[Lldb-commits] [PATCH] D43471: Handle typeof() expressions

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

The code part of this looks fine.  I had a few quibbles with the test, see inline.



================
Comment at: packages/Python/lldbsuite/test/expression_command/test/TestExprs.py:270
+        # output: (typeof (i)) $0 = 1
+        self.expect("expression p int i; __typeof__(i) j = 1; j",
+                    substrs=['(typeof (i)) $',
----------------
labath wrote:
> Are you sure this command is actually correct? The "p" here seems odd...
Yes, all the other commands in this test that run expressions either get the stopped frame and run SBFrame.EvaluateExpression, or they spell out  "expression" completely.  Unless you are actually testing that an alias works, it's better not to use aliases in tests.

Also, was there a reason why you didn't use TestEBasicExprCommandsTestCase.build_and_run?  It looks like everything you do is done there.  I was going to say you should make sure that run stopped at the breakpoint - otherwise if that fails you'll get some weird error from the expression that may be harder to diagnose.  But then I noticed the build_and_run function which ALSO doesn't test that the run stopped at your breakpoint, but if all the tests used that then we could just add the check there.



https://reviews.llvm.org/D43471