Problem: Several string utilities return or store views into their input.
A temporary std::string can leave these views dangling, although no current caller does this.
Fix: Add LIFETIMEBOUND so Clang diagnoses the misuse while preserving immediate use.
Pass the Split span by value so lvalue strings do not trigger false warnings.
The string-view helpers return views into their input, while `LineReader` stores one.
Annotate their inputs so Clang can warn when a returned or stored view outlives a temporary string.
`Split` can return views into its input, but annotating its old reference warns for lvalue strings.
Take the span by value so Clang follows the backing storage.
`Split<std::string>` copies results but can still warn, while `SplitString` is unaffected.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36164.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | stickies-v, hodlinator, kevkevinpal, sedited |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
ACK b57b0dbebd56dac69d702e508af188e059db9c19
ACK b57b0dbebd56dac69d702e508af188e059db9c19
Clang 19 seems to be underpowered in using these annotations, but Clang 22 delivers.
<details><summary>Tested by introducing unsafe code and examining compile errors</summary>
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -1297,6 +1297,15 @@ BOOST_AUTO_TEST_CASE(test_script_parsing)
BOOST_CHECK_EQUAL(SpanToStr(results[1]), "");
BOOST_CHECK_EQUAL(SpanToStr(results[2]), "");
BOOST_CHECK_EQUAL(SpanToStr(results[3]), "");
+ std::string_view s = TrimStringView(std::string{"temporary"});
+ (void)s;
+ std::string_view s_p = util::RemoveSuffixView(std::string{"temporary"}, "y");
+ (void)s_p;
+ std::string_view s_s = util::RemovePrefixView(std::string{"temporary"}, "t");
+ (void)s_s;
+ std::vector<std::span<const char>> dangle_str = Split(std::string{"temporary"}, "x");
+ std::vector<std::span<const char>> dangle_c = Split(std::string{"temporary"}, 'c');
+ util::LineReader reader{std::string{"foo"}, 123};
input = "one#two#three";
results = Split(input, '-');
[2/3] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_tests.cpp.o
../src/test/util_tests.cpp:1300:41: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1300 | std::string_view s = TrimStringView(std::string{"temporary"});
| ^~~~~~~~~~~~~~~~~~~~~~~~
../src/test/util_tests.cpp:1302:51: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1302 | std::string_view s_p = util::RemoveSuffixView(std::string{"temporary"}, "y");
| ^~~~~~~~~~~~~~~~~~~~~~~~
../src/test/util_tests.cpp:1304:51: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1304 | std::string_view s_s = util::RemovePrefixView(std::string{"temporary"}, "t");
| ^~~~~~~~~~~~~~~~~~~~~~~~
../src/test/util_tests.cpp:1306:59: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1306 | std::vector<std::span<const char>> dangle_str = Split(std::string{"temporary"}, "x");
| ^~~~~~~~~~~~~~~~~~~~~~~~
../src/test/util_tests.cpp:1307:57: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1307 | std::vector<std::span<const char>> dangle_c = Split(std::string{"temporary"}, 'c');
| ^~~~~~~~~~~~~~~~~~~~~~~~
../src/test/util_tests.cpp:1308:29: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
1308 | util::LineReader reader{std::string{"foo"}, 123};
| ^~~~~~~~~~~~~~~~~~
6 warnings generated.
</details>
crACK b57b0db
Looks good to me, and makes sense to add LIFETIMEBOUND. We could also do a follow-up to do something similar for span.h as well, for MakeUCharSpan and MakeByteSpan
ACK b57b0dbebd56dac69d702e508af188e059db9c19