This refactoring was waiting on converting LLVM to C++17.
Leave StringView.h and cleanup around for subsequent cleanup.
Paths
| Differential D148384
[Demangle] replace use of llvm::StringView w/ std::string_view ClosedPublic Authored by nickdesaulniers on Apr 14 2023, 3:13 PM.
Details Summary This refactoring was waiting on converting LLVM to C++17. Leave StringView.h and cleanup around for subsequent cleanup.
Diff Detail
Event Timelinenickdesaulniers added a parent revision: D148375: [StringView] remove consumeFront.Apr 14 2023, 3:14 PM Comment Actions LLVMDemangle is used by bolt, clang, lld, lldb. You may want to build all of them to ensure all in-tree users have been migrated. This revision is now accepted and ready to land.Apr 14 2023, 3:24 PM Comment Actions
Oops! Wasn't buildnig bolt or lldb; thanks for pointing that out! This revision is now accepted and ready to land.Apr 14 2023, 3:41 PM This revision was landed with ongoing or failed builds.Apr 14 2023, 3:50 PM Closed by commit rG3e559509b426: [Demangle] replace use of llvm::StringView w/ std::string_view (authored by nickdesaulniers). · Explain Why This revision was automatically updated to reflect the committed changes. Comment Actions These windows buildbot failures are tough to make out: https://lab.llvm.org/buildbot/#/builders/127/builds/46749/steps/4/logs/stdio [2/1778] Building CXX object lib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj FAILED: lib/Demangle/CMakeFiles/LLVMDemangle.dir/DLangDemangle.cpp.obj C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\Demangle -IC:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle -Iinclude -IC:\b\slave\sanitizer-windows\llvm-project\llvm\include /DWIN32 /D_WINDOWS /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Zi /Oi /bigobj /permissive- /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 -wd5105 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2 /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Folib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj /Fdlib\Demangle\CMakeFiles\LLVMDemangle.dir\LLVMDemangle.pdb /FS -c C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\DLangDemangle.cpp C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111): error C2664: 'void *memcpy(void *,const void *,size_t)': cannot convert argument 2 from 'std::_String_view_iterator<_Traits>' to 'const void *' with [ _Traits=std::char_traits<char> ] C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\include\vcruntime_string.h(43): note: see declaration of 'memcpy' So something crazy is going on with std::string_view::iterator and implicit conversion to void*? Is this an issue in Windows C++17 support? EDIT: also: C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2403): error C2678: binary '-': no operator found which takes a left-hand operand of type 'std::_String_view_iterator<_Traits>' (or there is no acceptable conversion) C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2380): warning C4477: 'printf' : format string '%.*s' requires an argument of type 'char *', but variadic argument 3 has type 'std::_String_view_iterator<_Traits>' C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2388): warning C4477: 'printf' : format string '%.*s' requires an argument of type 'char *', but variadic argument 3 has type 'std::_String_view_iterator<_Traits>' C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\ItaniumDemangle.cpp(82): warning C4477: 'fprintf' : format string '%.*s' requires an argument of type 'char *', but variadic argument 2 has type 'std::_String_view_iterator<_Traits>' Comment Actions
Perhaps: diff --git a/llvm/lib/Demangle/ItaniumDemangle.cpp b/llvm/lib/Demangle/ItaniumDemangle.cpp index 4bc00c5144a4..7a7bcd36fc68 100644 --- a/llvm/lib/Demangle/ItaniumDemangle.cpp +++ b/llvm/lib/Demangle/ItaniumDemangle.cpp @@ -79,7 +79,7 @@ struct DumpVisitor { void printStr(const char *S) { fprintf(stderr, "%s", S); } void print(std::string_view SV) { - fprintf(stderr, "\"%.*s\"", (int)SV.size(), SV.begin()); + fprintf(stderr, "\"%.*s\"", (int)SV.size(), &*SV.begin()); } void print(const Node *N) { if (N) diff --git a/llvm/lib/Demangle/MicrosoftDemangle.cpp b/llvm/lib/Demangle/MicrosoftDemangle.cpp index 15cb0e7d8864..5204c24164d6 100644 --- a/llvm/lib/Demangle/MicrosoftDemangle.cpp +++ b/llvm/lib/Demangle/MicrosoftDemangle.cpp @@ -2377,7 +2377,7 @@ void Demangler::dumpBackReferences() { T->output(OB, OF_Default); std::string_view B = OB; - std::printf(" [%d] - %.*s\n", (int)I, (int)B.size(), B.begin()); + std::printf(" [%d] - %.*s\n", (int)I, (int)B.size(), &*B.begin()); } std::free(OB.getBuffer()); @@ -2386,7 +2386,7 @@ void Demangler::dumpBackReferences() { std::printf("%d name backreferences\n", (int)Backrefs.NamesCount); for (size_t I = 0; I < Backrefs.NamesCount; ++I) { std::printf(" [%d] - %.*s\n", (int)I, (int)Backrefs.Names[I]->Name.size(), - Backrefs.Names[I]->Name.begin()); + &*Backrefs.Names[I]->Name.begin()); } if (Backrefs.NamesCount > 0) std::printf("\n"); @@ -2400,7 +2400,7 @@ char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled, std::string_view Name{MangledName}; SymbolNode *AST = D.parse(Name); if (!D.Error && NMangled) - *NMangled = Name.begin() - MangledName; + *NMangled = Name.begin() - &*MangledName; if (Flags & MSDF_DumpBackrefs) D.dumpBackReferences(); ?
Comment Actions It looks like this breaks the build: https://lab.llvm.org/buildbot#builders/119/builds/12865 https://lab.llvm.org/buildbot#builders/123/builds/18388 https://lab.llvm.org/buildbot#builders/60/builds/11628 I'm pretty new with LLVM so I'll leave reverting the change to someone else, but I figured I should give you a heads up. Comment Actions
I am unsure about the Windows issue and find it difficult to repair... I'm testing a revert commit to not leave issues to the weekend. Comment Actions
Ah, I see what happened. Those 3 builds had my follow up commit https://reviews.llvm.org/D148392, BUT the initial broken builds stopped producing diagnostics after the initial set of diagnostics that I fixed; kind of like how clang stops emitting diagnostics after the first 20. You can fix the 20, but there might be more to fix. That's what happened here. Specifically, I see 3 new diagnostics that weren't addressed in https://reviews.llvm.org/D148392:
I'll reopen this with https://reviews.llvm.org/D148392 squashed, plus the above three issues fixed, then get a colleague with a windows machine to help verify.
That was the right thing to do, thank you. This revision is now accepted and ready to land.Apr 17 2023, 10:16 AM
Comment Actions
This revision is now accepted and ready to land.Apr 17 2023, 10:41 AM Comment Actions Creating https://reviews.llvm.org/D148546 instead; arcanist really doesn't like how I've reopened this review. This revision is now accepted and ready to land.Apr 17 2023, 10:54 AM Comment Actions Let's use D148546 as the reland (sorry, couldn't get arcanist to work to reuse this instance of code review reopened).
Revision Contents
Diff 513762 llvm/include/llvm/Demangle/ItaniumDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/include/llvm/Demangle/Utility.h
llvm/lib/Demangle/DLangDemangle.cpp
llvm/lib/Demangle/ItaniumDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
llvm/lib/Demangle/RustDemangle.cpp
llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
llvm/unittests/Demangle/ItaniumDemangleTest.cpp
llvm/unittests/Demangle/OutputBufferTest.cpp
|
Odd layering...