Update codebase from boost::string_view into std::string_view

[ckeshava]

This PR seeks to solve issue #3435

High Level Overview of Change

Context of Change

Type of Change

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [x] Refactor (non-breaking change that only restructures code)
• [ ] Tests (You added tests for code that already exists, or your new feature included in this PR)
• [ ] Documentation Updates
• [ ] Release

Before: The codebase makes use of boost:string_view for cheap manipulation of strings.

After: This PR uses std::string_view instead of boost::string_view to perform similar operations.

Can I avoid making expensive conversions into std::string inside Ping.cpp for jss::ip and jss::username response fields?

The http_request_type has a dependency on boost. Is it okay to occasionally convert from boost's string_view into std::string_view? I'm not sure about the performance implications.

[scottschurr]

It seems like you're running into some problems interfacing with boost::beast, since it is inclined to use boost::string_view instead of std:::string_view. Boost must support many users who are stuck on C++11, which doesn't have std::string_view as part of the library. So this limitation makes a lot of sense to the Boost community.

I noticed in the Boost documentation that there's a configuration preprocessor macro that may be useful: BOOST_BEAST_USE_STD_STRING_VIEW. That macro appears to be available in Boost 1.77, which looks like the oldest version of Boost we currently support. Here's the 1.77 documentation: https://www.boost.org/doc/libs/1_77_0/libs/beast/doc/html/beast/config/configuration_preprocessor_defin.html

What I don't know is how many wrenches that change would throw into the Conan build process. Perhaps @thejohnfreeman can advise us on how much trouble using that configuration preprocessor macro would cause, or if it's even possible.

[ckeshava]

addressed PR comments. tagging @thejohnfreeman to get his feedback on Boost's macro usage and it's impact on the conan build process.

[thejohnfreeman]

Sorry, I missed the earlier pings for my attention. It is not a problem to use the Boost macro. We already have to do something similar for std::result_of, which has instructions in BUILD.md.

If you want to enable / require the macro BOOST_BEAST_USE_STD_STRING_VIEW, then the instructions will be similar:

• Add the macro to conf.tools.build:cxxflags in the Conan profile. This is the future-proof method.
• Add it to env.CXXFLAGS too. This is the backwards-compatible method.
• Do not add it to CFLAGS if you can build without it.
• Add it to options.boost:extra_b2_flags in case it changes the ABI of the non-header-only Boost libraries we link against.

[ckeshava]

Thanks for the tip John. I executed the above commands, but I ran into an issue here:

➜  cmake-build-debug git:(updateStringView) ✗ cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Debug ..  
-- Using Conan toolchain: /Users/ckeshavabs/personal-code/rippled/cmake-build-debug/build/generators/conan_toolchain.cmake
-- Conan toolchain: Setting CMAKE_POSITION_INDEPENDENT_CODE=ON (options.fPIC)
-- Conan toolchain: C++ Standard 20 with extensions OFF
-- Conan toolchain: Setting BUILD_SHARED_LIBS = OFF
-- The C compiler identification is AppleClang 14.0.3.14030022
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- gch:c966149aeb0daa028cd23c481c0215a15cca68f8
-- Using 5 cores for ExternalProject builds.
-- rippled version: 1.10.1
-- docker NOT found -- won't be able to build containers for packaging
-- Conan: Component target declared 'Boost::diagnostic_definitions'
-- Conan: Component target declared 'Boost::disable_autolinking'
-- Conan: Component target declared 'Boost::dynamic_linking'
-- Conan: Component target declared 'Boost::headers'
-- Conan: Component target declared 'Boost::boost'
-- Conan: Component target declared 'boost::_libboost'
-- Conan: Component target declared 'Boost::atomic'
-- Conan: Component target declared 'Boost::container'
-- Conan: Component target declared 'Boost::context'
-- Conan: Component target declared 'Boost::date_time'
-- Conan: Component target declared 'Boost::exception'
-- Conan: Component target declared 'Boost::math'
-- Conan: Component target declared 'Boost::math_c99'
-- Conan: Component target declared 'Boost::math_c99f'
-- Conan: Component target declared 'Boost::math_c99l'
-- Conan: Component target declared 'Boost::math_tr1'
-- Conan: Component target declared 'Boost::math_tr1f'
-- Conan: Component target declared 'Boost::math_tr1l'
-- Conan: Component target declared 'Boost::program_options'
-- Conan: Component target declared 'Boost::regex'
-- Conan: Component target declared 'Boost::serialization'
-- Conan: Component target declared 'Boost::stacktrace'
-- Conan: Component target declared 'Boost::stacktrace_addr2line'
-- Conan: Component target declared 'Boost::stacktrace_backtrace'
-- Conan: Component target declared 'Boost::stacktrace_basic'
-- Conan: Component target declared 'Boost::stacktrace_noop'
-- Conan: Component target declared 'Boost::system'
-- Conan: Component target declared 'Boost::test'
-- Conan: Component target declared 'Boost::test_exec_monitor'
-- Conan: Component target declared 'Boost::wserialization'
-- Conan: Component target declared 'Boost::chrono'
-- Conan: Component target declared 'Boost::filesystem'
-- Conan: Component target declared 'Boost::json'
-- Conan: Component target declared 'Boost::nowide'
-- Conan: Component target declared 'Boost::prg_exec_monitor'
-- Conan: Component target declared 'Boost::random'
-- Conan: Component target declared 'Boost::thread'
-- Conan: Component target declared 'Boost::timer'
-- Conan: Component target declared 'Boost::type_erasure'
-- Conan: Component target declared 'Boost::unit_test_framework'
-- Conan: Component target declared 'Boost::wave'
-- Conan: Component target declared 'Boost::contract'
-- Conan: Component target declared 'Boost::coroutine'
-- Conan: Component target declared 'Boost::fiber'
-- Conan: Component target declared 'Boost::fiber_numa'
-- Conan: Component target declared 'Boost::graph'
-- Conan: Component target declared 'Boost::iostreams'
-- Conan: Component target declared 'Boost::locale'
-- Conan: Component target declared 'Boost::log'
-- Conan: Component target declared 'Boost::log_setup'
-- Conan: Target declared 'boost::boost'
CMake Error at .debug/build/generators/cmakedeps_macros.cmake:39 (message):
  Library 'boost_log_setup' not found in package.  If 'boost_log_setup' is a
  system library, declare it with 'cpp_info.system_libs' property
Call Stack (most recent call first):
  .debug/build/generators/Boost-Target-debug.cmake:24 (conan_package_library_targets)
  .debug/build/generators/BoostTargets.cmake:26 (include)
  .debug/build/generators/BoostConfig.cmake:16 (include)
  Builds/CMake/deps/Boost.cmake:1 (find_package)
  CMakeLists.txt:69 (include)


-- Configuring incomplete, errors occurred!

Are we using CMake.patch_config_paths anywhere in the conan configuration? I found this comment to be relevant: https://github.com/conan-io/conan/issues/10816#issuecomment-1071247773

[thejohnfreeman]

You executed what commands? You've not included enough context to debug your situation. Please copy a full log that reproduces your error into a Gist (not a GitHub comment). CMake.patch_config_paths seems like a red herring; not sure why you're asking about it. We've never used it.

[ckeshava]

Ok, here is the complete gist: https://gist.github.com/ckeshava/a47fcf02624c95c7d8a9e3824c961cc9

I'm asking about Cmake. patch_config_paths because that configuration parameter is flagged as a possible red-herring in the comment thread linked in my previous comment.

To summarize, I updated the conan profile and generated new

conan profile update 'conf.tools.build:cxxflags+=["-DBOOST_BEAST_USE_STD_STRING_VIEW"]' default
conan profile update 'env.CXXFLAGS="-DBOOST_BEAST_USE_STD_STRING_VIEW"' default                
conan profile update 'options.boost:extra_b2_flags="define=BOOST_BEAST_USE_STD_STRING_VIEW

conan install .. --output-folder . --build missing --settings build_type=Debug # from within the build directory

The above error is generated in the last command

[thejohnfreeman]

"Red herring" means distraction, irrelevant. I don't know why you linked that comment, or what it has to do with this issue. I can see it mentions CMake.patch_config_paths, but this issue does not, our code does not and has never.

From your log:

[options]
boost:extra_b2_flags="define=BOOST_BEAST_USE_STD_STRING_VIEW"
[build_requires]
[env]
CFLAGS=-DBOOST_ASIO_HAS_STD_INVOKE_RESULT
CXXFLAGS=-DBOOST_BEAST_USE_STD_STRING_VIEW
[conf]
tools.build:cflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT']
tools.build:cxxflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT', '-DBOOST_BEAST_USE_STD_STRING_VIEW', '-DBOOST_BEAST_USE_STD_STRING_VIEW']

Look at what has happened here. You have added (twice) the string '-DBOOST_BEAST_USE_STD_STRING_VIEW' to the array [conf]tools.build:cxxflags, but you have replaced the strings [env]CXXFLAGS and [options]boost:extra_b2_flags instead of adding a definition to the lists in those strings. It looks as if you modified the profile without understanding what you were doing or what you were trying to accomplish. You need a profile that looks like this:

[options]
boost:extra_b2_flags="define=BOOST_ASIO_HAS_STD_INVOKE_RESULT define=BOOST_BEAST_USE_STD_STRING_VIEW"
[build_requires]
[env]
CFLAGS=-DBOOST_ASIO_HAS_STD_INVOKE_RESULT -DBOOST_BEAST_USE_STD_STRING_VIEW
CXXFLAGS=-DBOOST_ASIO_HAS_STD_INVOKE_RESULT -DBOOST_BEAST_USE_STD_STRING_VIEW
[conf]
tools.build:cflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT', '-DBOOST_BEAST_USE_STD_STRING_VIEW']
tools.build:cxxflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT', '-DBOOST_BEAST_USE_STD_STRING_VIEW']

That was not the cause of the error you shared, but it would have likely caused an error later in the pipeline. Your error suggests a corruption in the generated CMake modules that makes CMake look for Boost binaries in a cache directory that you deleted. You should delete your bulid directory (cmake-build-debug), fix your Conan profile, and start over from the top.

[ckeshava]

Okay thanks for the help, I have followed your suggestions. I fixed my conan profile, deleted the cmake-build-debug folder.

I deleted the conan cache with rm -rf ~/.conan/data I executed this command from my buidl directory: conan install .. --output-folder . --build missing --settings build_type=Debug -> This completed successfully (I have included the last few lines of this command's output below)

This command fails: cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Debug ..

Snapshot of the logs:

soci/4.0.3 package(): Packaged 1 '.txt' file: LICENSE_1_0.txt
soci/4.0.3 package(): Packaged 49 '.h' files
soci/4.0.3 package(): Packaged 2 '.a' files: libsoci_sqlite3.a, libsoci_core.a
soci/4.0.3: Package 'd83c95fc80dd73000c607492f399917b2447b7a7' created
soci/4.0.3: Created package revision f47470056797bc0450e227dc953b72e4
conanfile.py (xrpl/1.10.1): WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
conanfile.py (xrpl/1.10.1): Generator 'CMakeDeps' calling 'generate()'
conanfile.py (xrpl/1.10.1): Generator txt created conanbuildinfo.txt
conanfile.py (xrpl/1.10.1): Calling generate()
conanfile.py (xrpl/1.10.1): WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
conanfile.py (xrpl/1.10.1): Preset 'debug' added to CMakePresets.json. Invoke it manually using 'cmake --preset debug'
conanfile.py (xrpl/1.10.1): If your CMake version is not compatible with CMakePresets (<3.19) call cmake like: 'cmake <path> -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE=/Users/ckeshavabs/personal-code/rippled/cmake-build-debug/build/generators/conan_toolchain.cmake -DCMAKE_POLICY_DEFAULT_CMP0091=NEW -DCMAKE_BUILD_TYPE=Debug'
conanfile.py (xrpl/1.10.1): Aggregating env generators
conanfile.py (xrpl/1.10.1): Generated conaninfo.txt
conanfile.py (xrpl/1.10.1): Generated graphinfo
➜  cmake-build-debug git:(updateStringView) ✗ cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Debug ..  
-- Using Conan toolchain: /Users/ckeshavabs/personal-code/rippled/cmake-build-debug/build/generators/conan_toolchain.cmake
-- Conan toolchain: Setting CMAKE_POSITION_INDEPENDENT_CODE=ON (options.fPIC)
-- Conan toolchain: C++ Standard 20 with extensions OFF
-- Conan toolchain: Setting BUILD_SHARED_LIBS = OFF
-- The C compiler identification is AppleClang 14.0.3.14030022
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- gch:c966149aeb0daa028cd23c481c0215a15cca68f8
-- Using 5 cores for ExternalProject builds.
-- rippled version: 1.10.1
-- docker NOT found -- won't be able to build containers for packaging
-- Conan: Component target declared 'Boost::diagnostic_definitions'
-- Conan: Component target declared 'Boost::disable_autolinking'
-- Conan: Component target declared 'Boost::dynamic_linking'
-- Conan: Component target declared 'Boost::headers'
-- Conan: Component target declared 'Boost::boost'
-- Conan: Component target declared 'boost::_libboost'
-- Conan: Component target declared 'Boost::atomic'
-- Conan: Component target declared 'Boost::container'
-- Conan: Component target declared 'Boost::context'
-- Conan: Component target declared 'Boost::date_time'
-- Conan: Component target declared 'Boost::exception'
-- Conan: Component target declared 'Boost::math'
-- Conan: Component target declared 'Boost::math_c99'
-- Conan: Component target declared 'Boost::math_c99f'
-- Conan: Component target declared 'Boost::math_c99l'
-- Conan: Component target declared 'Boost::math_tr1'
-- Conan: Component target declared 'Boost::math_tr1f'
-- Conan: Component target declared 'Boost::math_tr1l'
-- Conan: Component target declared 'Boost::program_options'
-- Conan: Component target declared 'Boost::regex'
-- Conan: Component target declared 'Boost::serialization'
-- Conan: Component target declared 'Boost::stacktrace'
-- Conan: Component target declared 'Boost::stacktrace_addr2line'
-- Conan: Component target declared 'Boost::stacktrace_backtrace'
-- Conan: Component target declared 'Boost::stacktrace_basic'
-- Conan: Component target declared 'Boost::stacktrace_noop'
-- Conan: Component target declared 'Boost::system'
-- Conan: Component target declared 'Boost::test'
-- Conan: Component target declared 'Boost::test_exec_monitor'
-- Conan: Component target declared 'Boost::wserialization'
-- Conan: Component target declared 'Boost::chrono'
-- Conan: Component target declared 'Boost::filesystem'
-- Conan: Component target declared 'Boost::json'
-- Conan: Component target declared 'Boost::nowide'
-- Conan: Component target declared 'Boost::prg_exec_monitor'
-- Conan: Component target declared 'Boost::random'
-- Conan: Component target declared 'Boost::thread'
-- Conan: Component target declared 'Boost::timer'
-- Conan: Component target declared 'Boost::type_erasure'
-- Conan: Component target declared 'Boost::unit_test_framework'
-- Conan: Component target declared 'Boost::wave'
-- Conan: Component target declared 'Boost::contract'
-- Conan: Component target declared 'Boost::coroutine'
-- Conan: Component target declared 'Boost::fiber'
-- Conan: Component target declared 'Boost::fiber_numa'
-- Conan: Component target declared 'Boost::graph'
-- Conan: Component target declared 'Boost::iostreams'
-- Conan: Component target declared 'Boost::locale'
-- Conan: Component target declared 'Boost::log'
-- Conan: Component target declared 'Boost::log_setup'
-- Conan: Target declared 'boost::boost'
CMake Error at .debug/build/generators/cmakedeps_macros.cmake:39 (message):
  Library 'boost_log_setup' not found in package.  If 'boost_log_setup' is a
  system library, declare it with 'cpp_info.system_libs' property
Call Stack (most recent call first):
  .debug/build/generators/Boost-Target-debug.cmake:24 (conan_package_library_targets)
  .debug/build/generators/BoostTargets.cmake:26 (include)
  .debug/build/generators/BoostConfig.cmake:16 (include)
  Builds/CMake/deps/Boost.cmake:1 (find_package)
  CMakeLists.txt:69 (include)


-- Configuring incomplete, errors occurred!
➜  cmake-build-debug git:(updateStringView) ✗ conan profile show default
Configuration for profile default:

[settings]
os=Macos
os_build=Macos
arch=armv8
arch_build=armv8
compiler=apple-clang
compiler.version=14
compiler.libcxx=libc++
build_type=Debug
compiler.cppstd=20
[options]
boost:extra_b2_flags="define=BOOST_ASIO_HAS_STD_INVOKE_RESULT define=BOOST_BEAST_USE_STD_STRING_VIEW"
[conf]
tools.build:cflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT', '-DBOOST_BEAST_USE_STD_STRING_VIEW']
tools.build:cxxflags=['-DBOOST_ASIO_HAS_STD_INVOKE_RESULT', '-DBOOST_BEAST_USE_STD_STRING_VIEW']
[build_requires]
[env]
CFLAGS=-DBOOST_ASIO_HAS_STD_INVOKE_RESULT -DBOOST_BEAST_USE_STD_STRING_VIEW
CXXFLAGS=-DBOOST_ASIO_HAS_STD_INVOKE_RESULT -DBOOST_BEAST_USE_STD_STRING_VIEW
➜  cmake-build-debug git:(updateStringView) ✗ 

[intelliot]

@ckeshava - have you made any progress on this?

[ckeshava]

@intelliot Sorry, I forgot about this PR, I'll work on it.

@thejohnfreeman Based on your suggestions above, should we expect all rippled validators to perform the conan profile updates manually? Can we make it easier for validators to set up this conan profile?

[thejohnfreeman]

I don't think we expect validators to build rippled. I think they use our packages. @legleux @manojsdoshi can you confirm?

Regardless, there are other profile changes we already require and document in the build instructions. This would just be one more instruction.

[legleux]

I don't think we expect validators to build rippled. I think they use our packages. @legleux @manojsdoshi can you confirm?

I think it's a fairly safe assumption that most validtators do not build from source and are installing via binary packages.

[ckeshava]

I have brought this branch up to speed, sorry about the delay.

I need to update the build instructions, I'll do that just before the merge. I'm concerned that it will unnecessarily add complexity to the build procedure otherwise.

[thejohnfreeman]

If you're not going to put the build instructions in this PR right now, can you please share them in a comment?

[ckeshava]

Yes, regarding the usage of BOOST_BEAST_USE_STD_STRING_VIEW macro in the conan build process: I believe it is required only in Role.cpp and OverlayImpl.cpp. It does not appear to affect other parts of the codebase.

The changes compile and work well even without the addition of this macro. I feel the changes to the build process could be put into another PR, what do you guys think?

[thejohnfreeman]

The definition of BOOST_BEAST_USE_STD_STRING_VIEW will need to be the same for all translation units whether the source file uses it directly or not.

It is expected that the current PR should compile with or without the definition. The difference is that with the definition, we should be able to get rid of manual conversions between boost::string_view and std::string_view because they should be the same type. Did you try that?

If we decide to take advantage of that difference, then the build instructions for adding that definition should be included in this PR. If we decide otherwise, then we should remain silent on the definition.

[ckeshava]

I believe I have addressed all of your comments @thejohnfreeman . I have updated the build instructions too.

I'm surprised that the Github CI passed all the tests. Without the BOOST_BEAST_USE_STD_STRING_VIEW macro, my compiler threw a few errors.

[thejohnfreeman]

For your consideration: Three reasons to pass std::string_view by value

[ckeshava]

@thejohnfreeman thanks for the article. I don't agree with the third point in the blog post, but the first two were pretty satisfactory.

The third example seems contrived, it doesn't feel correct when the size data member (which should be ideally private), is written into by an external function byRef

[scottschurr]

@ckeshava, while working on this review I noticed that there are still some std::string_view const& parameters. I think that the article from @thejohnfreeman convinced you that passing std::string_view by value is the right choice? If you're not convinced I'd be curious why. But I would like to get that detail straight before proceeding with the code review. Thanks.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 76.59574% with 11 lines in your changes missing coverage. Please review.

Project coverage is 71.3%. Comparing base (06733ec) to head (ba99ab4).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #4509     +/-   ##
=========================================
- Coverage     71.3%   71.3%   -0.0%     
=========================================
  Files          796     796             
  Lines        66966   66968      +2     
  Branches     10867   10866      -1     
=========================================
  Hits         47769   47769             
- Misses       19197   19199      +2     

Files	Coverage Δ
src/ripple/app/misc/ValidatorList.h	100.0% <ø> (ø)
src/ripple/app/misc/impl/ValidatorList.cpp	85.6% <100.0%> (ø)
src/ripple/basics/StringUtilities.h	88.5% <100.0%> (ø)
src/ripple/basics/base64.h	100.0% <ø> (ø)
src/ripple/basics/impl/StringUtilities.cpp	95.1% <100.0%> (ø)
src/ripple/basics/impl/base64.cpp	100.0% <100.0%> (ø)
src/ripple/json/Output.h	100.0% <ø> (ø)
src/ripple/overlay/impl/Cluster.cpp	95.1% <100.0%> (ø)
src/ripple/resource/impl/ResourceManager.cpp	75.0% <100.0%> (ø)
src/ripple/rpc/ServerHandler.h	100.0% <ø> (ø)
... and 8 more

... and 3 files with indirect coverage changes

[ckeshava]

@scottschurr Thanks for pointing it out, I have modified the relevant occurrences to use pass-by-value semantics in cfa960f

Do I need to git add the files inside the .install/include/ directory? I'm not sure if they are auto-generated from within the source code or whether they need to be manually edited.

[thejohnfreeman]

@ckeshava, no. I'm assuming you used Cupcake to install locally into the .install directory (its default choice). That is part of your personal workflow. That directory should not be added to the tracked files. If you prefer to ignore them, I suggest you add /.install/ to your global ignore file (like I have).

[intelliot]

• awaiting update or reply from @ckeshava

[ckeshava]

@scottschurr Okay, I have addressed your comments, thanks for your feedback.

There are two more instances where a boost::string_view is returned from two functions: extractIpAddrFromField and forwardedFor in Role.cpp. This has been in the code for the past 3 years.

Do you want me to change their return types to std::string instead? That would be a change in the behavior of the existing codebase (not only a refactor from boost::string_view to std::string_view)

[ckeshava]

@scottschurr I think it would be better to change the return type of those functions into a std::string. I like your pointer safety rationale in the return types (above comment).

But I agree, another time, another PR

[intelliot]

@ckeshava could you confirm whether this is ready to merge, from your perspective? Also, please suggest a commit message for the squashed commit.

[ckeshava]

It looks good from my end. Suggested commit message: Replaces the usage of boost::string_view to std::string_view