[core] Remove backports of C++17 features #13993

guitargeek · 2023-11-01T02:15:18Z

Remove the std::string_view backport and others, because it's not necessary anymore since the minimum C++ standard is now C++17.

vgvassilev · 2023-11-01T06:38:53Z

Thank you for cleaning up that mess!

Axel-Naumann

Excellent, thanks for doing this! See comments, please! And let's make sure that the CI passes before merging :-)

Axel-Naumann · 2023-11-01T07:00:21Z

core/foundation/inc/ROOT/RWrap_libcpp_string_view.h

@@ -1,591 +0,0 @@
-// -*- C++ -*-


GitHub tells me that this file got emptied but isn't deleted. Is that intentional? Or a bug in GitHub's interface?

For me it shows it like this, so correctly deleted. Maybe a bug in the comparison interface

Axel-Naumann · 2023-11-01T07:00:58Z

core/foundation/inc/ROOT/libcpp_string_view.h

@@ -1,824 +0,0 @@
-// -*- C++ -*-


Same here: empty but file remains?

Axel-Naumann · 2023-11-01T07:02:56Z

core/base/inc/TString.h

@@ -309,15 +309,8 @@ friend std::strong_ordering operator<=>(const TString &s1, const TString &s2) {

   // Type conversion
   operator const char*() const { return GetPointer(); }
-#if (__cplusplus >= 201700L) && !defined(_MSC_VER) && (!defined(__clang_major__) || __clang_major__ > 5)


How certain are we that Windows doesn't need this anymore? I guess the CI will tell...

Okay, it's still needed. I don't like that it's there, but that would be another story

Axel-Naumann · 2023-11-01T07:05:37Z

tmva/tmva/inc/TMVA/DNN/Architectures/Cuda/CudaMatrix.h

-#ifdef R__HAS_STD_STRING_VIEW
-#ifndef R__CUDA_HAS_STD_STRING_VIEW
-#undef R__HAS_STD_STRING_VIEW
-#define R__HAS_STD_EXPERIMENTAL_STRING_VIEW


Isn't anything using these preprocessor defines?

My gut feeling here is "pretty sure", but maybe @lmoneta can also comment

I think these are not needed anymore. There were there to fix conflict happening in past Cuda version when including string_view backport

Do we have any CUDA build in the CI?

No, we don't have it!

Sister PR of root-project/root#13993.

github-actions · 2023-11-01T07:30:57Z

Test Results

        9 files         9 suites 1d 10h 48m 12s ⏱️
  2 444 tests   2 444 ✔️ 0 💤 0 ❌
21 837 runs 21 837 ✔️ 0 💤 0 ❌

Results for commit 671be60.

♻️ This comment has been updated with latest results.

guitargeek · 2023-11-01T07:44:56Z

Thanks @Axel-Naumann for the green light! FYI, I have added one more commit that replaces the usage of the RStringView header with <string_view>.

Also, can you please approve the sister PR in roottest to fix the CI failures? Thanks!
root-project/roottest#1031

guitargeek · 2023-11-01T07:45:48Z

Oh, and the Windows 10 failures seem unrelated to this PR, right @bellenot? In any case, this PR should not be merged before we see any green Windows build

bellenot · 2023-11-01T07:53:50Z

@guitargeek the only failure I see (on Jenkins) was due to Out of diskspace error. But it's running on another node now, so we'll see. (and I'm cleaning up the Windows nodes right now)

phsft-bot · 2023-11-01T08:29:38Z

Build failed on windows10/default.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2023-11-01T08:24:00.367Z] C:\build\workspace\root-pullrequests-build\root\tree\tree\src\TChain.cxx(377,81): error C2440: '<function-style-cast>': cannot convert from 'TString' to 'std::string' [C:\build\workspace\root-pullrequests-build\build\tree\tree\Tree.vcxproj]

phsft-bot · 2023-11-01T13:09:57Z

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

phsft-bot · 2023-11-01T13:56:58Z

Build failed on windows10/default.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(1400,51): error C2664: 'bool TClassEdit::IsStdPair(std::string_view)': cannot convert argument 1 from 'TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(1400,15): error C2737: 'ispair': const object must be initialized [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(1464,46): error C2665: 'TClassEdit::IsSTLCont': none of the 2 overloads could convert all the argument types [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(1495,7): error C2664: 'bool TClassEdit::IsArtificial(std::string_view)': cannot convert argument 1 from 'TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(1532,7): error C2664: 'bool TClassEdit::IsArtificial(std::string_view)': cannot convert argument 1 from 'TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(4098,7): error C2664: 'bool TClassEdit::IsStdPair(std::string_view)': cannot convert argument 1 from 'TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:05.150Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TClass.cxx(6587,51): error C2665: 'TClassEdit::IsSTLCont': none of the 2 overloads could convert all the argument types [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:30.080Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TSchemaRule.cxx(459,59): error C2664: 'void TClassEdit::GetNormalizedName(std::string &,std::string_view)': cannot convert argument 2 from 'const TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:30.080Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TSchemaRule.cxx(477,59): error C2664: 'void TClassEdit::GetNormalizedName(std::string &,std::string_view)': cannot convert argument 2 from 'const TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]
[2023-11-01T13:56:30.665Z] C:\build\workspace\root-pullrequests-build\root\core\meta\src\TSchemaRuleSet.cxx(234,10): error C2664: 'bool TClassEdit::IsStdPair(std::string_view)': cannot convert argument 1 from 'const TString' to 'std::string_view' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]

phsft-bot · 2023-11-01T14:04:42Z

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

phsft-bot · 2023-11-01T14:16:21Z

Build failed on windows10/default.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2023-11-01T14:16:19.373Z] LINK : fatal error LNK1104: cannot open file 'C:\build\workspace\root-pullrequests-build\build\bin\libCore.dll' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]

Remove the `std::string_view` backport, because it's not necessary anymore since the minimum C++ standard is now C++17.

Since C++17 is now the minimum standard, we can use `<string_view>`.

phsft-bot · 2023-11-01T14:56:18Z

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

guitargeek · 2023-11-01T16:30:03Z

Windows is green now

Sister PR of root-project/root#13993.

phsft-bot · 2023-11-01T16:57:19Z

Build failed on mac12arm/cxx20.
Running on macphsft26.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-11-01T16:57:18.225Z] CMake Error at /Users/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1126 (message):

phsft-bot · 2023-11-01T17:37:29Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2023-11-01T17:37:28.836Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1126 (message):

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

As these operators are not needed anymore we can get rid of them. Related to issue #13993 and #14244

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

As these operators are not needed anymore we can get rid of them. Related to issue #13993 and #14244

guitargeek self-assigned this Nov 1, 2023

guitargeek requested review from bellenot, lmoneta, sitongan, pcanal, Axel-Naumann and vepadulano as code owners November 1, 2023 02:15

guitargeek changed the title ~~[core] Remove std::string_view backport~~ [core] Remove backports of C++17 features Nov 1, 2023

Axel-Naumann approved these changes Nov 1, 2023

View reviewed changes

guitargeek added a commit to guitargeek/roottest that referenced this pull request Nov 1, 2023

Adapt to removal of internal TStringView

e47a9b0

Sister PR of root-project/root#13993.

guitargeek mentioned this pull request Nov 1, 2023

Adapt to removal of internal TStringView root-project/roottest#1031

Merged

guitargeek force-pushed the string_view_backport branch from bb92818 to 5e864e5 Compare November 1, 2023 07:35

guitargeek requested review from jblomer, eguiraud, hageboeck, gganis and couet as code owners November 1, 2023 07:35

guitargeek force-pushed the string_view_backport branch 2 times, most recently from 35b6a40 to 671be60 Compare November 1, 2023 07:43

root-project deleted a comment from phsft-bot Nov 1, 2023

guitargeek force-pushed the string_view_backport branch from 671be60 to e74a3c2 Compare November 1, 2023 13:09

lmoneta approved these changes Nov 1, 2023

View reviewed changes

guitargeek force-pushed the string_view_backport branch from e74a3c2 to 280830a Compare November 1, 2023 14:04

guitargeek added 4 commits November 1, 2023 15:55

[core] Remove std::string_view backport

e1734d4

Remove the `std::string_view` backport, because it's not necessary anymore since the minimum C++ standard is now C++17.

[core] Remove std::apply backport

991b1fa

[cmake] Remove unused checks for C++ features

d70dfbc

[core] Replace usage of ROOT/RStringView.hxx header inside ROOT

4f5cbf8

Since C++17 is now the minimum standard, we can use `<string_view>`.

guitargeek force-pushed the string_view_backport branch from 280830a to 4f5cbf8 Compare November 1, 2023 14:56

guitargeek merged commit bb33e9c into root-project:master Nov 1, 2023
12 of 14 checks passed

guitargeek deleted the string_view_backport branch November 1, 2023 16:30

guitargeek added a commit to root-project/roottest that referenced this pull request Nov 1, 2023

Adapt to removal of internal TStringView

a377631

Sister PR of root-project/root#13993.

guitargeek mentioned this pull request Dec 18, 2023

Remove std::string_view comparison operators #14245

Merged

1 task

bachandi added a commit to bachandi/root that referenced this pull request Dec 18, 2023

Remove std::string_view comparison operators.

0e327ec

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

guitargeek pushed a commit that referenced this pull request Dec 20, 2023

Remove std::string_view comparison operators.

1127826

As these operators are not needed anymore we can get rid of them. Related to issue #13993 and #14244

maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024

Remove std::string_view comparison operators.

0463f24

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

vgvassilev pushed a commit to vgvassilev/root that referenced this pull request Feb 8, 2024

Remove std::string_view comparison operators.

70a09b8

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

guitargeek pushed a commit to guitargeek/root that referenced this pull request Apr 8, 2024

Remove std::string_view comparison operators.

bc1ee18

As these operators are not needed anymore we can get rid of them. Related to issue root-project#13993 and root-project#14244

guitargeek pushed a commit that referenced this pull request Apr 9, 2024

Remove std::string_view comparison operators.

cad191f

As these operators are not needed anymore we can get rid of them. Related to issue #13993 and #14244

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Remove backports of C++17 features #13993

[core] Remove backports of C++17 features #13993

guitargeek commented Nov 1, 2023 •

edited

Loading

vgvassilev commented Nov 1, 2023

Axel-Naumann left a comment

Axel-Naumann Nov 1, 2023

guitargeek Nov 1, 2023

Axel-Naumann Nov 1, 2023

Axel-Naumann Nov 1, 2023

guitargeek Nov 1, 2023

guitargeek Nov 1, 2023

Axel-Naumann Nov 1, 2023

guitargeek Nov 1, 2023

lmoneta Nov 1, 2023

pcanal Nov 1, 2023

Axel-Naumann Nov 1, 2023

lmoneta Nov 2, 2023

github-actions bot commented Nov 1, 2023 •

edited

Loading

guitargeek commented Nov 1, 2023

guitargeek commented Nov 1, 2023

bellenot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

guitargeek commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

[core] Remove backports of C++17 features #13993

[core] Remove backports of C++17 features #13993

Conversation

guitargeek commented Nov 1, 2023 • edited Loading

vgvassilev commented Nov 1, 2023

Axel-Naumann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 1, 2023 • edited Loading

Test Results

guitargeek commented Nov 1, 2023

guitargeek commented Nov 1, 2023

bellenot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

Errors:

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

Errors:

phsft-bot commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

Errors:

phsft-bot commented Nov 1, 2023

guitargeek commented Nov 1, 2023

phsft-bot commented Nov 1, 2023

Errors:

phsft-bot commented Nov 1, 2023

Errors:

guitargeek commented Nov 1, 2023 •

edited

Loading

github-actions bot commented Nov 1, 2023 •

edited

Loading