[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308008: Keep the IdentifierInfo in the Token for alternative
operator keyword (authored by ogoffart).
Changed prior to commit:
https://reviews.llvm.org/D35172?vs=105816=106594#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35172
Files:
cfe/trunk/include/clang/Basic/IdentifierTable.h
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/PPExpressions.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/test/Parser/MicrosoftExtensions.cpp
cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -220,26 +220,18 @@
return Diag(MacroNameTok, diag::err_pp_missing_macro_name);
IdentifierInfo *II = MacroNameTok.getIdentifierInfo();
- if (!II) {
-bool Invalid = false;
-std::string Spelling = getSpelling(MacroNameTok, );
-if (Invalid)
- return Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
-II = getIdentifierInfo(Spelling);
-
-if (!II->isCPlusPlusOperatorKeyword())
- return Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
+ if (!II)
+return Diag(MacroNameTok, diag::err_pp_macro_not_identifier);
+ if (II->isCPlusPlusOperatorKeyword()) {
// C++ 2.5p2: Alternative tokens behave the same as its primary token
// except for their spellings.
Diag(MacroNameTok, getLangOpts().MicrosoftExt
? diag::ext_pp_operator_used_as_macro_name
: diag::err_pp_operator_used_as_macro_name)
<< II << MacroNameTok.getKind();
-
// Allow #defining |and| and friends for Microsoft compatibility or
// recovery when legacy C headers are included in C++.
-MacroNameTok.setIdentifierInfo(II);
}
if ((isDefineUndef != MU_Other) && II->getPPKeywordID() == tok::pp_defined) {
Index: cfe/trunk/lib/Lex/Preprocessor.cpp
===
--- cfe/trunk/lib/Lex/Preprocessor.cpp
+++ cfe/trunk/lib/Lex/Preprocessor.cpp
@@ -712,14 +712,6 @@
II.setIsFutureCompatKeyword(false);
}
- // C++ 2.11p2: If this is an alternative representation of a C++ operator,
- // then we act as if it is the actual operator and not the textual
- // representation of it.
- if (II.isCPlusPlusOperatorKeyword() &&
- !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
// If this is an extension token, diagnose its use.
// We avoid diagnosing tokens that originate from macro definitions.
// FIXME: This warning is disabled in cases where it shouldn't be,
Index: cfe/trunk/lib/Lex/PPExpressions.cpp
===
--- cfe/trunk/lib/Lex/PPExpressions.cpp
+++ cfe/trunk/lib/Lex/PPExpressions.cpp
@@ -237,35 +237,32 @@
PP.setCodeCompletionReached();
PP.LexNonComment(PeekTok);
}
-
- // If this token's spelling is a pp-identifier, check to see if it is
- // 'defined' or if it is a macro. Note that we check here because many
- // keywords are pp-identifiers, so we can't check the kind.
- if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
- return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
- PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
- II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
- }
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
+// 'defined' or if it is a macro. Note that we check here because many
+// keywords are pp-identifiers, so we can't check the kind.
+if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
+ // Handle "defined X" and "defined(X)".
+ if (II->isStr("defined"))
+return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
+
+ if (!II->isCPlusPlusOperatorKeyword()) {
+// If this identifier isn't 'defined' or
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
ogoffart updated this revision to Diff 105816.
https://reviews.llvm.org/D35172
Files:
include/clang/Basic/IdentifierTable.h
lib/Lex/PPDirectives.cpp
lib/Lex/PPExpressions.cpp
lib/Lex/Preprocessor.cpp
test/Parser/MicrosoftExtensions.cpp
test/Preprocessor/cxx_oper_keyword.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -333,6 +333,16 @@
verifyFormat("x = (a) xor (b);");
}
+TEST_F(FormatTest, RecognizesUnaryOperatorKeywords) {
+ verifyFormat("x = compl(a);");
+ verifyFormat("x = not(a);");
+ verifyFormat("x = bitand(a);");
+ // Unary operator must not be merged with the next identifier
+ verifyFormat("x = compl a;");
+ verifyFormat("x = not a;");
+ verifyFormat("x = bitand a;");
+}
+
//===--===//
// Tests for control statements.
//===--===//
Index: test/Preprocessor/cxx_oper_keyword.cpp
===
--- test/Preprocessor/cxx_oper_keyword.cpp
+++ test/Preprocessor/cxx_oper_keyword.cpp
@@ -29,3 +29,15 @@
#ifdef and
#warning and is defined
#endif
+
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{invalid token at start of a preprocessor expression}}
+#endif
+#if or
+#endif
+
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{invalid token at start of a preprocessor expression}}
+#endif
+#if and_eq
+#endif
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -261,9 +261,7 @@
#define identifier_weird(x) __identifier(x
int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}
-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}
+extern int __identifier(and);
void f() {
__identifier(() // expected-error {{cannot convert '(' token to an identifier}}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -710,14 +710,6 @@
II.setIsFutureCompatKeyword(false);
}
- // C++ 2.11p2: If this is an alternative representation of a C++ operator,
- // then we act as if it is the actual operator and not the textual
- // representation of it.
- if (II.isCPlusPlusOperatorKeyword() &&
- !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
// If this is an extension token, diagnose its use.
// We avoid diagnosing tokens that originate from macro definitions.
// FIXME: This warning is disabled in cases where it shouldn't be,
Index: lib/Lex/PPExpressions.cpp
===
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -237,35 +237,32 @@
PP.setCodeCompletionReached();
PP.LexNonComment(PeekTok);
}
-
- // If this token's spelling is a pp-identifier, check to see if it is
- // 'defined' or if it is a macro. Note that we check here because many
- // keywords are pp-identifiers, so we can't check the kind.
- if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
- return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
- PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
- II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
- }
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
+// 'defined' or if it is a macro. Note that we check here because many
+// keywords are pp-identifiers, so we can't check the kind.
+if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
+ // Handle "defined X" and
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
ogoffart updated this revision to Diff 105815.
ogoffart marked an inline comment as done.
ogoffart added a comment.
Added check for "#if and_eq"
https://reviews.llvm.org/D35172
Files:
include/clang/Basic/IdentifierTable.h
lib/Lex/PPDirectives.cpp
lib/Lex/PPExpressions.cpp
lib/Lex/Preprocessor.cpp
test/Parser/MicrosoftExtensions.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -333,6 +333,16 @@
verifyFormat("x = (a) xor (b);");
}
+TEST_F(FormatTest, RecognizesUnaryOperatorKeywords) {
+ verifyFormat("x = compl(a);");
+ verifyFormat("x = not(a);");
+ verifyFormat("x = bitand(a);");
+ // Unary operator must not be merged with the next identifier
+ verifyFormat("x = compl a;");
+ verifyFormat("x = not a;");
+ verifyFormat("x = bitand a;");
+}
+
//===--===//
// Tests for control statements.
//===--===//
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -261,9 +261,7 @@
#define identifier_weird(x) __identifier(x
int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}
-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}
+extern int __identifier(and);
void f() {
__identifier(() // expected-error {{cannot convert '(' token to an identifier}}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -712,14 +712,6 @@
II.setIsFutureCompatKeyword(false);
}
- // C++ 2.11p2: If this is an alternative representation of a C++ operator,
- // then we act as if it is the actual operator and not the textual
- // representation of it.
- if (II.isCPlusPlusOperatorKeyword() &&
- !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
// If this is an extension token, diagnose its use.
// We avoid diagnosing tokens that originate from macro definitions.
// FIXME: This warning is disabled in cases where it shouldn't be,
Index: lib/Lex/PPExpressions.cpp
===
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -237,35 +237,30 @@
PP.setCodeCompletionReached();
PP.LexNonComment(PeekTok);
}
-
- // If this token's spelling is a pp-identifier, check to see if it is
- // 'defined' or if it is a macro. Note that we check here because many
- // keywords are pp-identifiers, so we can't check the kind.
- if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
- return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
- PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
- II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
- }
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
+// 'defined' or if it is a macro. Note that we check here because many
+// keywords are pp-identifiers, so we can't check the kind.
+if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
+ // Handle "defined X" and "defined(X)".
+ if (II->isStr("defined"))
+return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
+
+ // If this identifier isn't 'defined' or one of the special
+ // preprocessor keywords and it wasn't macro expanded, it turns
+ // into a simple 0
+ if (ValueLive)
+PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
+ Result.Val = 0;
+ Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
+
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
ogoffart marked an inline comment as done.
ogoffart added inline comments.
Comment at: lib/Lex/PPExpressions.cpp:242
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
rsmith wrote:
> I'm concerned that this will do the wrong thing for a keyword operator that
> is not permitted in a pp expression, like and_eq. It seems safer to revert
> this change and instead add a isCPlusPlusOperatorKeyword check to the "if"
> condition above.
Well spotted. I kept this code in the default: so true and false are handled
separatly, but added a condition for isCPlusPlusOperatorKeyword, and an
additional test.
https://reviews.llvm.org/D35172
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
rsmith added a comment.
It looks like there are fewer special cases with this direction then our
present one, though I worry that they'll be less obvious. On the whole, this
seems like a improvement.
Comment at: lib/Lex/PPExpressions.cpp:242
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
I'm concerned that this will do the wrong thing for a keyword operator that is
not permitted in a pp expression, like and_eq. It seems safer to revert this
change and instead add a isCPlusPlusOperatorKeyword check to the "if" condition
above.
https://reviews.llvm.org/D35172
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
erichkeane added a comment. My interaction with this is as a submitter for Melanie, so hopefully she can validate that this doesn't break anything subtle. To me it DOES seem like a good approach, however Richard likely has a better idea about it, so I want to give him a bit of time before accepting. https://reviews.llvm.org/D35172 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword
ogoffart created this revision.
Herald added a subscriber: klimek.
The goal of this commit is to fix clang-format so it does not merge tokens when
using the alternative spelling keywords. (eg: "not foo" should not become
"notfoo")
The problem is that Preprocessor::HandleIdentifier used to drop the identifier
info
from the token for these keyword. This means the first condition of
TokenAnnotator::spaceRequiredBefore is not met. We could add explicit check for
the spelling in that condition, but I think it is better to keep the
IdentifierInfo
and handle the operator keyword explicitly when needed. That actually leads to
simpler
code, and probably slightly more efficient as well.
Another side effect of this change is that __identifier(and) will now work as
one would expect, removing a FIXME from the MicrosoftExtensions.cpp test
https://reviews.llvm.org/D35172
Files:
include/clang/Basic/IdentifierTable.h
lib/Lex/PPDirectives.cpp
lib/Lex/PPExpressions.cpp
lib/Lex/Preprocessor.cpp
test/Parser/MicrosoftExtensions.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -333,6 +333,16 @@
verifyFormat("x = (a) xor (b);");
}
+TEST_F(FormatTest, RecognizesUnaryOperatorKeywords) {
+ verifyFormat("x = compl(a);");
+ verifyFormat("x = not(a);");
+ verifyFormat("x = bitand(a);");
+ // Unary operator must not be merged with the next identifier
+ verifyFormat("x = compl a;");
+ verifyFormat("x = not a;");
+ verifyFormat("x = bitand a;");
+}
+
//===--===//
// Tests for control statements.
//===--===//
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -261,9 +261,7 @@
#define identifier_weird(x) __identifier(x
int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}
-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}
+extern int __identifier(and);
void f() {
__identifier(() // expected-error {{cannot convert '(' token to an identifier}}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -712,14 +712,6 @@
II.setIsFutureCompatKeyword(false);
}
- // C++ 2.11p2: If this is an alternative representation of a C++ operator,
- // then we act as if it is the actual operator and not the textual
- // representation of it.
- if (II.isCPlusPlusOperatorKeyword() &&
- !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
// If this is an extension token, diagnose its use.
// We avoid diagnosing tokens that originate from macro definitions.
// FIXME: This warning is disabled in cases where it shouldn't be,
Index: lib/Lex/PPExpressions.cpp
===
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -237,35 +237,30 @@
PP.setCodeCompletionReached();
PP.LexNonComment(PeekTok);
}
-
- // If this token's spelling is a pp-identifier, check to see if it is
- // 'defined' or if it is a macro. Note that we check here because many
- // keywords are pp-identifiers, so we can't check the kind.
- if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
- return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
- PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
- II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
- }
switch (PeekTok.getKind()) {
- default: // Non-value token.
+ default:
+// If this token's spelling is a pp-identifier, check to see if it is
+//
