Incompatible function pointer types assigning to 'Function *' at readline module

[corona10]

At macOS 12.5.1 with Apple clang version 14.0.0 (clang-1400.0.29.202)

./Modules/readline.c:1252:21: warning: incompatible function pointer types assigning to 'Function *' (aka 'int (*)(const char *, int)') from 'int (void)' [-Wincompatible-function-pointer-types]
    rl_startup_hook = on_startup_hook;
                    ^ ~~~~~~~~~~~~~~~
./Modules/readline.c:1254:23: warning: incompatible function pointer types assigning to 'Function *' (aka 'int (*)(const char *, int)') from 'int (void)' [-Wincompatible-function-pointer-types]
    rl_pre_input_hook = on_pre_input_hook;

Linked PRs

• [WIP] gh-105323: Suppress function pointer warning by type casting #106284
• gh-105323: Add special workround for macOS default editline #108633
• gh-105323: Update readline module to detect apple editline variant #108665
• gh-105323: Remove WITH_APPLE_EDITLINE to use the same declaration for all editline #112513

[corona10]

Header from apple Xcode.

/* typedefs */
typedef int	  Function(const char *, int);
typedef void	  VFunction(void);
typedef void	  VCPFunction(char *);
typedef char	 *CPFunction(const char *, int);
typedef char	**CPPFunction(const char *, int, int);
typedef char     *rl_compentry_func_t(const char *, int);
typedef int	  rl_command_func_t(int, int);

[erlend-aasland]

@corona10, did you get any further with this? I'm getting annoyed at these warnings, myself :)

[corona10]

@erlend-aasland I will try to solve this issue at this weekend ;)

[corona10]

@ned-deily cc @erlend-aasland

Hi Ned,
AFAIK, Apple uses a different readline implementation from GNU and I checked API signatures were the same across versions.
https://github.com/phracker/MacOSX-SDKs
So we can easily solve this issue by using a conditional compile approach.

But if we have to care about linking with the GNU version on the macOS, the problem will be more difficult.
So we need to know that we should care or not.
https://stackoverflow.com/questions/7214374/how-do-i-link-against-the-gnu-readline-library-rather-than-libedit-in-macosx

[erlend-aasland]

But if we have to care about linking with the GNU version on the macOS, the problem will be more difficult.

You can use the preprocessor define WITH_EDITLINE to check if we're compiling with editline or readline.

[corona10]

You can use the preprocessor define WITH_EDITLINE to check if we're compiling with editline or readline.

Wow, That's good! I will prepare the patch by this weekend :)

[erlend-aasland]

If you grep for READLINE in configure.ac, you'll find that the dependency resolver for readline is more complex than that for other extension modules with third party dependencies. See also the WITH_EDITLINE conditional at the top of Modules/readline.c :)

[erlend-aasland]

See discussion on #108588

[corona10]

For reference what I investigated

Backend	configure	header	Real verison	Macro Version (RL_READLINE_VERSION)	Install	Source	rl_startup_hook	rl_pre_input_hook	User
readline	--with-readline=readline	readline/readline.h	8.2.1	0x0802	brew / MacPorts	https://ftp.gnu.org/gnu/readline/	typedef int rl_hook_func_t (void);	typedef int rl_hook_func_t (void);	Linux/BSD/ macOS/...
libedit w/readline wrapper	--with-readline=editline	editline/readline.h	20221030-3.1	0x0402	brew / MacPorts	https://www.thrysoee.dk/editline/	rl_hook_func_t(void);	rl_hook_func_t(void);	Linux/BSD/ macOS/...
libedit	--with-readline=editline	editline/editline.h	1.17.1	n/a	https://github.com/troglobit/editline	https://github.com/troglobit/editline	n/a	n/a	Linux/BSD/ macOS/...
Apple editline	--with-readline=apple	readline/readline.h editline/readline.h	libedit-57	0x0402	XCode SDK	https://github.com/apple-oss-distributions/libedit/	typedef int Function(const char *, int);	typedef int Function(const char *, int);	macOS

[erlend-aasland]

Ok, it looks like the libedit I linked to would be unusable with CPython, so we can disregard that.

[erlend-aasland]

BTW, we have some really outdated readline checks in configure.ac. Any check for functionality introduced in readline versions before 4.2 could probably be tossed out.

[gaogaotiantian]

I think this issue impacts more than MacOS.

I tried to link libedit on Ubuntu and it has the same issue.

sudo apt install libedit-dev
./configure --with-readline=editline
make

gcc -pthread -I/usr/include/editline -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include    -fPIC -c ./Modules/readline.c -o Modules/readline.o
./Modules/readline.c: In function ‘setup_readline’:
./Modules/readline.c:1309:21: warning: assignment to ‘int (*)(const char *, int)’ from incompatible pointer type ‘int (*)(void)’ [-Wincompatible-pointer-types]
 1309 |     rl_startup_hook = on_startup_hook;
      |                     ^
./Modules/readline.c:1311:23: warning: assignment to ‘int (*)(const char *, int)’ from incompatible pointer type ‘int (*)(void)’ [-Wincompatible-pointer-types]
 1311 |     rl_pre_input_hook = on_pre_input_hook;
      |                       ^

We only use two headers, <editline/readline.h> if with-readline=editline and <readline/readline.h> if with-readline=readline (default). I think <editline/readline.h> always uses the "old" declaration with typedef int Function(const char *, int);.

I was wondering if we need WITH_APPLE_EDITLINE. _RL_FUNCTION_TYPEDEF seems to be the macro to indicate that the rl_* functions have the new style declarations, and the corresponding "old style" is the one with Function.

I think we should do something like

static int
#if defined(_RL_FUNCTION_TYPEDEF)
on_startup_hook(void)
#else
on_startup_hook(const char *Py_UNUSED(text), int Py_UNUSED(state))
#endif

[corona10]

I was wondering if we need WITH_APPLE_EDITLINE. RL_FUNCTION_TYPEDEF seems to be the macro to indicate that the rl* functions have the new style declarations, and the corresponding "old style" is the one with Function.

Okay if this issue is happening outside of macOS too. We can use your suggestion for this issue.
The patch itself tried to reduce impact to outside of macOS.

[corona10]

@gaogaotiantian Do you want to submit the patch?

[gaogaotiantian]

Yeah I'll do it. As WITH_APPLE_EDITLINE is only used for this, should I just basically revert all the changes in #108665? The docs change to --with-readline should be kept I presume. Anything else?

[corona10]

Yeah I'll do it. As WITH_APPLE_EDITLINE is only used for this, should I just basically revert all the changes in #108665?

Just remove WITH_APPLE_EDITLINE please

[corona10]

Two things are needed.

1. Remove WITH_APPLE_EDITLINE from configure.ac and regendeate configure file.
2. Replace #elif defined(WITH_APPLE_EDITLINE) to #else

[gaogaotiantian]

Two things are needed.

1. Remove WITH_APPLE_EDITLINE from configure.ac and regendeate configure file.
2. Replace #elif defined(WITH_APPLE_EDITLINE) to #else

Done in #112513

[erlend-aasland]

Can this be closed, @corona10?

[corona10]

Can this be closed, @corona10?

Yeah sure!