Remove ^^ as a token in OpenCL #108224
Remove ^^ as a token in OpenCL #108224
Conversation
OpenCL has a reserved operator (^^), the use of which was diagnosed as an error (735c6cd). However, OpenCL also encourages working with the blocks language extension. This token has a parsing ambiguity as a result. Consider: unsigned x=0; unsigned y=x^^{return 0;}(); This should result in y holding the value zero (0^0) through an immediately invoked block call as the right-hand side of the xor operator. However, it causes errors instead because of this reserved token: https://godbolt.org/z/navf7jTv1 This token is still reserved in OpenCL 3.0, so we still wish to issue a diagnostic for its use. However, we do not need to create a token for an extension point that's been unused for about a decade. So this patch moves the diagnostic from a parsing diagnostic to a lexing diagnostic and no longer forms a single token. The diagnostic behavior is slightly worse as a result, but still seems acceptable. Part of the reason this is coming up is because WG21 is considering using ^^ as a token for reflection, so this token may come back in the future.
AaronBallman
added
clang:frontend
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Aaron Ballman (AaronBallman) ChangesOpenCL has a reserved operator (^^), the use of which was diagnosed as an error (735c6cd). However, OpenCL also encourages working with the blocks language extension. This token has a parsing ambiguity as a result. Consider: unsigned x=0; This should result in y holding the value zero (0^0) through an immediately invoked block call as the right-hand side of the xor operator. However, it causes errors instead because of this reserved token: https://godbolt.org/z/navf7jTv1 This token is still reserved in OpenCL 3.0, so we still wish to issue a diagnostic for its use. However, we do not need to create a token for an extension point that's been unused for about a decade. So this patch moves the diagnostic from a parsing diagnostic to a lexing diagnostic and no longer forms a single token. The diagnostic behavior is slightly worse as a result, but still seems acceptable. Part of the reason this is coming up is because WG21 is considering using ^^ as a token for reflection, so this token may come back in the future. Full diff: https://github.com/llvm/llvm-project/pull/108224.diff 10 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
index d92d0e8f2dbf7a..ca5fc358ce290a 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
@@ -84,7 +84,8 @@ struct AvoidUnconditionalPreprocessorIfPPCallbacks : public PPCallbacks {
return (Tok.getRawIdentifier() == "true" ||
Tok.getRawIdentifier() == "false");
default:
- return Tok.getKind() >= tok::l_square && Tok.getKind() <= tok::caretcaret;
+ return Tok.getKind() >= tok::l_square &&
+ Tok.getKind() <= tok::greatergreatergreater;
}
}
diff --git a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
index 36caf7b1e63379..fbd964d4abe861 100644
--- a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -639,7 +639,6 @@ operator-name := >
operator-name := <=
operator-name := >=
operator-name := <=>
-operator-name := ^^
operator-name := ||
operator-name := <<
operator-name := greatergreater
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index fc14bb6aa21651..889370221f32f0 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -508,6 +508,8 @@ def note_macro_expansion_here : Note<"expansion of macro %0 requested here">;
def ext_pp_opencl_variadic_macros : Extension<
"variadic macros are a Clang extension in OpenCL">;
+def err_opencl_logical_exclusive_or : Error<
+ "^^ is a reserved operator in OpenCL">;
def ext_pp_gnu_line_directive : Extension<
"this style of line directive is a GNU extension">,
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0aa2c4a70849a8..09dd193db18f20 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1394,8 +1394,6 @@ def err_modifier_expected_colon : Error<"missing ':' after %0 modifier">;
// OpenCL errors.
def err_opencl_taking_function_address_parser : Error<
"taking address of function is not allowed">;
-def err_opencl_logical_exclusive_or : Error<
- "^^ is a reserved operator in OpenCL">;
// C++ for OpenCL.
def err_openclcxx_virtual_function : Error<
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index a82ff684b2ac7d..00e150dbd7a3a7 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -255,9 +255,6 @@ PUNCTUATOR(at, "@")
PUNCTUATOR(lesslessless, "<<<")
PUNCTUATOR(greatergreatergreater, ">>>")
-// CL support
-PUNCTUATOR(caretcaret, "^^")
-
// C99 6.4.1: Keywords. These turn into kw_* tokens.
// Flags allowed:
// KEYALL - This is a keyword in all variants of C and C++, or it
diff --git a/clang/lib/Basic/OperatorPrecedence.cpp b/clang/lib/Basic/OperatorPrecedence.cpp
index 02876f14291d12..c4e8fe96cdf4b5 100644
--- a/clang/lib/Basic/OperatorPrecedence.cpp
+++ b/clang/lib/Basic/OperatorPrecedence.cpp
@@ -52,7 +52,6 @@ prec::Level getBinOpPrecedence(tok::TokenKind Kind, bool GreaterThanIsOperator,
case tok::pipeequal: return prec::Assignment;
case tok::question: return prec::Conditional;
case tok::pipepipe: return prec::LogicalOr;
- case tok::caretcaret:
case tok::ampamp: return prec::LogicalAnd;
case tok::pipe: return prec::InclusiveOr;
case tok::caret: return prec::ExclusiveOr;
diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 8647e9f2f27c3d..12cb46042c946b 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -4325,10 +4325,9 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
if (Char == '=') {
CurPtr = ConsumeChar(CurPtr, SizeTmp, Result);
Kind = tok::caretequal;
- } else if (LangOpts.OpenCL && Char == '^') {
- CurPtr = ConsumeChar(CurPtr, SizeTmp, Result);
- Kind = tok::caretcaret;
} else {
+ if (LangOpts.OpenCL && Char == '^')
+ Diag(CurPtr, diag::err_opencl_logical_exclusive_or);
Kind = tok::caret;
}
break;
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 64f284d78b24db..e7514500dc53a4 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -446,10 +446,6 @@ Parser::ParseRHSOfBinaryExpression(ExprResult LHS, prec::Level MinPrec) {
Token OpToken = Tok;
ConsumeToken();
- if (OpToken.is(tok::caretcaret)) {
- return ExprError(Diag(Tok, diag::err_opencl_logical_exclusive_or));
- }
-
// If we're potentially in a template-id, we may now be able to determine
// whether we're actually in one or not.
if (OpToken.isOneOf(tok::comma, tok::greater, tok::greatergreater,
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 88d4732c7d5c6a..3e31f3d82657a3 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -520,7 +520,6 @@ static QualType getPreferredTypeOfBinaryRHS(Sema &S, Expr *LHS,
// Logical operators, assume we want bool.
case tok::ampamp:
case tok::pipepipe:
- case tok::caretcaret:
return S.getASTContext().BoolTy;
// Operators often used for bit manipulation are typically used with the type
// of the left argument.
diff --git a/clang/test/SemaOpenCL/unsupported.cl b/clang/test/SemaOpenCL/unsupported.cl
index 75175c88fe2673..7195ceb7ca49c8 100644
--- a/clang/test/SemaOpenCL/unsupported.cl
+++ b/clang/test/SemaOpenCL/unsupported.cl
@@ -17,5 +17,7 @@ void no_vla(int n) {
}
void no_logxor(int n) {
- int logxor = n ^^ n; // expected-error {{^^ is a reserved operator in OpenCL}}
+ int logxor = n ^^ n; // expected-error {{^^ is a reserved operator in OpenCL}} \
+ expected-error {{type name requires a specifier or qualifier}} \
+ expected-error {{expected expression}}
}
|
You can test this locally with the following command:git-clang-format --diff b9c2e2e3e910f8283f52c574fd8b6a7981d6cb0d 81c7e305213deaa55131d044f17029a62e185025 --extensions cpp -- clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp clang/lib/Basic/OperatorPrecedence.cpp clang/lib/Lex/Lexer.cpp clang/lib/Parse/ParseExpr.cpp clang/lib/Sema/SemaCodeComplete.cppView the diff from clang-format here.diff --git a/clang/lib/Basic/OperatorPrecedence.cpp b/clang/lib/Basic/OperatorPrecedence.cpp
index c4e8fe96cd..1db5775844 100644
--- a/clang/lib/Basic/OperatorPrecedence.cpp
+++ b/clang/lib/Basic/OperatorPrecedence.cpp
@@ -51,7 +51,8 @@ prec::Level getBinOpPrecedence(tok::TokenKind Kind, bool GreaterThanIsOperator,
case tok::caretequal:
case tok::pipeequal: return prec::Assignment;
case tok::question: return prec::Conditional;
- case tok::pipepipe: return prec::LogicalOr;
+ case tok::pipepipe:
+ return prec::LogicalOr;
case tok::ampamp: return prec::LogicalAnd;
case tok::pipe: return prec::InclusiveOr;
case tok::caret: return prec::ExclusiveOr;
|
|
The changes to the lexer look fine. I do have a question though However it does not appear to be the case - in the OpenCL 2.0 specification for c++ |
I think I need @AnastasiaStulova to weigh in on that one, good catch! |
|
I think that the diagnostic change is reasonable. Clang doesn't support OpenCL C++ at present so we won't need to worry about it. However the mission of C++ for OpenCL is to track C++ and track OpenCL i.e. we may want to update to C++ with reflection. So it would be good to keep this as an option. But we don't have this plan right now. So if we keep this as reserve identifier for C++ for OpenCL it is probably the best approach since if one day we decide to upgrade it to C++ with reflection the code using |
Thanks! Making sure I follow along, basically you're saying this patch is good because it keeps the One thing to make sure we're both on the same page for -- should the OpenCL spec folks be alerted to the fact that |
Exactly!
It may make sense to report this indeed. An issue can be submitted to this repo: https://github.com/KhronosGroup/OpenCL-Docs. CC: @svenvh |
Thank you, I filed KhronosGroup/OpenCL-Docs#1264 |
OpenCL has a reserved operator (^^), the use of which was diagnosed as an error (735c6cd). However, OpenCL also encourages working with the blocks language extension. This token has a parsing ambiguity as a result. Consider:
unsigned x=0;
unsigned y=x^^{return 0;}();
This should result in y holding the value zero (0^0) through an immediately invoked block call as the right-hand side of the xor operator. However, it causes errors instead because of this reserved token: https://godbolt.org/z/navf7jTv1
This token is still reserved in OpenCL 3.0, so we still wish to issue a diagnostic for its use. However, we do not need to create a token for an extension point that's been unused for about a decade. So this patch moves the diagnostic from a parsing diagnostic to a lexing diagnostic and no longer forms a single token. The diagnostic behavior is slightly worse as a result, but still seems acceptable.
Part of the reason this is coming up is because WG21 is considering using ^^ as a token for reflection, so this token may come back in the future.