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 

Comment at: 
+        # 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 


lldb-commits mailing list

Reply via email to