Page MenuHomePhabricator

Do not add a colon chunk to the code completion of class inheritance access modifiers
ClosedPublic

Authored by yvvan on Oct 6 2017, 4:46 AM.

Details

Summary

With enabled CINDEXTEST_CODE_COMPLETE_PATTERNS env option (which enables IncludeCodePatterns in completion options) code completion after colon currently suggests access modifiers with 2 completion chunks which is incorrect.

Example:
class A : <Cursor>B
{
}

Currently we get 'NotImplemented:{TypedText public}{Colon :} (40)'
but the correct line is just 'NotImplemented:{TypedText public} (40)'

The fix introduces more specific scope that occurs between ':' and '{'
It allows us to determine when we don't need to add ':' as a second chunk to the public/protected/private access modifiers.

Diff Detail

Repository
rL LLVM

Event Timeline

yvvan created this revision.Oct 6 2017, 4:46 AM
erikjv added a comment.Oct 6 2017, 6:30 AM

lgtm, but someone else will probably have to review it too.

lib/Parse/ParseDeclCXX.cpp
3196 ↗(On Diff #117979)

InheritanceScope instead of ClassScope? I mean, it's not really the scope of the class, that would be inside the class decl.
Also, clang-format this line.

yvvan updated this revision to Diff 117988.Oct 6 2017, 6:52 AM

Fix according to the review comment

arphaman added inline comments.Oct 6 2017, 9:59 AM
include/clang/Sema/Scope.h
129 ↗(On Diff #117988)

Could you please rebase this patch? There's another scope flag that uses this value already.

yvvan updated this revision to Diff 119273.Oct 17 2017, 2:43 AM

Rebased to update ScopeFlags.
Rereview please

arphaman accepted this revision.Oct 23 2017, 2:08 PM

Overall LGTM, just a couple of comments:

include/clang/Sema/Scope.h
131 ↗(On Diff #119273)

Nit: Add '.' to the end of the comment + clang-format.

lib/Parse/ParseDeclCXX.cpp
3198 ↗(On Diff #119273)

Might be better to use getCurScope()->getFlags() | Scope::ClassInheritanceScope to avoid dealing with any future scoping rule changes if such changes will occur.

lib/Sema/SemaCodeComplete.cpp
1661 ↗(On Diff #119273)

80 columns violation, please clang-format

This revision is now accepted and ready to land.Oct 23 2017, 2:08 PM
This revision was automatically updated to reflect the committed changes.