You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "wgtmac (via GitHub)" <gi...@apache.org> on 2024/12/23 07:42:49 UTC
[PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
wgtmac opened a new pull request, #45100:
URL: https://github.com/apache/arrow/pull/45100
### Rationale for this change
The `Status::message` function below has defined a static const string in the header file which may cause troubles in different translation units.
```
const std::string& message() const {
static const std::string no_message = "";
return ok() ? no_message : state_->msg;
}
```
### What changes are included in this PR?
Move the definition of `Status::message` function into the source file.
### Are these changes tested?
Pass CIs.
### Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #45100:
URL: https://github.com/apache/arrow/pull/45100#issuecomment-2585566159
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit a2f19888e30b931f494c1ec66a3082c24d441d9a.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/35482754168) has more details. It also includes information about 29 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #45100:
URL: https://github.com/apache/arrow/pull/45100#issuecomment-2559084808
:warning: GitHub issue #45099 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #45100:
URL: https://github.com/apache/arrow/pull/45100#discussion_r1895380512
##########
cpp/src/arrow/status.h:
##########
@@ -332,10 +332,7 @@ class ARROW_EXPORT [[nodiscard]] Status : public util::EqualityComparable<Status
constexpr StatusCode code() const { return ok() ? StatusCode::OK : state_->code; }
/// \brief Return the specific error message attached to this status.
- const std::string& message() const {
- static const std::string no_message = "";
- return ok() ? no_message : state_->msg;
- }
+ const std::string& message() const;
/// \brief Return the status detail attached to this message.
const std::shared_ptr<StatusDetail>& detail() const {
Review Comment:
Do we need to do the same thing for this too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #45100:
URL: https://github.com/apache/arrow/pull/45100
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45099: [C++] Avoid static const variable in the status.h [arrow]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #45100:
URL: https://github.com/apache/arrow/pull/45100#discussion_r1895388886
##########
cpp/src/arrow/status.h:
##########
@@ -332,10 +332,7 @@ class ARROW_EXPORT [[nodiscard]] Status : public util::EqualityComparable<Status
constexpr StatusCode code() const { return ok() ? StatusCode::OK : state_->code; }
/// \brief Return the specific error message attached to this status.
- const std::string& message() const {
- static const std::string no_message = "";
- return ok() ? no_message : state_->msg;
- }
+ const std::string& message() const;
/// \brief Return the status detail attached to this message.
const std::shared_ptr<StatusDetail>& detail() const {
Review Comment:
I thought this is fine because it is nullptr. But it doesn't hurt to fix it as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org