Currently, in core.cc, construct_text_object function uses a ~1600 lines long if-elseif chain.
All of those are wrapped in macros and really difficult to reason about.
I propose all those macros get replaced with functions that handle parameters and construction in a cleaner C++ way, and gperf can be used to generate a hash based lookup for the appropriate function pointer.
A single function dereference ought to be faster than 450+ consecutive strcmps. But performance isn't really as much the point as this code being very unreadable:
```c++
#define __OBJ_HEAD(a, n) \
if (!strcmp(s, #a)) { \
obj->cb_handle = create_cb_handle(n);
#define __OBJ_IF obj_be_ifblock_if(ifblock_opaque, obj)
#define __OBJ_ARG(...) \
if (!arg) { \
free(s); \
CRIT_ERR_FREE(obj, free_at_crash, __VA_ARGS__); \
}
/* defines to be used below */
#define OBJ(a, n) __OBJ_HEAD(a, n) {
#define OBJ_ARG(a, n, ...) __OBJ_HEAD(a, n) __OBJ_ARG(__VA_ARGS__) {
#define OBJ_IF(a, n) \
__OBJ_HEAD(a, n) __OBJ_IF; \
{
#define OBJ_IF_ARG(a, n, ...) \
__OBJ_HEAD(a, n) __OBJ_ARG(__VA_ARGS__) __OBJ_IF; \
{
#define END \
} \
} \
else
#ifdef BUILD_GUI
if (s[0] == '#') {
obj->data.l = parse_color(s).to_argb32();
obj->callbacks.print = &new_fg;
} else
#endif /* BUILD_GUI */
#ifndef __OpenBSD__
OBJ(acpitemp, nullptr)
obj->data.i = open_acpi_temperature(arg);
obj->callbacks.print = &print_acpitemp;
obj->callbacks.free = &free_acpitemp;
END OBJ(acpiacadapter, nullptr) if (arg != nullptr) {
#ifdef __linux__
if (strpbrk(arg, "/.") != nullptr) {
/*
* a bit of paranoia. screen out funky paths
* i hope no device will have a '.' in its name
*/
NORM_ERR("acpiacadapter: arg must not contain '/' or '.'");
} else
obj->data.opaque = strdup(arg);
#else
NORM_ERR("acpiacadapter: arg is only used on linux");
#endif
}
obj->callbacks.print = &print_acpiacadapter;
obj->callbacks.free = &gen_free_opaque;
#endif /* !__OpenBSD__ */
END OBJ(freq, nullptr) get_cpu_count();
if ((arg == nullptr) || strlen(arg) >= 3 ||
strtol(&arg[0], nullptr, 10) == 0 ||
static_cast(strtol(&arg[0], nullptr, 10)) >
info.cpu_count) {
obj->data.i = 1;
/* NORM_ERR("freq: Invalid CPU number or you don't have that many CPUs! "
"Displaying the clock for CPU 1."); */
} else {
obj->data.i = strtol(&arg[0], nullptr, 10);
}
obj->callbacks.print = &print_freq;
END /* ... 1000+ more lines of this ... */
```
The macro processor effectively produces a chain of blocks looking like this:
CMake can collect these populate_object_function functions, generate a gperf input file and produce a perfect hash as part of the build process. Using gperf (again) isn't really the point, it would be ok to generate the same if-elseif chain from the build command, but I'd like to avoid having all those constructors as part of a huge macro sequence because its unsightly, hard to understand, and makes clang-format and syntax highlighting go "wuut".
This is a very tedious change that's mostly aimed at making this file easier to understand and maintain, but that's why I'm marking this issue as a low priority enhancement.
Currently, in
core.cc,construct_text_objectfunction uses a ~1600 lines long if-elseif chain.All of those are wrapped in macros and really difficult to reason about.
I propose all those macros get replaced with functions that handle parameters and construction in a cleaner C++ way, and gperf can be used to generate a hash based lookup for the appropriate function pointer.
A single function dereference ought to be faster than 450+ consecutive
strcmps. But performance isn't really as much the point as this code being very unreadable:The macro processor effectively produces a chain of blocks looking like this:
and the entire chain can be simplified to:
CMake can collect these
populate_object_functionfunctions, generate a gperf input file and produce a perfect hash as part of the build process. Using gperf (again) isn't really the point, it would be ok to generate the same if-elseif chain from the build command, but I'd like to avoid having all those constructors as part of a huge macro sequence because its unsightly, hard to understand, and makes clang-format and syntax highlighting go "wuut".This is a very tedious change that's mostly aimed at making this file easier to understand and maintain, but that's why I'm marking this issue as a low priority enhancement.