script_nullpo_end: Use error parameter as an argument to printf, rather than building part of the formatter
PR !270 (merged) is an ongoing investigation into outstanding compiler warnings.
One such warning is warning: format '%s' expects a matching 'char*' argument
from script_nullpo_end(nd, STRPRINTF("no such npc: '%s'"_fmt, name));
in
src/map/script-fun.cpp
. No such warning is emitted from a similar line in
src/map/atcommand.cpp
: AString output = STRPRINTF("Jump to %s"_fmt, npc);
.
script_nullpo_end
is a macro, rather than a function. error
is passed in
verbatim by the preprocessor. The macro definition of script_nullpo_end
includes lines such as the following:
PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
#<variable>
instructs the preprocessor to transform <variable>
into a
literal string. In this case, the string literal:
"STRPRINTF(\"no such npc: '%s'\"_fmt, name)"
So the entire line partly resolves to something like:
printf("script:%s: STRPRINTF(\"no such npc: '%s'\"_fmt, name) @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
Once the compiler sees this, it throws out the warning seen here, because printf is seeing three placeholders in the formatter, but is only given two value arguments.
Checking with gcc -E
, the full expansion is as follows:
if (nullpo_chk("script-fun.cpp", 4885, __PRETTY_FUNCTION__, nd)) { if (st->oid) { dumb_ptr<npc_data> nullpo_nd = map_id_is_npc(st->oid); if (nullpo_nd && nullpo_nd->name) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " @ %s\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name), nullpo_nd->name); }); } else if (nullpo_nd) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (unnamed npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } st->state = ScriptEndState::END; return; };
Note the string literal: "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)"
.
script_nullpo_end
currently can't take as its argument for error
any
expression which, when stringified, contains something which printf
will
interpret as its formatter expecting another argument.
This appears to have been broken since it was first introduced in 594eafd4 since the first revision implicitly had these requirements, and also introduced usage which didn't meet them.
This rewrites each PRINTF line in script_nullpo_end
from the template
PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
to something like
PRINTF("script:%s: %s @ %s\n"_fmt, BUILTIN_NAME(), error, nullpo_nd->name);
This means that instead of an expression being stringified, error is interpreted as an expression that resolves (or decays) to a string.
This seems consistent with all current usage of script_nullpo_end
.