This is an archive of the discontinued LLVM Phabricator instance.

Fix expression parser to not accept any type whose basename matches for a type that must exist at root level
ClosedPublic

Authored by clayborg on Apr 26 2018, 9:15 AM.

Details

Summary

This patch fixes an issue where we weren't looking for exact matches in the expression parser and also fixed the type lookup logic in the Module.cpp. Tests added to make sure we don't regress.

Diff Detail

Repository
rL LLVM

Event Timeline

clayborg created this revision.Apr 26 2018, 9:15 AM
aprantl added inline comments.Apr 27 2018, 8:58 AM
packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py
41 ↗(On Diff #144133)

accidentally

aprantl added a subscriber: aprantl.
labath resigned from this revision.Apr 30 2018, 6:09 AM

Jim seems to have an idea on how the type lookup should work, so I'll defer to him.

aprantl accepted this revision.Apr 30 2018, 1:49 PM

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.

packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py
48 ↗(On Diff #144133)

This currently returns

(a::namespace_only) $0 = (a = 123)

which — while convenient — behaves differently from code injected at that point in the program.

55 ↗(On Diff #144133)

both of these work at the moment.

59 ↗(On Diff #144133)

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.

67 ↗(On Diff #144133)

this works

69 ↗(On Diff #144133)

accidentally

76 ↗(On Diff #144133)

(lldb) p *((in_contains_type *)&i)
(a::contains_type::in_contains_type) $5 = (aaa = 123)

80 ↗(On Diff #144133)

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

88 ↗(On Diff #144133)

these work as expected.

This revision is now accepted and ready to land.Apr 30 2018, 1:49 PM
This revision was automatically updated to reflect the committed changes.