Skip to content

cpp(BREAKING)!: use shared ptr instead of raw ptr for HubOptions & LoadOptions#108

Merged
wenchy merged 12 commits intomasterfrom
cpp-hub
Jul 18, 2025
Merged

cpp(BREAKING)!: use shared ptr instead of raw ptr for HubOptions & LoadOptions#108
wenchy merged 12 commits intomasterfrom
cpp-hub

Conversation

@Kybxd
Copy link
Copy Markdown
Collaborator

@Kybxd Kybxd commented Jul 15, 2025

❗BREAKING CHANGE

The plugin protoc-gen-cpp-tableau-loader function Load changed from bool Load(const std::string& dir, Format fmt = Format::kJSON, const LoadOptions* options = nullptr); to bool Load(const std::string& dir, Format fmt = Format::kJSON, std::shared_ptr<const LoadOptions> options = nullptr);

Compatibility resolution

  • If you have introduced custom messagers in your code, change it's parameter option type from pointer to shared_ptr.
  • Hub::Init changed to Hub::InitOnce

Comment thread test/cpp-tableau-loader/src/protoconf/hub.pc.cc Outdated
const std::shared_ptr<Messager> GetMessager(const std::string& name) const;
std::shared_ptr<MessagerContainer> GetProvidedMessagerContainer() const {
if (options_ != nullptr && options_->provider != nullptr) {
return options_->provider();
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also check the nilness of options_->provider()?

Copy link
Copy Markdown
Member

@wenchy wenchy Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the provider should guarantee that returned MessagerContainer MUST not be nil.

So we may add comment to the MessagerProvider: // Must return non-nil MessagerContainer.

Comment thread test/cpp-tableau-loader/src/protoconf/hub_shard0.pc.cc Outdated
Comment thread test/cpp-tableau-loader/src/main.cpp
Comment thread test/cpp-tableau-loader/src/hub/hub.cpp
Comment thread test/cpp-tableau-loader/src/protoconf/hub.pc.h Outdated
Comment thread test/cpp-tableau-loader/src/protoconf/hub_shard0.pc.cc Outdated
Comment thread test/cpp-tableau-loader/src/protoconf/hub.pc.cc Outdated
Comment thread test/cpp-tableau-loader/src/protoconf/hub.pc.cc
Comment thread test/cpp-tableau-loader/src/protoconf/hub.pc.h Outdated
Comment thread test/cpp-tableau-loader/src/main.cpp
Comment thread test/cpp-tableau-loader/src/hub/hub.h Outdated
@Kybxd Kybxd changed the title cpp: use shared ptr instead of raw ptr for HubOptions & LoadOptions perf(cpp)(BREAKING)!: use shared ptr instead of raw ptr for HubOptions & LoadOptions Jul 17, 2025
@wenchy wenchy changed the title perf(cpp)(BREAKING)!: use shared ptr instead of raw ptr for HubOptions & LoadOptions cpp(BREAKING)!: use shared ptr instead of raw ptr for HubOptions & LoadOptions Jul 18, 2025
auto options = std::make_shared<tableau::HubOptions>();
options->filter = DefaultFilter;
options->provider = DefaultMessagerContainerProvider;
tableau::Hub::InitOnce(options);
Copy link
Copy Markdown
Member

@wenchy wenchy Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify, please add comment: // call base hub's InitOnce

Comment thread test/cpp-tableau-loader/src/hub/hub.cpp
Comment thread test/cpp-tableau-loader/src/main.cpp
Comment on lines +40 to +41
// InitOnce sets the hub's options.
void InitOnce(std::shared_ptr<const HubOptions> options);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve comment to:

// InitOnce inits the hub only once, and the subsequent calls will not take effect.

@wenchy wenchy merged commit b268414 into master Jul 18, 2025
@wenchy wenchy deleted the cpp-hub branch July 18, 2025 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants