[jira] [Created] (IMPALA-11412) CodegenFnPtr::store() has a compile time error when instantiated

["Daniel Becker (Jira)" <ji...@apache.org>]

Daniel Becker created IMPALA-11412:
--------------------------------------

             Summary: CodegenFnPtr<FuncType>::store() has a compile time error when instantiated
                 Key: IMPALA-11412
                 URL: https://issues.apache.org/jira/browse/IMPALA-11412
             Project: IMPALA
          Issue Type: Bug
          Components: Backend
            Reporter: Daniel Becker
            Assignee: Daniel Becker


The function template {{CodegenFnPtr<FuncType>::store() }}tries to implicitly cast a function pointer (of type '{{{}FuncType{}}}') to {{{}void*{}}}, which is a compile time error. The reason this didn't come up in the builds is that this function template is currently not used anywhere, and the function pointers are stored through the parent class, using {{{}CodegenFnPtrBase::store(){}}}, which  takes a {{{}void*{}}}.

We should either 
 # remove the hitherto unused {{CodegenFnPtr<FuncType>::store()}} function template 
OR
 # add the correct explicit cast from function pointer to {{void*}} AND add a test which instantiates (and tests) this function template so we can be sure that the new implementation is correct.

I'm inclined to choose the second option because I think the interface of {{CodegenFnPtr<FuncType>}} is more complete if we have this function as well, even if it is currently not used.

Note:
After digging a bit on the internet I found that the reason that implicit function pointer to {{void*}} cast is not allowed (as opposed to implicit regular pointer to {{{}void*{}}}) is because the standard doesn't guarantee that regular and function pointers have the same size, and there are some architectures where they actually don't.

However, according to 8) on [https://en.cppreference.com/w/cpp/language/reinterpret_cast|https://en.cppreference.com/w/cpp/language/reinterpret_cast], POSIX compliant systems do have this guarantee, so it shouldn't be a problem that we store funcion pointers as {{{}void*{}}}. We don't really have a choice because LLVM does the same as  {{llvm::ExecutionEngine::getPointerToFunction()}} returns a {{void*}} (see [https://llvm.org/doxygen/classllvm_1_1ExecutionEngine.html#acc46759a6acfc3d116c3f22110326ffa|https://llvm.org/doxygen/classllvm_1_1ExecutionEngine.html#acc46759a6acfc3d116c3f22110326ffa]); we call the function [https://github.com/apache/impala/blob/fefb9f24be1f99ac0077a8d6ef00834d8e90ef45/be/src/codegen/llvm-codegen.cc#L1315|here].



--
This message was sent by Atlassian Jira
(v8.20.10#820010)