chore: migrate from deprecated absl types to std equivalents - #16400
chore: migrate from deprecated absl types to std equivalents#16400scotthart wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates various Abseil utilities (such as absl::variant, absl::visit, absl::holds_alternative, absl::get, absl::conjunction, absl::disjunction, and absl::void_t) to their standard C++17 library equivalents (std::variant, std::visit, etc.) across the codebase. The review feedback identifies a potential compilation failure in default_row_reader.cc due to mismatched return types, an unused Abseil header in list_objects_and_prefixes_reader.h, and a missed migration from absl::optional to std::optional in validate_metadata.cc.
| while (true) { | ||
| auto variant = AdvanceOrFail(); | ||
| if (absl::holds_alternative<bigtable::Row>(variant)) { | ||
| if (std::holds_alternative<bigtable::Row>(variant)) { |
There was a problem hiding this comment.
The return type of DefaultRowReader::Advance() (and potentially AdvanceOrFail()) still appears to be absl::variant (as seen in the hunk header). If AdvanceOrFail() returns absl::variant, using std::holds_alternative and std::get on it will fail to compile. Please ensure that the return types of both Advance() and AdvanceOrFail() are also migrated to std::variant in the corresponding header and source files to fully adhere to the style guide.
References
- Use
std::variantinstead ofabsl::variant. (link)
| #include "absl/types/variant.h" | ||
| #include <iterator> | ||
| #include <string> | ||
| #include <variant> |
There was a problem hiding this comment.
Since absl::visit has been migrated to std::visit in this file, the #include "absl/types/variant.h" on line 20 is likely no longer needed and should be removed to keep the includes clean.
References
- Use
std::variantinstead ofabsl::variant. (link)
| template <typename T> | ||
| struct GetAuthorityImpl< | ||
| T, absl::void_t<decltype(std::declval<T>().ExperimentalGetAuthority())>> { | ||
| T, std::void_t<decltype(std::declval<T>().ExperimentalGetAuthority())>> { |
There was a problem hiding this comment.
On the next line, absl::optional<std::string> is still used as the return type for Get. Please migrate it to std::optional<std::string> to adhere to the repository style guide.
References
- Use
std::optionalinstead ofabsl::optional. (link)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16400 +/- ##
==========================================
- Coverage 92.26% 92.26% -0.01%
==========================================
Files 2246 2246
Lines 212121 212121
==========================================
- Hits 195721 195705 -16
- Misses 16400 16416 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fixes #16354