Skip to content

Commit

Permalink
[clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier
Browse files Browse the repository at this point in the history
Summary:
Previously, we checked tokens for `tok::identifier` to see if they
were identifiers inside an Objective-C selector.

However, this missed C++ keywords like `new` and `delete`.

To fix this, this diff uses `getIdentifierInfo()` to find
identifiers or keywords inside Objective-C selectors.

Test Plan: New tests added. Ran tests with:
  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak

Reviewed By: djasper

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D46143

llvm-svn: 331067
  • Loading branch information
bhamiltoncx committed Apr 27, 2018
1 parent 47aece1 commit 345f873
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
25 changes: 21 additions & 4 deletions clang/lib/Format/TokenAnnotator.cpp
Expand Up @@ -25,6 +25,21 @@ namespace format {

namespace {

/// \brief Returns \c true if the token can be used as an identifier in
/// an Objective-C \c @selector, \c false otherwise.
///
/// Because getFormattingLangOpts() always lexes source code as
/// Objective-C++, C++ keywords like \c new and \c delete are
/// lexed as tok::kw_*, not tok::identifier, even for Objective-C.
///
/// For Objective-C and Objective-C++, both identifiers and keywords
/// are valid inside @selector(...) (or a macro which
/// invokes @selector(...)). So, we allow treat any identifier or
/// keyword as a potential Objective-C selector component.
static bool canBeObjCSelectorComponent(const FormatToken &Tok) {
return Tok.Tok.getIdentifierInfo() != nullptr;
}

/// \brief A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
Expand Down Expand Up @@ -703,9 +718,10 @@ class AnnotatingParser {
Tok->Type = TT_CtorInitializerColon;
else
Tok->Type = TT_InheritanceColon;
} else if (Tok->Previous->is(tok::identifier) && Tok->Next &&
} else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
(Tok->Next->isOneOf(tok::r_paren, tok::comma) ||
Tok->Next->startsSequence(tok::identifier, tok::colon))) {
(canBeObjCSelectorComponent(*Tok->Next) && Tok->Next->Next &&
Tok->Next->Next->is(tok::colon)))) {
// This handles a special macro in ObjC code where selectors including
// the colon are passed as macro arguments.
Tok->Type = TT_ObjCMethodExpr;
Expand Down Expand Up @@ -1346,7 +1362,7 @@ class AnnotatingParser {
TT_LeadingJavaAnnotation)) {
Current.Type = Current.Previous->Type;
}
} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
} else if (canBeObjCSelectorComponent(Current) &&
// FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
Current.Previous && Current.Previous->is(TT_CastRParen) &&
Current.Previous->MatchingParen &&
Expand Down Expand Up @@ -2650,7 +2666,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
return true;
if (Left.is(tok::r_paren) && Right.isOneOf(tok::identifier, tok::kw_new))
if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right))
// Don't space between ')' and <id> or ')' and 'new'. 'new' is not a
// keyword in Objective-C, and '+ (instancetype)new;' is a standard class
// method declaration.
Expand Down Expand Up @@ -3128,6 +3144,7 @@ void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {
for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
llvm::errs() << Tok->FakeLParens[i] << "/";
llvm::errs() << " FakeRParens=" << Tok->FakeRParens;
llvm::errs() << " II=" << Tok->Tok.getIdentifierInfo();
llvm::errs() << " Text='" << Tok->TokenText << "'\n";
if (!Tok->Next)
assert(Tok == Line.Last);
Expand Down
13 changes: 12 additions & 1 deletion clang/unittests/Format/FormatTestObjC.cpp
Expand Up @@ -945,7 +945,7 @@ TEST_F(FormatTestObjC, ObjCForIn) {
" }]) {\n}");
}

TEST_F(FormatTestObjC, ObjCNew) {
TEST_F(FormatTestObjC, ObjCCxxKeywords) {
verifyFormat("+ (instancetype)new {\n"
" return nil;\n"
"}\n");
Expand All @@ -954,6 +954,17 @@ TEST_F(FormatTestObjC, ObjCNew) {
"}\n");
verifyFormat("SEL NewSelector(void) { return @selector(new); }\n");
verifyFormat("SEL MacroSelector(void) { return MACRO(new); }\n");
verifyFormat("+ (instancetype)delete {\n"
" return nil;\n"
"}\n");
verifyFormat("+ (instancetype)myDelete {\n"
" return [self delete];\n"
"}\n");
verifyFormat("SEL DeleteSelector(void) { return @selector(delete); }\n");
verifyFormat("SEL MacroSelector(void) { return MACRO(delete); }\n");
verifyFormat("MACRO(new:)\n");
verifyFormat("MACRO(delete:)\n");
verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
}

TEST_F(FormatTestObjC, ObjCLiterals) {
Expand Down

0 comments on commit 345f873

Please sign in to comment.