This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Fix name lookup in ClangASTContext
ClosedPublic

Authored by evgeny777 on Nov 10 2015, 8:25 AM.

Details

Summary

The check for already searched namespaces has disappeared from DeclContextFindDeclByName() recently. This breaks variable evaluation in many cases, for example in this one

namespace ns1 {
    int var = 100;
}

namespace ns2 {
    int var = 101;
}

int main(void) {
    {
        using namespace ns1;
        printf("var=%d\n", var); // evaluation fails - multiple candidates
    }

    {
        using namespace ns2;
        printf("var=%d\n", var); // evaluation fails - multiple candidates
    }
}

Diff Detail

Event Timeline

evgeny777 updated this revision to Diff 39813.Nov 10 2015, 8:25 AM
evgeny777 retitled this revision from to [lldb] Fix name lookup in ClangASTContext.
evgeny777 updated this object.
evgeny777 added a reviewer: clayborg.
evgeny777 added subscribers: lldb-commits, KLapshin.
tberghammer requested changes to this revision.Nov 10 2015, 8:30 AM
tberghammer added a reviewer: tberghammer.
tberghammer added a subscriber: tberghammer.

Please create a test case for this scenario so it won't break again

This revision now requires changes to proceed.Nov 10 2015, 8:30 AM
evgeny777 updated this revision to Diff 39893.Nov 11 2015, 3:10 AM
evgeny777 edited edge metadata.

Added test case

tberghammer accepted this revision.Nov 11 2015, 4:57 AM
tberghammer edited edge metadata.

Te test case looks good to me. Thanks for adding it.

For the actual code change I don' know enough about this area to say anything so please wait for a feedback from Greg.

This revision is now accepted and ready to land.Nov 11 2015, 4:57 AM
dawn accepted this revision.Nov 11 2015, 9:51 AM
dawn added a reviewer: dawn.
dawn added a subscriber: dawn.

Patch is correct - without it we loop through the same decl context again. I've tested it locally - there are no new regressions.

clayborg accepted this revision.Nov 11 2015, 10:43 AM
clayborg edited edge metadata.
dawn added a comment.Nov 12 2015, 1:49 PM

See inline comment.

source/Symbol/ClangASTContext.cpp
9191

Minor efficiency improvement - change these 3 lines to:

if (!searched.insert(it->second).first)
    continue;

No need for new patch - just change in final commit.

evgeny777 closed this revision.Nov 13 2015, 3:05 AM