[clang] [clang-format] Add space after a word token (PR #92741)
https://github.com/robincaloudis closed https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
robincaloudis wrote: Thanks @owenca, @mydeveloperday and @HazardyKnusperkeks for the explanation and insights! I'm closing this issue as @owenca found a much better solution. https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
owenca wrote: See #92880. https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
HazardyKnusperkeks wrote: > > Since I am by no means an expert on Clang, a few questions arose > > > > * As Clang formats the code correctly (no code changes necessary) when > > instead of `xor` a different function name e.g. `xooor` is used, I wonder > > why `xor` is tokenized as Unary operator in the first place in C? > > * Is C and C++ using the same tokenizations? > > clang-format formats all C code as C++. Since `xor` is a C++ keyword, it's > lexed as `tok::caret`, which is then erroneously annotated as > `TT_UnaryOperator`. This bug was uncovered by #90161. > > > * How to properly distinguish between the C and C++ language in Clang > > Format? > > clang-format can't do it properly. @mydeveloperday, @HazardyKnusperkeks, and > @rymiel may know more about why we didn't add `LK_C` to the > [`Language`](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#language) > option. If I remember correctly, I was in favor of adding a C language. > Because if this code was in a .h and not a .c you wouldn't know what language > you were in There are certainly headers which are ambiguous, and we could add an option to set the language of such headers. But if we hit a `class`, `namespace`, or a `::` we could fairly certain use it as C++ header. Similar to `guessIsObjC`, and skip checking for C++ if the new option is set to C++ (which of course would be the default, to keep existing behavior). https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
mydeveloperday wrote: I thought we had some code like this around using a variable called "try" https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
mydeveloperday wrote: Because if this code was in a .h and not a .c you wouldn't know what language you were in https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
@@ -5280,7 +5280,8 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine , // handled. if (Left.is(tok::amp) && Right.is(tok::r_square)) return Style.SpacesInSquareBrackets; -return Style.SpaceAfterLogicalNot && Left.is(tok::exclaim); +return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) || + Right.is(TT_BinaryOperator); owenca wrote: This line was deleted in #90161. Adding it back will hide the uncovered bug and only fix #92688 superficially IMO. For example, `SpaceBeforeAssignmentOperators` will have no effect. https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
owenca wrote: > Since I am by no means an expert on Clang, a few questions arose > > * As Clang formats the code correctly (no code changes necessary) when > instead of `xor` a different function name e.g. `xooor` is used, I wonder why > `xor` is tokenized as Unary operator in the first place in C? > * Is C and C++ using the same tokenizations? clang-format formats all C code as C++. Since `xor` is a C++ keyword, it's lexed as `tok::caret`, which is then erroneously annotated as `TT_UnaryOperator`. This bug was uncovered by #90161. > * How to properly distinguish between the C and C++ language in Clang Format? clang-format can't do it properly. @mydeveloperday, @HazardyKnusperkeks, and @rymiel may know more about why we didn't add `LK_C` to the [`Language`](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#language) option. https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: Robin Caloudis (robincaloudis)
Changes
Closes https://github.com/llvm/llvm-project/issues/92688
---
Full diff: https://github.com/llvm/llvm-project/pull/92741.diff
2 Files Affected:
- (modified) clang/lib/Format/TokenAnnotator.cpp (+2-1)
- (modified) clang/unittests/Format/FormatTest.cpp (+7)
``diff
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index 7c4c76a91f2c5..7786b85e8a1fc 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5280,7 +5280,8 @@ bool TokenAnnotator::spaceRequiredBefore(const
AnnotatedLine ,
// handled.
if (Left.is(tok::amp) && Right.is(tok::r_square))
return Style.SpacesInSquareBrackets;
-return Style.SpaceAfterLogicalNot && Left.is(tok::exclaim);
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+ Right.is(TT_BinaryOperator);
}
// If the next token is a binary operator or a selector name, we have
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 6f57f10e12e88..ca0edd7b22630 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24545,6 +24545,13 @@ TEST_F(FormatTest, STLWhileNotDefineChed) {
"#endif // while");
}
+TEST_F(FormatTest, BinaryOperatorAfterUnaryOperator) {
+ verifyFormat("void test(void) {\n"
+ " static void (*xor)(uint8_t *, size_t, uint8_t);\n"
+ " xor = resolve_xor_x86();\n"
+ "}");
+}
+
TEST_F(FormatTest, OperatorSpacing) {
FormatStyle Style = getLLVMStyle();
Style.PointerAlignment = FormatStyle::PAS_Right;
``
https://github.com/llvm/llvm-project/pull/92741
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
https://github.com/robincaloudis ready_for_review https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
robincaloudis wrote: Even though this PR gives us the correct behavior, I do not think it is good. Since I am by no means an expert on Clang, a few questions arose * Why is `xor` tokenized as unary operator even though it's a word token in C? * How to properly distinguish between keyword difference in the C and C++ language? https://github.com/llvm/llvm-project/pull/92741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add space after a word token (PR #92741)
https://github.com/robincaloudis created
https://github.com/llvm/llvm-project/pull/92741
Closes https://github.com/llvm/llvm-project/issues/92688
>From 9e8c360029fb6789360ad4296e2f14098db76dd6 Mon Sep 17 00:00:00 2001
From: Robin Caloudis
Date: Mon, 20 May 2024 13:21:32 +0200
Subject: [PATCH 1/2] Test binary after unary operator
---
clang/unittests/Format/FormatTest.cpp | 7 +++
1 file changed, 7 insertions(+)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 6f57f10e12e88..ca0edd7b22630 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24545,6 +24545,13 @@ TEST_F(FormatTest, STLWhileNotDefineChed) {
"#endif // while");
}
+TEST_F(FormatTest, BinaryOperatorAfterUnaryOperator) {
+ verifyFormat("void test(void) {\n"
+ " static void (*xor)(uint8_t *, size_t, uint8_t);\n"
+ " xor = resolve_xor_x86();\n"
+ "}");
+}
+
TEST_F(FormatTest, OperatorSpacing) {
FormatStyle Style = getLLVMStyle();
Style.PointerAlignment = FormatStyle::PAS_Right;
>From 76e17eee617ee4ec9fb2562579a38c60cce0f76a Mon Sep 17 00:00:00 2001
From: Robin Caloudis
Date: Mon, 20 May 2024 13:21:54 +0200
Subject: [PATCH 2/2] Support binary after unary operator
---
clang/lib/Format/TokenAnnotator.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index 7c4c76a91f2c5..7786b85e8a1fc 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5280,7 +5280,8 @@ bool TokenAnnotator::spaceRequiredBefore(const
AnnotatedLine ,
// handled.
if (Left.is(tok::amp) && Right.is(tok::r_square))
return Style.SpacesInSquareBrackets;
-return Style.SpaceAfterLogicalNot && Left.is(tok::exclaim);
+return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
+ Right.is(TT_BinaryOperator);
}
// If the next token is a binary operator or a selector name, we have
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
