Remove all function definitions from header files #728

Closed
petdance opened this Issue Mar 5, 2012 · 7 comments

Comments

Projects
None yet
5 participants
@petdance
Contributor

petdance commented Mar 5, 2012

There are a number of header files that have functions declared in them. Move these to their own .c files.

@Util

This comment has been minimized.

Show comment
Hide comment
@Util

Util Mar 5, 2012

Member

FYI, I see 17 functions in 2 header files.

include/parrot/pointer_array.h :
#define ASSERT_ARGS_allocate_more_chunks __attribute__unused__ int _ASSERT_ARGS_CHECK = (PARROT_ASSERT_ARG(interp), PARROT_ASSERT_ARG(self))
    static void   allocate_more_chunks(PARROT_INTERP, Parrot_Pointer_Array *self)
    static void * Parrot_pa_insert(    PARROT_INTERP, Parrot_Pointer_Array *self, void *ptr)
    static void   Parrot_pa_remove(    PARROT_INTERP, Parrot_Pointer_Array *self, void *ptr)

src/packfile/byteorder.h :
    static INTVAL fetch_iv_le(INTVAL w)
    static INTVAL fetch_iv_be(INTVAL w)
    static opcode_t fetch_op_be(opcode_t w)
    static opcode_t fetch_op_le(opcode_t w)
    static void fetch_buf_be_4( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_4( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_8( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_8( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_12(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_12(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_16(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_16(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_32(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_32(unsigned char *rb, const unsigned char *b)
Member

Util commented Mar 5, 2012

FYI, I see 17 functions in 2 header files.

include/parrot/pointer_array.h :
#define ASSERT_ARGS_allocate_more_chunks __attribute__unused__ int _ASSERT_ARGS_CHECK = (PARROT_ASSERT_ARG(interp), PARROT_ASSERT_ARG(self))
    static void   allocate_more_chunks(PARROT_INTERP, Parrot_Pointer_Array *self)
    static void * Parrot_pa_insert(    PARROT_INTERP, Parrot_Pointer_Array *self, void *ptr)
    static void   Parrot_pa_remove(    PARROT_INTERP, Parrot_Pointer_Array *self, void *ptr)

src/packfile/byteorder.h :
    static INTVAL fetch_iv_le(INTVAL w)
    static INTVAL fetch_iv_be(INTVAL w)
    static opcode_t fetch_op_be(opcode_t w)
    static opcode_t fetch_op_le(opcode_t w)
    static void fetch_buf_be_4( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_4( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_8( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_8( unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_12(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_12(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_16(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_16(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_le_32(unsigned char *rb, const unsigned char *b)
    static void fetch_buf_be_32(unsigned char *rb, const unsigned char *b)
@gerdr

This comment has been minimized.

Show comment
Hide comment
@gerdr

gerdr Mar 5, 2012

Contributor

Moving the functions out of the headers means that C compilers without link-time optimization can no longer inline the code. At least some of these functions (possibly all) should remain where they are.

Contributor

gerdr commented Mar 5, 2012

Moving the functions out of the headers means that C compilers without link-time optimization can no longer inline the code. At least some of these functions (possibly all) should remain where they are.

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Mar 5, 2012

Contributor

The byteorder functions are all static and should be moved to the one file that uses them, src/packfile/pf_items.c. They will inline there just fine.

Contributor

petdance commented Mar 5, 2012

The byteorder functions are all static and should be moved to the one file that uses them, src/packfile/pf_items.c. They will inline there just fine.

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Mar 5, 2012

Contributor

The Parrot_pa_* functions are also static, although defined in the header. That means that each file that includes the headers is making its own copy of the same static functions.

Contributor

petdance commented Mar 5, 2012

The Parrot_pa_* functions are also static, although defined in the header. That means that each file that includes the headers is making its own copy of the same static functions.

@bacek

This comment has been minimized.

Show comment
Hide comment
@bacek

bacek Mar 5, 2012

Member

Yes, Parrot_pa_* functions are declared in header file on purpose. They are heavily used in GC and have to be inlined.

Member

bacek commented Mar 5, 2012

Yes, Parrot_pa_* functions are declared in header file on purpose. They are heavily used in GC and have to be inlined.

petdance added a commit that referenced this issue Mar 6, 2012

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Mar 6, 2012

Contributor

byteorder functions are now inlined on commit 4cb2d3b

Contributor

petdance commented Mar 6, 2012

byteorder functions are now inlined on commit 4cb2d3b

@petdance petdance closed this Mar 6, 2012

@petdance petdance reopened this Mar 6, 2012

@rurban

This comment has been minimized.

Show comment
Hide comment
@rurban

rurban Mar 11, 2014

Member

There are no todo item left. pf is done, and pa is there on purpose.

Member

rurban commented Mar 11, 2014

There are no todo item left. pf is done, and pa is there on purpose.

@rurban rurban closed this Mar 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment