This is an archive of the discontinued LLVM Phabricator instance.

Add `CharLiteral` to SyntaxTree
ClosedPublic

Authored by eduucaldas on Jun 22 2020, 9:41 AM.

Diff Detail

Event Timeline

eduucaldas created this revision.Jun 22 2020, 9:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2020, 9:41 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
eduucaldas marked 2 inline comments as done.Jun 23 2020, 1:55 AM
eduucaldas added inline comments.
clang/unittests/Tooling/Syntax/TreeTest.cpp
1270

Rename to CharacterLiteralUtf?

1304

Rename to CharacterLiteralUtf8?

gribozavr2 accepted this revision.Jun 24 2020, 3:19 AM

LGTM after new tests are added.

clang/unittests/Tooling/Syntax/TreeTest.cpp
1244

Could you add tests for escape sequences ('\n', '\x20' etc.), and non-ASCII characters, here, and below in Unicode literals?

1270

Either name is good, I think.

1304

Either name is good, I think.

This revision is now accepted and ready to land.Jun 24 2020, 3:19 AM
eduucaldas marked 3 inline comments as done.

Add tests for unicode characters

This revision was automatically updated to reflect the committed changes.

Hello Eduardo,

sorry, but one of your these commits break the clang unit tests on the following builders:

with the following errors:

FAILED: tools/clang/unittests/Tooling/Syntax/CMakeFiles/SyntaxTests.dir/TreeTest.cpp.obj 
C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\clang\unittests\Tooling\Syntax -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\include -Itools\clang\include -Iinclude -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googletest\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googlemock\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2 /DNDEBUG    /EHs-c- /GR- -std:c++14 /showIncludes /Fotools\clang\unittests\Tooling\Syntax\CMakeFiles\SyntaxTests.dir\TreeTest.cpp.obj /Fdtools\clang\unittests\Tooling\Syntax\CMakeFiles\SyntaxTests.dir\ /FS -c C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2017: illegal escape sequence
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ')' before identifier 'n'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2660: 'testing::internal::GetBoolAssertionFailureMessage': function does not take 2 arguments
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googletest\include\gtest/internal/gtest-internal.h(226): note: see declaration of 'testing::internal::GetBoolAssertionFailureMessage'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'testing::internal::AssertHelper'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): note: No constructor could take the source type, or constructor overload resolution was ambiguous
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ';' before identifier 'n'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2059: syntax error: ')'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2065: 'n': undeclared identifier
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ';' before identifier 'SyntaxTree'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2065: 'SyntaxTree': undeclared identifier
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2143: syntax error: missing ';' before 'string'

see more details here: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/18313/steps/test-check-clang-unit/logs/stdio

I mean the following commits:
*221d7bbe49cceb0e408f0f46d9f8371e6c9fee2c* https://reviews.llvm.org/D82312
*466e8b7ea6e162d48cac42ccda210bdeb11080e3* https://reviews.llvm.org/D82360
*7b404b6d003181e990f53d27866ee98d5151c4f3* https://reviews.llvm.org/D82318

Would you take care of this problem?

Hello Eduardo,

sorry, but one of your these commits break the clang unit tests on the following builders:

with the following errors:

FAILED: tools/clang/unittests/Tooling/Syntax/CMakeFiles/SyntaxTests.dir/TreeTest.cpp.obj 
C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\clang\unittests\Tooling\Syntax -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\include -Itools\clang\include -Iinclude -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googletest\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googlemock\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2 /DNDEBUG    /EHs-c- /GR- -std:c++14 /showIncludes /Fotools\clang\unittests\Tooling\Syntax\CMakeFiles\SyntaxTests.dir\TreeTest.cpp.obj /Fdtools\clang\unittests\Tooling\Syntax\CMakeFiles\SyntaxTests.dir\ /FS -c C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2017: illegal escape sequence
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ')' before identifier 'n'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2660: 'testing::internal::GetBoolAssertionFailureMessage': function does not take 2 arguments
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\utils\unittest\googletest\include\gtest/internal/gtest-internal.h(226): note: see declaration of 'testing::internal::GetBoolAssertionFailureMessage'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'testing::internal::AssertHelper'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): note: No constructor could take the source type, or constructor overload resolution was ambiguous
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ';' before identifier 'n'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2059: syntax error: ')'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2065: 'n': undeclared identifier
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1665): error C2146: syntax error: missing ';' before identifier 'SyntaxTree'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2065: 'SyntaxTree': undeclared identifier
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\clang\unittests\Tooling\Syntax\TreeTest.cpp(1639): error C2143: syntax error: missing ';' before 'string'

see more details here: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/18313/steps/test-check-clang-unit/logs/stdio

I mean the following commits:
*221d7bbe49cceb0e408f0f46d9f8371e6c9fee2c* https://reviews.llvm.org/D82312
*466e8b7ea6e162d48cac42ccda210bdeb11080e3* https://reviews.llvm.org/D82360
*7b404b6d003181e990f53d27866ee98d5151c4f3* https://reviews.llvm.org/D82318

Would you take care of this problem?

Sorry for that. Yes, I'll take care this afternoon. I don't have a computer with me now

@vvereschaka Sorry about that. It looks like a bug in MSVC. I implemented a workaround in https://reviews.llvm.org/D82636 -- please review if you have time.