cor3ntin created this revision.
Herald added a subscriber: dexonsmith.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
According to The Unicode standard, line feed and vertical tab
should be considered line breaks.
See notably http://unicode.org/reports/tr14/#BK
In addition, the C++ standard stipulates:
> If there is a form-feed or a vertical-tab character in such a comment,
> only whitespace characters shall appear between it
> and the new-line that terminates the comment; no diagnostic is required.
Given the amount of `??` I found in the test, it isn't clear that
it was intended for this characters not to be considered
vertical spaces.
This came up in SG16/WG21 in the context of
`https://wg21.link/p2348`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108742
Files:
clang/lib/Basic/CharInfo.cpp
clang/unittests/Basic/CharInfoTest.cpp
clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
clang/unittests/Lex/LexerTest.cpp
Index: clang/unittests/Lex/LexerTest.cpp
===================================================================
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -493,10 +493,11 @@
EXPECT_TRUE(hasNewLineEscaped("\\\r"));
EXPECT_TRUE(hasNewLineEscaped("\\\n"));
+ EXPECT_TRUE(hasNewLineEscaped("\\\t"));
+ EXPECT_TRUE(hasNewLineEscaped("\\\v"));
EXPECT_TRUE(hasNewLineEscaped("\\\r\n"));
EXPECT_TRUE(hasNewLineEscaped("\\\n\r"));
- EXPECT_TRUE(hasNewLineEscaped("\\ \t\v\f\r"));
- EXPECT_TRUE(hasNewLineEscaped("\\ \t\v\f\r\n"));
+ EXPECT_TRUE(hasNewLineEscaped("\\ \t\r\n"));
EXPECT_FALSE(hasNewLineEscaped("\\\r\r"));
EXPECT_FALSE(hasNewLineEscaped("\\\r\r\n"));
Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===================================================================
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -162,29 +162,15 @@
ASSERT_FALSE(minimizeSourceToDependencyDirectives(
"#define MACRO(\t)\tcon \t tent\t", Out));
EXPECT_STREQ("#define MACRO() con \t tent\n", Out.data());
-
- ASSERT_FALSE(minimizeSourceToDependencyDirectives(
- "#define MACRO(\f)\fcon \f tent\f", Out));
- EXPECT_STREQ("#define MACRO() con \f tent\n", Out.data());
-
- ASSERT_FALSE(minimizeSourceToDependencyDirectives(
- "#define MACRO(\v)\vcon \v tent\v", Out));
- EXPECT_STREQ("#define MACRO() con \v tent\n", Out.data());
-
- ASSERT_FALSE(minimizeSourceToDependencyDirectives(
- "#define MACRO \t\v\f\v\t con\f\t\vtent\v\f \v", Out));
- EXPECT_STREQ("#define MACRO con\f\t\vtent\n", Out.data());
}
TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgs) {
SmallVector<char, 128> Out;
ASSERT_FALSE(
- minimizeSourceToDependencyDirectives("#define MACRO(a \\\n"
+ minimizeSourceToDependencyDirectives("#define MACRO(a \\\n\\\v\\\f"
" )",
Out));
- EXPECT_STREQ("#define MACRO(a)\n", Out.data());
-
ASSERT_FALSE(
minimizeSourceToDependencyDirectives("#define MACRO(a, \\\n"
" b) \\\n"
Index: clang/unittests/Basic/CharInfoTest.cpp
===================================================================
--- clang/unittests/Basic/CharInfoTest.cpp
+++ clang/unittests/Basic/CharInfoTest.cpp
@@ -17,8 +17,8 @@
using namespace charinfo;
EXPECT_EQ((unsigned)CHAR_SPACE, InfoTable[(unsigned)' ']);
EXPECT_EQ((unsigned)CHAR_HORZ_WS, InfoTable[(unsigned)'\t']);
- EXPECT_EQ((unsigned)CHAR_HORZ_WS, InfoTable[(unsigned)'\f']); // ??
- EXPECT_EQ((unsigned)CHAR_HORZ_WS, InfoTable[(unsigned)'\v']); // ??
+ EXPECT_EQ((unsigned)CHAR_VERT_WS, InfoTable[(unsigned)'\f']);
+ EXPECT_EQ((unsigned)CHAR_VERT_WS, InfoTable[(unsigned)'\v']);
EXPECT_EQ((unsigned)CHAR_VERT_WS, InfoTable[(unsigned)'\n']);
EXPECT_EQ((unsigned)CHAR_VERT_WS, InfoTable[(unsigned)'\r']);
EXPECT_EQ((unsigned)CHAR_UNDER, InfoTable[(unsigned)'_']);
@@ -101,8 +101,6 @@
EXPECT_TRUE(isHorizontalWhitespace(' '));
EXPECT_TRUE(isHorizontalWhitespace('\t'));
- EXPECT_TRUE(isHorizontalWhitespace('\f')); // ??
- EXPECT_TRUE(isHorizontalWhitespace('\v')); // ??
EXPECT_FALSE(isHorizontalWhitespace('\n'));
EXPECT_FALSE(isHorizontalWhitespace('\r'));
@@ -123,9 +121,9 @@
EXPECT_FALSE(isVerticalWhitespace(' '));
EXPECT_FALSE(isVerticalWhitespace('\t'));
- EXPECT_FALSE(isVerticalWhitespace('\f')); // ??
- EXPECT_FALSE(isVerticalWhitespace('\v')); // ??
+ EXPECT_TRUE(isVerticalWhitespace('\f')); // ??
+ EXPECT_TRUE(isVerticalWhitespace('\v')); // ??
EXPECT_TRUE(isVerticalWhitespace('\n'));
EXPECT_TRUE(isVerticalWhitespace('\r'));
Index: clang/lib/Basic/CharInfo.cpp
===================================================================
--- clang/lib/Basic/CharInfo.cpp
+++ clang/lib/Basic/CharInfo.cpp
@@ -19,8 +19,8 @@
0 , 0 , 0 , 0 ,
// 8 BS 9 HT 10 NL 11 VT
//12 NP 13 CR 14 SO 15 SI
- 0 , CHAR_HORZ_WS, CHAR_VERT_WS, CHAR_HORZ_WS,
- CHAR_HORZ_WS, CHAR_VERT_WS, 0 , 0 ,
+ 0 , CHAR_HORZ_WS, CHAR_VERT_WS, CHAR_VERT_WS,
+ CHAR_VERT_WS, CHAR_VERT_WS, 0 , 0 ,
//16 DLE 17 DC1 18 DC2 19 DC3
//20 DC4 21 NAK 22 SYN 23 ETB
0 , 0 , 0 , 0 ,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
