This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Fix the crash in getQualification
ClosedPublic

Authored by kbobyrev on Jul 29 2021, 2:30 AM.

Details

Summary

Happens when DestContext is LinkageSpecDecl and hense CurContext happens to be
both not TagDecl and NamespaceDecl.

Minimal reproducer: trigger define outline in

namespace ns {
extern "C" {
typedef int foo;
}
foo Fo^o(int id) { return id; }
}

Diff Detail

Event Timeline

kbobyrev created this revision.Jul 29 2021, 2:30 AM
kbobyrev requested review of this revision.Jul 29 2021, 2:30 AM
kbobyrev updated this revision to Diff 362687.Jul 29 2021, 2:31 AM

Fix formatting

thanks for chasing this down! as mentioned offline a test case like:

namespace ns {
extern "C" {
typedef int foo;
}
foo Fo^o(int id) { return id; }
}

in define outline should be enough to ensure it is WAI.

clang-tools-extra/clangd/AST.cpp
131

rather than having !NNS in the check below it might be nicer to put:

else {
// Other types of contexts cannot be spelled in code, just skip over them.
continue;
}

I believe continue makes sense, that way we can spell in cases like:

namespace ns {
extern "C" {
typedef int foo;
}
foo Foo(int id) { return id; }
}

foo should still become ns::foo in other contexts.

kbobyrev updated this revision to Diff 362707.Jul 29 2021, 3:39 AM

Make the controol flow less confusing.

kbobyrev marked an inline comment as done.Jul 29 2021, 3:41 AM
kbobyrev edited the summary of this revision. (Show Details)
kbobyrev updated this revision to Diff 363383.Aug 1 2021, 11:59 PM

Add test, improve docs in code.

kadircet accepted this revision.Aug 2 2021, 12:06 AM

thanks, lgtm!

This revision is now accepted and ready to land.Aug 2 2021, 12:06 AM
This revision was landed with ongoing or failed builds.Aug 2 2021, 12:08 AM
This revision was automatically updated to reflect the committed changes.