bkietz commented on code in PR #46303:
URL: https://github.com/apache/arrow/pull/46303#discussion_r2074232183


##########
cpp/src/arrow/compute/kernels/vector_rank.cc:
##########
@@ -381,9 +381,13 @@ class RankMetaFunction : public 
RankMetaFunctionBase<RankMetaFunction> {
   }
 
   RankMetaFunction()
-      : RankMetaFunctionBase("rank", Arity::Unary(), rank_doc, 
&kDefaultOptions) {}
+      : RankMetaFunctionBase("rank", Arity::Unary(), rank_doc, 
GetDefaultOptions()) {}

Review Comment:
   I'm not sure. It seems to work on my machine but I really expected this to 
be an ODR violation. On my machine, the static local becomes a weak symbol so 
the linker ignores multiple definitions and just uses the first.
   
   After reading through the [provisions for multiple 
definitions](https://timsong-cpp.github.io/cppwp/n4659/basic.def.odr#6), I 
tried using an inline variable and that seemed to work also:
   
   ```cpp
   template <typename OptionsType>
   inline const auto kDefault = OptionsType::Defaults();
   ```
   
   I'll keep thinking about this.



-- 
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-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to