Skip to content

Wconsumed-for-expected #63207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Mryange wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Wconsumed-for-expected #63207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions be/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ if (COMPILER_CLANG)
endif()

add_compile_options(-fcolor-diagnostics
-Wconsumed
-Wpedantic
-Wshadow
-Wshadow-field
Expand Down
14 changes: 9 additions & 5 deletions be/src/agent/task_worker_pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ void cloud_submit_table_compaction_callback(CloudStorageEngine& engine,
<< ", error=" << tablet_res.error();
return;
}
CloudTabletSPtr tablet = std::move(tablet_res).value();
CloudTabletSPtr tablet = *std::move(tablet_res);
if (tablet == nullptr) {
LOG(WARNING) << "cloud tablet not found. tablet_id=" << compaction_req.tablet_id;
return;
Expand Down Expand Up @@ -1728,7 +1728,7 @@ void update_s3_resource(const TStorageResource& param, io::RemoteFileSystemSPtr
if (!res.has_value()) {
st = std::move(res).error();
} else {
fs = std::move(res).value();
fs = *std::move(res);
}
} else {
DCHECK_EQ(existed_fs->type(), io::FileSystemType::S3) << param.id << ' ' << param.name;
Expand Down Expand Up @@ -1763,7 +1763,7 @@ void update_hdfs_resource(const TStorageResource& param, io::RemoteFileSystemSPt
if (!res.has_value()) {
st = std::move(res).error();
} else {
fs = std::move(res).value();
fs = *std::move(res);
}

} else {
Expand Down Expand Up @@ -2367,14 +2367,18 @@ void set_alter_version_before_enqueue(CloudStorageEngine& engine, const TAgentTa
return;
}
auto new_tablet = engine.tablet_mgr().get_tablet(alter_req.new_tablet_id);
if (!new_tablet.has_value() || new_tablet.value()->tablet_state() == TABLET_RUNNING) {
if (!new_tablet.has_value()) {
return;
}
auto* new_tablet_ptr = new_tablet.value().get();
if (new_tablet_ptr->tablet_state() == TABLET_RUNNING) {
return;
}
auto base_tablet = engine.tablet_mgr().get_tablet(alter_req.base_tablet_id);
if (!base_tablet.has_value()) {
return;
}
new_tablet.value()->set_alter_version(alter_req.alter_version);
new_tablet_ptr->set_alter_version(alter_req.alter_version);
base_tablet.value()->set_alter_version(alter_req.alter_version);
LOG(INFO) << "set alter_version=" << alter_req.alter_version
<< " before enqueue, base_tablet=" << alter_req.base_tablet_id
Expand Down
11 changes: 0 additions & 11 deletions be/src/common/expected.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#error need cpp support
#endif

#if __cplusplus <= 202002L
#include "util/expected.hpp"
namespace doris {
using tl::expected; // NOLINT
Expand All @@ -30,13 +29,3 @@ using tl::bad_expected_access; // NOLINT
using tl::unexpect_t; // NOLINT
using tl::unexpect; // NOLINT
} // namespace doris
#else
#include <expected>
namespace doris {
using std::expected; // NOLINT
using std::unexpected; // NOLINT
using std::bad_expected_access; // NOLINT
using std::unexpect_t; // NOLINT
using std::unexpect; // NOLINT
} // namespace doris
#endif
4 changes: 2 additions & 2 deletions be/src/common/status.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ using ResultError = unexpected<Status>;
if (!try_res.has_value()) [[unlikely]] { \
return std::forward<T>(try_res).error(); \
} \
std::forward<T>(try_res).value(); \
*std::forward<T>(try_res); \
});

#define TEST_TRY(stmt) \
Expand All @@ -756,7 +756,7 @@ using ResultError = unexpected<Status>;
if (!res.has_value()) [[unlikely]] { \
ASSERT_TRUE(res.has_value()) << "Expected success, but got error: " << res.error(); \
} \
std::forward<T>(res).value(); \
*std::forward<T>(res); \
})

#define TEST_RESULT_ERROR(stmt) \
Expand Down
12 changes: 8 additions & 4 deletions be/src/exec/rowid_fetcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,10 @@ Status RowIdStorageReader::read_by_rowids(const PMultiGetRequest& request,
BaseTabletSPtr tablet = scope_timer_run(
[&]() {
auto res = ExecEnv::get_tablet(row_loc.tablet_id(), nullptr, true);
return !res.has_value() ? nullptr
: std::dynamic_pointer_cast<BaseTablet>(res.value());
if (!res.has_value()) {
return BaseTabletSPtr(nullptr);
}
return std::dynamic_pointer_cast<BaseTablet>(res.value());
},
&acquire_tablet_ms);
RowsetId rowset_id;
Expand Down Expand Up @@ -1015,8 +1017,10 @@ Status RowIdStorageReader::read_doris_format_row(
tablet = scope_timer_run(
[&]() {
auto res = ExecEnv::get_tablet(tablet_id);
return !res.has_value() ? nullptr
: std::dynamic_pointer_cast<BaseTablet>(res.value());
if (!res.has_value()) {
return BaseTabletSPtr(nullptr);
}
return std::dynamic_pointer_cast<BaseTablet>(res.value());
},
acquire_tablet_ms);
if (!tablet) {
Expand Down
2 changes: 1 addition & 1 deletion be/src/io/cache/block_file_cache_downloader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ void FileCacheBlockDownloader::download_file_cache_block(
decrease_inflight_count();
return;
} else {
tablet = std::move(res).value();
tablet = *std::move(res);
}
if (!synced_tablets.contains(meta.tablet_id())) {
auto st = tablet->sync_rowsets();
Expand Down
2 changes: 1 addition & 1 deletion be/src/io/file_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ Result<io::FileReaderSPtr> FileFactory::create_file_reader(
if (!reader_res.has_value()) {
return unexpected(std::move(reader_res).error());
}
return std::move(reader_res).value();
return *std::move(reader_res);
}

Result<io::FileReaderSPtr> FileFactory::_create_file_reader_internal(
Expand Down
2 changes: 1 addition & 1 deletion be/src/io/fs/hdfs_file_system.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Status HdfsFileSystem::create_file_impl(const Path& file, FileWriterPtr* writer,
const FileWriterOptions* opts) {
auto res = io::HdfsFileWriter::create(file, _fs_handler, _fs_name, opts);
if (res.has_value()) {
*writer = std::move(res).value();
*writer = *std::move(res);
return Status::OK();
} else {
return std::move(res).error();
Expand Down
10 changes: 6 additions & 4 deletions be/src/load/channel/load_stream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,10 @@ Status TabletStream::append_data(const PStreamHeader& header, butil::IOBuf* data
auto st = _load_stream_writer->append_data(new_segid, header.offset(), buf, file_type);
if (!st.ok() && !config::is_cloud_mode()) {
auto res = ExecEnv::get_tablet(_id);
TabletSharedPtr tablet =
res.has_value() ? std::dynamic_pointer_cast<Tablet>(res.value()) : nullptr;
TabletSharedPtr tablet = nullptr;
if (res.has_value()) {
tablet = std::dynamic_pointer_cast<Tablet>(res.value());
}
if (tablet) {
tablet->report_error(st);
}
Expand Down Expand Up @@ -573,7 +575,7 @@ void LoadStream::_report_schema(StreamId stream, const PStreamHeader& hdr) {
for (const auto& req : hdr.tablets()) {
BaseTabletSPtr tablet;
if (auto res = ExecEnv::get_tablet(req.tablet_id()); res.has_value()) {
tablet = std::move(res).value();
tablet = *std::move(res);
} else {
st = std::move(res).error();
break;
Expand Down Expand Up @@ -621,7 +623,7 @@ void LoadStream::_collect_tablet_load_info_from_tablets(
for (auto tablet_id : tablet_ids) {
BaseTabletSPtr tablet;
if (auto res = ExecEnv::get_tablet(tablet_id); res.has_value()) {
tablet = std::move(res).value();
tablet = *std::move(res);
} else {
continue;
}
Expand Down
7 changes: 3 additions & 4 deletions be/src/service/internal_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void PInternalService::open_load_stream(google::protobuf::RpcController* control
cntl->SetFailed(st.to_string());
return;
} else {
tablet = std::move(res).value();
tablet = *std::move(res);
}
auto resp = response->add_tablet_schemas();
resp->set_index_id(req.index_id());
Expand Down Expand Up @@ -755,14 +755,13 @@ void PInternalService::outfile_write_success(google::protobuf::RpcController* co
.write_file_cache = false,
.sync_file_data = false,
});
using T = std::decay_t<decltype(res)>;
if (!res.has_value()) [[unlikely]] {
st = std::forward<T>(res).error();
st = res.error();
st.to_protobuf(result->mutable_status());
return;
}

std::unique_ptr<doris::io::FileWriter> _file_writer_impl = std::forward<T>(res).value();
std::unique_ptr<doris::io::FileWriter> _file_writer_impl = *std::move(res);
// must write somthing because s3 file writer can not writer empty file
st = _file_writer_impl->append({"success"});
if (!st.ok()) {
Expand Down
5 changes: 4 additions & 1 deletion be/src/storage/index/inverted/query_v2/null_bitmap_fetcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ class FieldNullBitmapFetcher {
}

auto has_null = iterator->has_null();
if (!has_null.has_value() || !has_null.value()) {
if (!has_null.has_value()) {
return nullptr;
}
if (!has_null.value()) {
return nullptr;
}

Expand Down
3 changes: 1 addition & 2 deletions be/src/storage/rowset/beta_rowset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,7 @@ Status BetaRowset::show_nested_index_file(rapidjson::Value* rowset_value,
LOG(INFO) << "IndexFileReader open error:" << ret.error();
return Status::InternalError("IndexFileReader open error");
}
using T = std::decay_t<decltype(ret)>;
auto reader = std::forward<T>(ret).value();
auto reader = *std::move(ret);
reader->list(&files);
for (auto& file : files) {
rapidjson::Value file_value(rapidjson::kObjectType);
Expand Down
6 changes: 3 additions & 3 deletions be/src/storage/schema_change/schema_change.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ Result<RowsetSharedPtr> VBaseSchemaChangeWithSorting::_internal_sorting(
[[unlikely]] {
return unexpected(std::move(result).error());
} else {
rowset_writer = std::move(result).value();
rowset_writer = *std::move(result);
}
RETURN_IF_ERROR_RESULT(merger.merge(blocks, rowset_writer.get(), &merged_rows));
_add_merged_rows(merged_rows);
Expand Down Expand Up @@ -739,7 +739,7 @@ Result<RowsetSharedPtr> VLocalSchemaChangeWithSorting::_internal_sorting(
[[unlikely]] {
return unexpected(std::move(result).error());
} else {
rowset_writer = std::move(result).value();
rowset_writer = *std::move(result);
}
auto guard = _local_storage_engine.pending_local_rowsets().add(context.rowset_id);
_pending_rs_guards.push_back(std::move(guard));
Expand Down Expand Up @@ -1300,7 +1300,7 @@ Status SchemaChangeJob::_convert_historical_rowsets(const SchemaChangeParams& sc
result.error().to_string());
return process_alter_exit();
}
auto rowset_writer = std::move(result).value();
auto rowset_writer = *std::move(result);
auto pending_rs_guard = _local_storage_engine.add_pending_rowset(context);

if (res = sc_procedure->process(rs_reader, rowset_writer.get(), _new_tablet, _base_tablet,
Expand Down
6 changes: 4 additions & 2 deletions be/src/storage/segment/segment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ Status Segment::open(io::FileSystemSPtr fs, const std::string& path, int64_t tab
if (!s.ok()) {
if (!config::is_cloud_mode()) {
auto res = ExecEnv::get_tablet(tablet_id);
TabletSharedPtr tablet =
res.has_value() ? std::dynamic_pointer_cast<Tablet>(res.value()) : nullptr;
TabletSharedPtr tablet = nullptr;
if (res.has_value()) {
tablet = std::dynamic_pointer_cast<Tablet>(res.value());
}
if (tablet) {
tablet->report_error(s);
}
Expand Down
6 changes: 5 additions & 1 deletion be/src/storage/tablet/tablet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2885,7 +2885,11 @@ std::string Tablet::get_segment_path(const RowsetMetaSharedPtr& rs_meta, int64_t
if (rs_meta->is_local()) {
segment_path = local_segment_path(_tablet_path, rs_meta->rowset_id().to_string(), seg_id);
} else {
segment_path = rs_meta->remote_storage_resource().value()->remote_segment_path(
auto storage_resource = rs_meta->remote_storage_resource();
if (!storage_resource.has_value()) [[unlikely]] {
throw Exception(storage_resource.error());
}
segment_path = storage_resource.value()->remote_segment_path(
rs_meta->tablet_id(), rs_meta->rowset_id().to_string(), seg_id);
}
return segment_path;
Expand Down
48 changes: 35 additions & 13 deletions be/src/util/expected.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@
#endif
#endif

#if defined(__clang__) && __has_attribute(consumable) && __has_attribute(callable_when) && \
__has_attribute(test_typestate) && __has_attribute(return_typestate)
#define TL_EXPECTED_CLANG_CONSUMABLE_UNKNOWN __attribute__((consumable(unknown)))
#define TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED __attribute__((callable_when("unconsumed")))
#define TL_EXPECTED_CLANG_TEST_UNCONSUMED __attribute__((test_typestate(unconsumed)))
#define TL_EXPECTED_CLANG_RETURN_UNKNOWN __attribute__((return_typestate(unknown)))
#else
#define TL_EXPECTED_CLANG_CONSUMABLE_UNKNOWN
#define TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED
#define TL_EXPECTED_CLANG_TEST_UNCONSUMED
#define TL_EXPECTED_CLANG_RETURN_UNKNOWN
#endif

#if (defined(__GNUC__) && __GNUC__ == 4 && __GNUC_MINOR__ <= 9 && \
!defined(__clang__))
// GCC < 5 doesn't support overloading on const&& for member functions
Expand Down Expand Up @@ -1249,10 +1262,11 @@ template <class E> class bad_expected_access : public std::exception {
/// has been destroyed. The initialization state of the contained object is
/// tracked by the expected object.
template <class T, class E>
class expected : private detail::expected_move_assign_base<T, E>,
private detail::expected_delete_ctor_base<T, E>,
private detail::expected_delete_assign_base<T, E>,
private detail::expected_default_ctor_base<T, E> {
class TL_EXPECTED_CLANG_CONSUMABLE_UNKNOWN
expected : private detail::expected_move_assign_base<T, E>,
private detail::expected_delete_ctor_base<T, E>,
private detail::expected_delete_assign_base<T, E>,
private detail::expected_default_ctor_base<T, E> {
static_assert(!std::is_reference<T>::value, "T must not be a reference");
static_assert(!std::is_same<T, std::remove_cv<in_place_t>::type>::value,
"T must not be in_place_t");
Expand Down Expand Up @@ -1529,9 +1543,9 @@ class expected : private detail::expected_move_assign_base<T, E>,
return or_else_impl(std::move(*this), std::forward<F>(f));
}
#endif
constexpr expected() = default;
constexpr expected(const expected &rhs) = default;
constexpr expected(expected &&rhs) = default;
constexpr expected() TL_EXPECTED_CLANG_RETURN_UNKNOWN = default;
constexpr expected(const expected &rhs) TL_EXPECTED_CLANG_RETURN_UNKNOWN = default;
constexpr expected(expected &&rhs) TL_EXPECTED_CLANG_RETURN_UNKNOWN = default;
expected &operator=(const expected &rhs) = default;
expected &operator=(expected &&rhs) = default;

Expand Down Expand Up @@ -1983,33 +1997,41 @@ class expected : private detail::expected_move_assign_base<T, E>,
return std::move(val());
}

constexpr bool has_value() const noexcept { return this->m_has_val; }
constexpr explicit operator bool() const noexcept { return this->m_has_val; }
constexpr bool has_value() const noexcept TL_EXPECTED_CLANG_TEST_UNCONSUMED {
return this->m_has_val;
}
constexpr explicit operator bool() const noexcept TL_EXPECTED_CLANG_TEST_UNCONSUMED {
return this->m_has_val;
}

template <class U = T,
detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
TL_EXPECTED_11_CONSTEXPR const U &value() const & {
TL_EXPECTED_11_CONSTEXPR const U &value() const &
TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED {
if (!has_value())
detail::throw_exception(bad_expected_access<E>(err().value()));
return val();
}
template <class U = T,
detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
TL_EXPECTED_11_CONSTEXPR U &value() & {
TL_EXPECTED_11_CONSTEXPR U &value() &
TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED {
if (!has_value())
detail::throw_exception(bad_expected_access<E>(err().value()));
return val();
}
template <class U = T,
detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
TL_EXPECTED_11_CONSTEXPR const U &&value() const && {
TL_EXPECTED_11_CONSTEXPR const U &&value() const &&
TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED {
if (!has_value())
detail::throw_exception(bad_expected_access<E>(std::move(err()).value()));
return std::move(val());
}
template <class U = T,
detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
TL_EXPECTED_11_CONSTEXPR U &&value() && {
TL_EXPECTED_11_CONSTEXPR U &&value() &&
TL_EXPECTED_CLANG_CALLABLE_WHEN_UNCONSUMED {
if (!has_value())
detail::throw_exception(bad_expected_access<E>(std::move(err()).value()));
return std::move(val());
Expand Down
Loading