C++: Changes in semantic highlighting

- Fix issues with virtual/non-virtual destructors. They were not
  being correctly identified in some cases - in particular on certain
  uses in derived classes.

- Since now we do have a highlighting item for regular functions,
  constructors and destructors are now highlighted as such. This is
  more semantically correct and actually makes navigation and readiblity
  more distinguishable, since it cleary differentiates the type itself
  from its uses in expressions and declarators. (This seems to be what
  other IDEs like Eclipse, Visual Studio, KDevelop are doing.)

  NOTE: There's a switch to disable this item in the case it doesn't
  get good acceptance. Actually, the switch can be made a user
  setting...?

- Change the default color scheme so regular and virtual functions
  have the same color (virtuals continue to be italic). This makes
  sense given the above mentioned changes in constructors/destructors
  highlighting behavior. (In other schemes virtual funcions don't have
  different color, so this shouldn't be necessary in those.)

- Small renaming: "members" are now "fields" - consistent, since
  they apply for data and it's the term used in the UI.

Change-Id: Ib1aa9c0bbf28a31d09f5696460b0095fbe29de80
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>
This commit is contained in:
Leandro Melo
2012-07-12 12:47:33 +02:00
parent 187ac69a5f
commit 4a2a17af8a
3 changed files with 305 additions and 140 deletions

View File

@@ -42,6 +42,7 @@
#include <Scope.h>
#include <AST.h>
#include <SymbolVisitor.h>
#include <Overview.h>
#include <utils/qtcassert.h>
@@ -51,6 +52,12 @@
#include <utils/runextensions.h>
// This is for experimeting highlighting ctors/dtors as functions (instead of types).
// Whenever this feature is considered "accepted" the switch below should be permanently
// removed, unless we decide to actually make this a user setting - that is why it's
// currently a bool instead of a define.
static const bool highlightCtorDtorAsType = false;
using namespace CPlusPlus;
using namespace CppTools;
@@ -67,7 +74,7 @@ class CollectSymbols: protected SymbolVisitor
Document::Ptr _doc;
Snapshot _snapshot;
QSet<QByteArray> _types;
QSet<QByteArray> _members;
QSet<QByteArray> _fields;
QSet<QByteArray> _functions;
QSet<QByteArray> _statics;
bool _mainDocument;
@@ -85,9 +92,9 @@ public:
return _types;
}
const QSet<QByteArray> &members() const
const QSet<QByteArray> &fields() const
{
return _members;
return _fields;
}
const QSet<QByteArray> &functions() const
@@ -137,14 +144,14 @@ protected:
}
}
void addMember(const Name *name)
void addField(const Name *name)
{
if (! name) {
return;
} else if (name->isNameId()) {
const Identifier *id = name->identifier();
_members.insert(QByteArray::fromRawData(id->chars(), id->size()));
_fields.insert(QByteArray::fromRawData(id->chars(), id->size()));
}
}
@@ -206,7 +213,7 @@ protected:
if (symbol->isTypedef())
addType(symbol->name());
else if (! symbol->type()->isFunctionType() && symbol->enclosingScope()->isClass())
addMember(symbol->name());
addField(symbol->name());
return true;
}
@@ -284,6 +291,22 @@ static bool sortByLinePredicate(const CheckSymbols::Use &lhs, const CheckSymbols
return lhs.line < rhs.line;
}
static bool acceptName(NameAST *ast, unsigned *referenceToken)
{
*referenceToken = ast->firstToken();
DestructorNameAST *dtor = ast->asDestructorName();
if (dtor)
*referenceToken = dtor->unqualified_name->firstToken();
if (highlightCtorDtorAsType)
return true;
return !dtor
&& !ast->asConversionFunctionId()
&& !ast->asOperatorFunctionId();
}
CheckSymbols::Future CheckSymbols::go(Document::Ptr doc, const LookupContext &context, const QList<CheckSymbols::Use> &macroUses)
{
QTC_ASSERT(doc, return Future());
@@ -299,7 +322,7 @@ CheckSymbols::CheckSymbols(Document::Ptr doc, const LookupContext &context, cons
_fileName = doc->fileName();
_potentialTypes = collectTypes.types();
_potentialMembers = collectTypes.members();
_potentialFields = collectTypes.fields();
_potentialFunctions = collectTypes.functions();
_potentialStatics = collectTypes.statics();
@@ -470,27 +493,45 @@ bool CheckSymbols::visit(EnumeratorAST *ast)
bool CheckSymbols::visit(SimpleDeclarationAST *ast)
{
NameAST *declrIdNameAST = 0;
if (ast->declarator_list && !ast->declarator_list->next) {
if (ast->symbols && ! ast->symbols->next && !ast->symbols->value->isGenerated()) {
Symbol *decl = ast->symbols->value;
if (NameAST *declId = declaratorId(ast->declarator_list->value)) {
if (NameAST *nameAST = declaratorId(ast->declarator_list->value)) {
if (Function *funTy = decl->type()->asFunctionType()) {
if (funTy->isVirtual()) {
addUse(declId, SemanticInfo::VirtualMethodUse);
} else {
addFunction(_context.lookup(decl->name(), decl->enclosingScope()), declId, funTy->argumentCount());
if (funTy->isVirtual()
|| (nameAST->asDestructorName()
&& hasVirtualDestructor(_context.lookupType(funTy->enclosingScope())))) {
addUse(nameAST, SemanticInfo::VirtualMethodUse);
declrIdNameAST = nameAST;
} else if (maybeAddFunction(_context.lookup(decl->name(),
decl->enclosingScope()),
nameAST, funTy->argumentCount())) {
declrIdNameAST = nameAST;
}
}
}
}
}
return true;
}
accept(ast->decl_specifier_list);
bool CheckSymbols::visit(NamedTypeSpecifierAST *)
{
return true;
for (DeclaratorListAST *it = ast->declarator_list; it ; it = it->next) {
DeclaratorAST *declr = it->value;
if (declrIdNameAST
&& declr->core_declarator
&& declr->core_declarator->asDeclaratorId()
&& declr->core_declarator->asDeclaratorId()->name == declrIdNameAST) {
accept(declr->attribute_list);
accept(declr->postfix_declarator_list);
accept(declr->post_attribute_list);
accept(declr->initializer);
} else {
accept(declr);
}
}
return false;
}
bool CheckSymbols::visit(ElaboratedTypeSpecifierAST *ast)
@@ -510,14 +551,14 @@ bool CheckSymbols::visit(MemberAccessAST *ast)
if (const Name *name = ast->member_name->name) {
if (const Identifier *ident = name->identifier()) {
const QByteArray id = QByteArray::fromRawData(ident->chars(), ident->size());
if (_potentialMembers.contains(id)) {
if (_potentialFields.contains(id)) {
const Token start = tokenAt(ast->firstToken());
const Token end = tokenAt(ast->lastToken() - 1);
const QByteArray expression = _doc->utf8Source().mid(start.begin(), end.end() - start.begin());
const QList<LookupItem> candidates =
typeOfExpression(expression, enclosingScope(), TypeOfExpression::Preprocess);
addClassMember(candidates, ast->member_name);
maybeAddField(candidates, ast->member_name);
}
}
}
@@ -528,50 +569,120 @@ bool CheckSymbols::visit(MemberAccessAST *ast)
bool CheckSymbols::visit(CallAST *ast)
{
if (ast->base_expression) {
accept(ast->base_expression);
unsigned argumentCount = 0;
for (ExpressionListAST *it = ast->expression_list; it; it = it->next)
++argumentCount;
ExpressionAST *expr = ast->base_expression;
if (MemberAccessAST *access = ast->base_expression->asMemberAccess()) {
if (access->member_name && access->member_name->name) {
if (maybeFunction(access->member_name->name)) {
const QByteArray expression = textOf(access);
expr = access->base_expression;
const QByteArray expression = textOf(access);
const QList<LookupItem> candidates =
typeOfExpression(expression, enclosingScope(),
TypeOfExpression::Preprocess);
NameAST *memberName = access->member_name;
if (QualifiedNameAST *q = memberName->asQualifiedName())
if (QualifiedNameAST *q = memberName->asQualifiedName()) {
checkNestedName(q);
memberName = q->unqualified_name;
}
addFunction(candidates, memberName, argumentCount);
if (!maybeAddFunction(candidates, memberName, argumentCount)
&& highlightCtorDtorAsType) {
expr = ast->base_expression;
}
}
}
} else if (IdExpressionAST *idExpr = ast->base_expression->asIdExpression()) {
if (const Name *name = idExpr->name->name) {
if (maybeFunction(name)) {
expr = 0;
NameAST *exprName = idExpr->name;
if (QualifiedNameAST *q = exprName->asQualifiedName())
if (QualifiedNameAST *q = exprName->asQualifiedName()) {
checkNestedName(q);
exprName = q->unqualified_name;
}
const QList<LookupItem> candidates =
typeOfExpression(textOf(idExpr), enclosingScope(),
TypeOfExpression::Preprocess);
addFunction(candidates, exprName, argumentCount);
if (!maybeAddFunction(candidates, exprName, argumentCount)
&& highlightCtorDtorAsType) {
expr = ast->base_expression;
}
}
}
}
accept(expr);
accept(ast->expression_list);
}
return false;
}
bool CheckSymbols::visit(NewExpressionAST *ast)
{
accept(ast->new_placement);
accept(ast->type_id);
if (highlightCtorDtorAsType) {
accept(ast->new_type_id);
} else {
ClassOrNamespace *binding = 0;
NameAST *nameAST = 0;
if (ast->new_type_id) {
for (SpecifierListAST *it = ast->new_type_id->type_specifier_list; it; it = it->next) {
if (NamedTypeSpecifierAST *spec = it->value->asNamedTypeSpecifier()) {
nameAST = spec->name;
if (QualifiedNameAST *qNameAST = nameAST->asQualifiedName()) {
binding = checkNestedName(qNameAST);
if (binding)
binding = binding->findType(qNameAST->unqualified_name->name);
nameAST = qNameAST->unqualified_name;
} else if (maybeType(nameAST->name)) {
binding = _context.lookupType(nameAST->name, enclosingScope());
}
break;
}
}
}
if (binding && nameAST) {
int arguments = 0;
if (ast->new_initializer) {
if (ExpressionAST *expr = ast->new_initializer->expression) {
while (BinaryExpressionAST *binExpr = expr->asBinaryExpression()) {
expr = binExpr->right_expression;
++arguments;
}
}
}
Scope *scope = enclosingScope();
foreach (Symbol *s, binding->symbols()) {
if (Class *klass = s->asClass()) {
scope = klass;
break;
}
}
maybeAddFunction(_context.lookup(nameAST->name, scope), nameAST, arguments);
}
}
accept(ast->new_initializer);
return false;
}
QByteArray CheckSymbols::textOf(AST *ast) const
{
const Token start = tokenAt(ast->firstToken());
@@ -651,16 +762,25 @@ void CheckSymbols::checkName(NameAST *ast, Scope *scope)
if (ast->asDestructorName() != 0) {
Class *klass = scope->asClass();
if (hasVirtualDestructor(_context.lookupType(klass)))
addUse(ast, SemanticInfo::VirtualMethodUse);
else
addUse(ast, SemanticInfo::FunctionUse);
if (!klass && scope->asFunction())
klass = scope->asFunction()->enclosingScope()->asClass();
if (klass) {
if (hasVirtualDestructor(_context.lookupType(klass))) {
addUse(ast, SemanticInfo::VirtualMethodUse);
} else {
bool added = false;
if (highlightCtorDtorAsType && maybeType(ast->name))
added = maybeAddTypeOrStatic(_context.lookup(ast->name, klass), ast);
if (!added)
addUse(ast, SemanticInfo::FunctionUse);
}
}
} else if (maybeType(ast->name) || maybeStatic(ast->name)) {
const QList<LookupItem> candidates = _context.lookup(ast->name, scope);
addTypeOrStatic(candidates, ast);
} else if (maybeMember(ast->name)) {
const QList<LookupItem> candidates = _context.lookup(ast->name, scope);
addClassMember(candidates, ast);
maybeAddTypeOrStatic(_context.lookup(ast->name, scope), ast);
} else if (maybeField(ast->name)) {
maybeAddField(_context.lookup(ast->name, scope), ast);
}
}
}
@@ -694,7 +814,38 @@ bool CheckSymbols::visit(ParameterDeclarationAST *ast)
bool CheckSymbols::visit(QualifiedNameAST *ast)
{
if (ast->name) {
ClassOrNamespace *binding = 0;
ClassOrNamespace *binding = checkNestedName(ast);
if (binding && ast->unqualified_name) {
if (ast->unqualified_name->asDestructorName() != 0) {
if (hasVirtualDestructor(binding)) {
addUse(ast->unqualified_name, SemanticInfo::VirtualMethodUse);
} else {
bool added = false;
if (highlightCtorDtorAsType && maybeType(ast->name))
added = maybeAddTypeOrStatic(binding->find(ast->unqualified_name->name),
ast->unqualified_name);
if (!added)
addUse(ast->unqualified_name, SemanticInfo::FunctionUse);
}
} else {
maybeAddTypeOrStatic(binding->find(ast->unqualified_name->name), ast->unqualified_name);
}
if (TemplateIdAST *template_id = ast->unqualified_name->asTemplateId())
accept(template_id->template_argument_list);
}
}
return false;
}
ClassOrNamespace *CheckSymbols::checkNestedName(QualifiedNameAST *ast)
{
ClassOrNamespace *binding = 0;
if (ast->name) {
if (NestedNameSpecifierListAST *it = ast->nested_name_specifier_list) {
NestedNameSpecifierAST *nested_name_specifier = it->value;
if (NameAST *class_or_namespace_name = nested_name_specifier->class_or_namespace_name) { // ### remove shadowing
@@ -729,23 +880,9 @@ bool CheckSymbols::visit(QualifiedNameAST *ast)
}
}
}
if (binding && ast->unqualified_name) {
if (ast->unqualified_name->asDestructorName() != 0) {
if (hasVirtualDestructor(binding))
addUse(ast->unqualified_name, SemanticInfo::VirtualMethodUse);
else
addUse(ast->unqualified_name, SemanticInfo::FunctionUse);
} else {
addTypeOrStatic(binding->find(ast->unqualified_name->name), ast->unqualified_name);
}
if (TemplateIdAST *template_id = ast->unqualified_name->asTemplateId())
accept(template_id->template_argument_list);
}
}
return false;
return binding;
}
bool CheckSymbols::visit(TypenameTypeParameterAST *ast)
@@ -769,8 +906,25 @@ bool CheckSymbols::visit(MemInitializerAST *ast)
if (ast->name && enclosingFunction->symbol) {
if (ClassOrNamespace *binding = _context.lookupType(enclosingFunction->symbol)) {
foreach (Symbol *s, binding->symbols()) {
if (Class *klass = s->asClass()){
checkName(ast->name, klass);
if (Class *klass = s->asClass()) {
NameAST *nameAST = ast->name;
if (QualifiedNameAST *q = nameAST->asQualifiedName()) {
checkNestedName(q);
nameAST = q->unqualified_name;
}
if (highlightCtorDtorAsType && maybeType(nameAST->name)) {
checkName(nameAST, klass);
} else if (maybeField(nameAST->name)) {
maybeAddField(_context.lookup(nameAST->name, klass), nameAST);
} else {
// It's a constructor
unsigned arguments = 0;
for (ExpressionListAST *it = ast->expression_list; it; it = it->next)
++arguments;
maybeAddFunction(_context.lookup(nameAST->name, klass), nameAST, arguments);
}
break;
}
}
@@ -806,21 +960,40 @@ bool CheckSymbols::visit(FunctionDefinitionAST *ast)
accept(ast->decl_specifier_list);
_astStack.append(thisFunction);
bool processEntireDeclr = true;
if (ast->declarator && ast->symbol && ! ast->symbol->isGenerated()) {
Function *fun = ast->symbol;
if (NameAST *declId = declaratorId(ast->declarator)) {
if (QualifiedNameAST *q = declId->asQualifiedName())
declId = q->unqualified_name;
processEntireDeclr = false;
if (fun->isVirtual()) {
if (QualifiedNameAST *q = declId->asQualifiedName()) {
checkNestedName(q);
declId = q->unqualified_name;
}
if (fun->isVirtual()
|| (declId->asDestructorName()
&& hasVirtualDestructor(_context.lookupType(fun->enclosingScope())))) {
addUse(declId, SemanticInfo::VirtualMethodUse);
} else {
addFunction(_context.lookup(fun->name(), fun->enclosingScope()), declId, fun->argumentCount());
} else if (!maybeAddFunction(_context.lookup(fun->name(),
fun->enclosingScope()),
declId, fun->argumentCount())) {
processEntireDeclr = true;
}
}
}
accept(ast->declarator);
if (ast->declarator) {
if (processEntireDeclr) {
accept(ast->declarator);
} else {
accept(ast->declarator->attribute_list);
accept(ast->declarator->postfix_declarator_list);
accept(ast->declarator->post_attribute_list);
accept(ast->declarator->initializer);
}
}
accept(ast->ctor_initializer);
accept(ast->function_body);
@@ -899,14 +1072,10 @@ void CheckSymbols::addUse(const Use &use)
void CheckSymbols::addType(ClassOrNamespace *b, NameAST *ast)
{
if (! b)
unsigned startToken;
if (! b || !acceptName(ast, &startToken))
return;
unsigned startToken = ast->firstToken();
if (DestructorNameAST *dtor = ast->asDestructorName())
if (dtor->unqualified_name)
startToken = dtor->unqualified_name->firstToken();
const Token &tok = tokenAt(startToken);
if (tok.generated())
return;
@@ -916,7 +1085,6 @@ void CheckSymbols::addType(ClassOrNamespace *b, NameAST *ast)
const unsigned length = tok.length();
const Use use(line, column, length, SemanticInfo::TypeUse);
addUse(use);
//qDebug() << "added use" << oo(ast->name) << line << column << length;
}
bool CheckSymbols::isTemplateClass(Symbol *symbol) const
@@ -932,16 +1100,15 @@ bool CheckSymbols::isTemplateClass(Symbol *symbol) const
return false;
}
void CheckSymbols::addTypeOrStatic(const QList<LookupItem> &candidates, NameAST *ast)
bool CheckSymbols::maybeAddTypeOrStatic(const QList<LookupItem> &candidates, NameAST *ast)
{
unsigned startToken = ast->firstToken();
if (DestructorNameAST *dtor = ast->asDestructorName())
if (dtor->unqualified_name)
startToken = dtor->unqualified_name->firstToken();
unsigned startToken;
if (!acceptName(ast, &startToken))
return false;
const Token &tok = tokenAt(startToken);
if (tok.generated())
return;
return false;
foreach (const LookupItem &r, candidates) {
Symbol *c = r.declaration();
@@ -958,39 +1125,39 @@ void CheckSymbols::addTypeOrStatic(const QList<LookupItem> &candidates, NameAST
const unsigned length = tok.length();
UseKind kind = SemanticInfo::TypeUse;
if (c->enclosingEnum() != 0)
kind = SemanticInfo::StaticUse;
const Use use(line, column, length, kind);
addUse(use);
//qDebug() << "added use" << oo(ast->name) << line << column << length;
break;
return true;
}
}
return false;
}
void CheckSymbols::addClassMember(const QList<LookupItem> &candidates, NameAST *ast)
bool CheckSymbols::maybeAddField(const QList<LookupItem> &candidates, NameAST *ast)
{
unsigned startToken = ast->firstToken();
if (DestructorNameAST *dtor = ast->asDestructorName())
if (dtor->unqualified_name)
startToken = dtor->unqualified_name->firstToken();
unsigned startToken;
if (!acceptName(ast, &startToken))
return false;
const Token &tok = tokenAt(startToken);
if (tok.generated())
return;
return false;
foreach (const LookupItem &r, candidates) {
Symbol *c = r.declaration();
if (! c)
continue;
else if (! c->isDeclaration())
return;
return false;
else if (! (c->enclosingScope() && c->enclosingScope()->isClass()))
return; // shadowed
return false; // shadowed
else if (c->isTypedef() || c->type()->isFunctionType())
return; // shadowed
return false; // shadowed
unsigned line, column;
getTokenStartPosition(startToken, &line, &column);
@@ -998,53 +1165,39 @@ void CheckSymbols::addClassMember(const QList<LookupItem> &candidates, NameAST *
const Use use(line, column, length, SemanticInfo::FieldUse);
addUse(use);
break;
return true;
}
return false;
}
void CheckSymbols::addStatic(const QList<LookupItem> &candidates, NameAST *ast)
{
if (ast->asDestructorName() != 0)
return;
unsigned startToken = ast->firstToken();
const Token &tok = tokenAt(startToken);
if (tok.generated())
return;
foreach (const LookupItem &r, candidates) {
Symbol *c = r.declaration();
if (! c)
return;
if (c->enclosingScope()->isEnum()) {
unsigned line, column;
getTokenStartPosition(startToken, &line, &column);
const unsigned length = tok.length();
const Use use(line, column, length, SemanticInfo::StaticUse);
addUse(use);
//qDebug() << "added use" << oo(ast->name) << line << column << length;
break;
}
}
}
void CheckSymbols::addFunction(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount)
bool CheckSymbols::maybeAddFunction(const QList<LookupItem> &candidates, NameAST *ast, unsigned argumentCount)
{
unsigned startToken = ast->firstToken();
if (DestructorNameAST *dtor = ast->asDestructorName())
bool isDestructor = false;
if (DestructorNameAST *dtor = ast->asDestructorName()) {
isDestructor = true;
if (dtor->unqualified_name)
startToken = dtor->unqualified_name->firstToken();
}
const Token &tok = tokenAt(startToken);
if (tok.generated())
return;
return false;
enum { Match_None, Match_TooManyArgs, Match_TooFewArgs, Match_Ok } matchType = Match_None;
SemanticInfo::UseKind kind = SemanticInfo::FunctionUse;
foreach (const LookupItem &r, candidates) {
Symbol *c = r.declaration();
if (! c)
// Skip current if there's no declaration or name.
if (! c || !c->name())
continue;
// In addition check for destructors, since the leading ~ is not taken into consideration.
// We don't want to compare destructors with something else or the other way around.
if (isDestructor != c->name()->isDestructorNameId())
continue;
Function *funTy = c->type()->asFunctionType();
@@ -1079,6 +1232,12 @@ void CheckSymbols::addFunction(const QList<LookupItem> &candidates, NameAST *ast
}
if (matchType != Match_None) {
if (highlightCtorDtorAsType
&& maybeType(ast->name)
&& kind == SemanticInfo::FunctionUse) {
return false;
}
unsigned line, column;
getTokenStartPosition(startToken, &line, &column);
const unsigned length = tok.length();
@@ -1091,7 +1250,11 @@ void CheckSymbols::addFunction(const QList<LookupItem> &candidates, NameAST *ast
const Use use(line, column, length, kind);
addUse(use);
return true;
}
return false;
}
NameAST *CheckSymbols::declaratorId(DeclaratorAST *ast) const
@@ -1120,12 +1283,12 @@ bool CheckSymbols::maybeType(const Name *name) const
return false;
}
bool CheckSymbols::maybeMember(const Name *name) const
bool CheckSymbols::maybeField(const Name *name) const
{
if (name) {
if (const Identifier *ident = name->identifier()) {
const QByteArray id = QByteArray::fromRawData(ident->chars(), ident->size());
if (_potentialMembers.contains(id))
if (_potentialFields.contains(id))
return true;
}
}