flint icon indicating copy to clipboard operation
flint copied to clipboard

Missing declarations runner

Open albinahlback opened this issue 1 year ago • 5 comments

Currently we have functions that are declared as global functions, but miss declarations in headers. Now, some may be missed in headers, other should be converted to static functions. Compiling with -Wmissing-prototypes shows which ones it is.

Doing this can show which ones should actually be local/static and which ones may miss a declaration in a header. On the other hand, if a helper function defined as a global function is no longer used, but still defined, it will still be compiled and take up space in the binary and in the source code. Hence, we should aim to cover the whole library with this compiler flag (will probably take years to fix this, thought).

Some files are problematic, such as inlines.c, but an easy guard with

#if defined(__GNUC__)
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wmissing-prototypes"
#endif

#include "footer.h"

#if defined(__GNUC__)
# pragma GCC diagnostic pop
#endif

fixes this

I think it is necessary to be able to sanitize the FLINT source code, but I'll wait a day or so before merging this in case someone has any objections.

albinahlback avatar Apr 12 '24 23:04 albinahlback

Your proposed sanitization sounds good to me.

On Fri, Apr 12, 2024, 19:39 Albin Ahlbäck @.***> wrote:

Currently we have functions that are declared as global functions, but miss declarations in headers. Now, some may be missed in headers, other should be converted to static functions. Compiling with -Wmissing-prototypes shows which ones it is.

Doing this can show which ones should actually be local/static and which ones may miss a declaration in a header. On the other hand, if a helper function defined as a global function is no longer used, but still defined, it will still be compiled and take up space in the binary and in the source code. Hence, we should aim to cover the whole library with this compiler flag (will probably take years, thought).

Some files are problematic, such as inlines.c, but an easy guard with

#if defined(GNUC)# pragma GCC diagnostic push# pragma GCC diagnostic ignored "-Wmissing-declarations"#endif #include "footer.h" #if defined(GNUC)# pragma GCC diagnostic pop#endif

I think it is necessary to be able to sanitize the FLINT source code, but I'll wait a day or so before merging this in case someone has any objections.

You can view, comment on, or merge this pull request online at:

https://github.com/flintlib/flint/pull/1921 Commit Summary

File Changes

(68 files https://github.com/flintlib/flint/pull/1921/files)

Patch Links:

  • https://github.com/flintlib/flint/pull/1921.patch
  • https://github.com/flintlib/flint/pull/1921.diff

— Reply to this email directly, view it on GitHub https://github.com/flintlib/flint/pull/1921, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACO2BX77BYEBV7ODMTTCHDY5BWCNAVCNFSM6AAAAABGE5VBFCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2DCMJRHE3TOMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

edgarcosta avatar Apr 12 '24 23:04 edgarcosta

I would really prefer to just insert more forward declarations instead of littering the codebase with pragmas except in very special circumstances.

fredrik-johansson avatar Apr 13 '24 11:04 fredrik-johansson

I would really prefer to just insert more forward declarations instead of littering the codebase with pragmas except in very special circumstances.

How about *-impl.h headers? That would allow us to keep track of global private functions and avoid us from using pragmas.

albinahlback avatar Apr 13 '24 11:04 albinahlback

How about *-impl.h headers? That would allow us to keep track of global private functions and avoid us from using pragmas.

I don't see the need. Nothing prevents us from declaring "global private" functions in the existing headers.

fredrik-johansson avatar Apr 14 '24 12:04 fredrik-johansson

How about *-impl.h headers? That would allow us to keep track of global private functions and avoid us from using pragmas.

I don't see the need. Nothing prevents us from declaring "global private" functions in the existing headers.

Ah, sorry. I have no objection to this. Will do this instead.

albinahlback avatar Apr 14 '24 12:04 albinahlback