aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
To aid with reviewing I documented LLDB's current behavior. I don't understand
the implementation well enough to comment on it, but I do think the the new
behavior is superior over the old one. After this patch LLDB behaves
deterministic and like the compiler would behave on that line, before it, LLDB
would do a DWIM thing with unpredictable results. So unless Jim has any
concerns about the implementation, I would give this a go.
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:48
+ self.assertTrue(expr_result.GetError().Fail(),
+ "'namespace_only' exists in namespace only")
+
----------------
This currently returns
(a::namespace_only) $0 = (a = 123)
which — while convenient — behaves differently from code injected at that point
in the program.
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:55
+ expr_result = frame.EvaluateExpression("*((b::namespace_only *)&i)")
+ self.check_value(expr_result, "b", 123)
+
----------------
both of these work at the moment.
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:59
+ expr_result = frame.EvaluateExpression("*((namespace_and_file *)&i)")
+ self.check_value(expr_result, "ff", 123)
+ # Make sure we can find the correct type in a namespace "a"
----------------
Currently:
```
error: reference to 'namespace_and_file' is ambiguous
candidate found by name lookup is 'namespace_and_file'
candidate found by name lookup is 'a::namespace_and_file'
error: reference to 'namespace_and_file' is ambiguous
candidate found by name lookup is 'namespace_and_file'
candidate found by name lookup is 'a::namespace_and_file'
```
that's a horrible failure mode.
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:67
+ "*((b::namespace_and_file *)&i)")
+ self.check_value(expr_result, "bb", 123)
+
----------------
this works
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:69
+
+ # Make sure we don't accedentally accept structures that exist only
+ # in namespaces when evaluating expressions with top level types.
----------------
accidentally
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:76
+ self.assertTrue(expr_result.GetError().Fail(),
+ "'in_contains_type' exists in struct only")
+
----------------
(lldb) p *((in_contains_type *)&i)
(a::contains_type::in_contains_type) $5 = (aaa = 123)
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:80
+ expr_result = frame.EvaluateExpression(
+ "*((contains_type::in_contains_type *)&i)")
+ self.check_value(expr_result, "fff", 123)
----------------
Similar ugly failure:
error: reference to 'contains_type' is ambiguous
candidate found by name lookup is 'contains_type'
candidate found by name lookup is 'a::contains_type'
error: expected expression
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:88
+ expr_result = frame.EvaluateExpression(
+ "*((b::contains_type::in_contains_type *)&i)")
+ self.check_value(expr_result, "bbb", 123)
----------------
these work as expected.
https://reviews.llvm.org/D46128
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
