On 9/23/21 10:33 AM, Ævar Arnfjörð Bjarmason wrote:
On Mon, Sep 20 2021, Jeff Hostetler via GitGitGadget wrote:
Here is V2 of Part 1 of my Builtin FSMonitor series.
Changes since V1 include:
* Drop the Trace2 memory leak.
* Added a new "child_ready" event to Trace2 as an alternative to the
"child_exit" event for background processes.
* Convert the Trace2-related NEEDSWORK items in "start_bg_command()" to use
the new "child_ready" event.
* Various minor code and documentation cleanups.
I see 7/7 still has a pattern you included only to make a compiler error
better. I noted in
https://lore.kernel.org/git/87ilyycko3.fsf@xxxxxxxxxxxxxxxxxxx/ that it
make the error worse, on at least clang. You didn't note which compiler
you were massaging, presumably MSVC.
I've been holding my tongue for days on this issue and hoping a third
party would step in an render an opinion one way or another.
Too me, a forward declaration seemed like no big deal and it does
have value as I tried to explain. And frankly, it felt a little bit
like bike-shedding and was trying to avoid that again.
The error message I quoted was from Clang v11.0.3. My forward
declaration of the function prior to the actual definition of
the function causes the compiler to stop at the function definition
and complain with a short message saying that the function itself
is incorrectly defined and doesn't match the typedef that it is
associated with.
When I use MSVC I get a similar error at the function definition.
When I use GCC I get error messages at both the function definition
and the usage in the call.
Additionally, the forward declaration states that the function is
associated with that typedef (something that is otherwise implicit
and may be hard to discover (more on that in a minute)).
And it doesn't require a reference to the function pointer (either
on the right side of an assignment, a vtable initialization, or passing
it in a function call) to flag the error. We always get the error
at the point of the definition.
The error message in your example is, I feel, worse than mine.
It splats 2 different function signatures -- only one of which has
the typedef name -- in a large, poorly wrapped brick of text.
Yes, your error message does print corresponding arg in the function
prototype of "start_bg_command()" that doesn't agree with the symbol
used at the call-site, but that is much later than where the actual
error occurred. And if the forward declaration were present, you'd
already know that back up at the definition, right.
Let's look at this from another point of view.
Suppose for example we have two function prototypes with the same
signature. Perhaps they describe groups of functions with different
semantics -- the fact that they have the same argument list and return
type is just a coincidence.
typedef int(t_fn_1)(int);
typedef int(t_fn_2)(int);
And then declare one or more instances of functions in those groups:
int foo_a(int x) { ... }
int foo_b(int x) { ... }
int foo_c(int x) { ... }
int foo_d(int x) { ... }
int foo_e(int x) { ... }
int foo_f(int x) { ... }
int foo_g(int x) { ... }
Which of those functions should be associated with "t_fn_1" and which
with "t_fn_2"? Again, they all have the same signature, but different
semantics. The author knows when they wrote the code, but it may be
hard to automatically determine later.
If I then have a function like start_bg_command() that receives a
function pointer:
int test(..., t_fn_1 *fn, ...) { ... }
In C -- even with the use of forward function declarations -- the
compiler won't complain if you pass test() a pointer of type t_fn_2
-- as long a t_fn_1 and t_fn_2 have the same signature.
But it does give the human a chance to catch the error. Of if we
later change the function signature in the t_fn_1 typedef, we will
automatically get a list of which of those foo_x functions do and
do not need to be updated.
Anyway, I've soapboxed on this enough. I think it is a worthy
feature for the price.
Jeff
