This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Addusing tweak: find insertion point after definition
ClosedPublic

Authored by adamcz on Nov 24 2020, 12:01 PM.

Details

Summary

When type/function is defined in the middle of the file, previuosly we
would sometimes insert a "using" line before that definition, leading to
a compilation error. With this fix, we pick a point after such
definition in translation unit.

This is not perfect solution. For example, it still doesn't handle
"using namespace" directives. It is, however, a significant improvement.

Diff Detail

Event Timeline

adamcz created this revision.Nov 24 2020, 12:01 PM
adamcz requested review of this revision.Nov 24 2020, 12:01 PM
adamcz updated this revision to Diff 307429.Nov 24 2020, 12:02 PM

typo in description

kadircet accepted this revision.Nov 24 2020, 1:24 PM

thanks, LGTM

clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
158

why not check for this below? as usings are sorted, we expect LastUsingLoc to be after this point. so maybe change the if below to:

if(LastUsingLoc.isValid() && (!MustInsertAfterLoc.isValid() || SM.isBeforeInTranslationUnit(MustInsertAfterLoc, LastUsingLoc)))

201

nit: maybe factor this into a lambda bool IsUsingValidAt(SourceLocation) ?

This revision is now accepted and ready to land.Nov 24 2020, 1:24 PM
adamcz updated this revision to Diff 307446.Nov 24 2020, 1:56 PM
adamcz marked an inline comment as done.

addressed review comments

clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
201

Good idea. Done.

This revision was landed with ongoing or failed builds.Nov 24 2020, 2:02 PM
This revision was automatically updated to reflect the committed changes.