Initial definition for TSK_TRACE_ERRORS

[jeromekelleher]

This is a proposal to address the discussion in #3094.

Essentially, we add a function-like macro tsk_trace_error, and everywhere we normally do

ret = TSK_ERR_X;
goto out;

we instead do

ret = tsk_trace_error(TSK_ERR_X);
goto out;

We can then define tsk_trace_error to be a no-op for production code, and define a function which emits some error traces. For the changes I've made here, we get this on stderr:

tskit-trace-error: -701='Sequence length must be > 0. (TSK_ERR_BAD_SEQUENCE_LENGTH)' at line 11000 in ../tskit/tables.c

Implementation notes

• tsk_trace_error must be a macro, otherwise we can't use FILE and LINE to get locations
• This structure has no change on control flow, which is essential
• It should be easy enough to sed the changes to implement this for the vast majority of the library.

I can push this through and document if folks like it.

[codecov]

Codecov Report

Attention: Patch coverage is 63.29897% with 178 lines in your changes missing coverage. Please review.

Project coverage is 89.94%. Comparing base (0e2fcc5) to head (56b3621).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
c/tskit/trees.c	56.81%	76 Missing ⚠️
c/tskit/tables.c	71.60%	71 Missing ⚠️
c/tskit/genotypes.c	35.00%	13 Missing ⚠️
c/tskit/core.c	22.22%	7 Missing ⚠️
c/tskit/haplotype_matching.c	56.25%	7 Missing ⚠️
c/tskit/convert.c	70.00%	3 Missing ⚠️
c/tskit/stats.c	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3095      +/-   ##
==========================================
- Coverage   89.95%   89.94%   -0.02%     
==========================================
  Files          29       29              
  Lines       32627    32641      +14     
  Branches     5851     5851              
==========================================
+ Hits        29351    29358       +7     
- Misses       1862     1869       +7     
  Partials     1414     1414              

Flag	Coverage Δ
c-tests	86.66% <63.29%> (-0.04%)	⬇️
lwt-tests	80.78% <ø> (ø)
python-c-tests	89.23% <ø> (ø)
python-tests	98.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
c/tskit/core.h	100.00% <ø> (ø)
c/tskit/stats.c	83.53% <75.00%> (-0.52%)	⬇️
c/tskit/convert.c	88.54% <70.00%> (ø)
c/tskit/core.c	95.50% <22.22%> (-0.40%)	⬇️
c/tskit/haplotype_matching.c	84.55% <56.25%> (ø)
c/tskit/genotypes.c	84.75% <35.00%> (ø)
c/tskit/tables.c	83.20% <71.60%> (+<0.01%)	⬆️
c/tskit/trees.c	90.61% <56.81%> (-0.04%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[benjeffery]

Seems like a solid idea to me, should be able to hit most of the changepoints with a regex!

[jeromekelleher]

OK cool, I'll fill this out as I get a chance.

[benjeffery]

Did you want this for 0.6.1? I'm happy to finish it up. If we did I guess we do a C API release too.

[jeromekelleher]

Would be great if you could pick this up, thanks @benjeffery. I think the code changes are done, it's just a case of documenting what's going on in the developer docs and telling devs how to turn on the flag at compile time.

[benjeffery]

@jeromekelleher I've added some detail to the docs and added some meson config that turns on the flag if --buildtype=debug is set.

I've also had a good look for places that needed the function added and found none.

[jeromekelleher]

That's great, thanks. I think we do need to document the actual setting of -DTSK_TRACE_ERRORS though, as users of the C API aren't necessarily using meson.

[benjeffery]

Ok added! Needs a squash, I'll do that if it all looks good.

[jeromekelleher]

LGTM! I can't approve as it's my PR. Squash away