Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed INLINE definition from inline to static inline in pdcscrn.c #2

Merged
merged 1 commit into from
Mar 9, 2016

Conversation

ihalila
Copy link
Contributor

@ihalila ihalila commented Mar 9, 2016

This fixes compilation failing on GCC 5.3.0 on MSYS2 when compiling with DEBUG defined.

The errors I received were as follows:

gcc -g -Wall -DPDCDEBUG -I.. -mwindows -otestcurs.exe ../demos/testcurs.c pdcurses.a -lgdi32 -lcomdlg32
pdcurses.a(pdcscrn.o): In function `final_cleanup':
..\PDCurses\win32a/../win32a/pdcscrn.c:109: undefined reference to `set_default_sizes_from_registry'
pdcurses.a(pdcscrn.o): In function `rectangle_from_chars_to_pixels':
...\PDCurses\win32a/../win32a/pdcscrn.c:779: undefined reference to `sort_out_rect'
pdcurses.a(pdcscrn.o): In function `WndProc':
...\PDCurses\win32a/../win32a/pdcscrn.c:1793: undefined reference to `HandleSizing'
...\PDCurses\win32a/../win32a/pdcscrn.c:1839: undefined reference to `HandleBlockCopy'
...\PDCurses\win32a/../win32a/pdcscrn.c:1994: undefined reference to `HandleMenuToggle'
...\PDCurses\win32a/../win32a/pdcscrn.c:2006: undefined reference to `show_mouse_rect'
...\PDCurses\win32a/../win32a/pdcscrn.c:2022: undefined reference to `milliseconds_since_1970'
pdcurses.a(pdcscrn.o): In function `PDC_scr_open':
...\PDCurses\win32a/../win32a/pdcscrn.c:2305: undefined reference to `set_up_window'
collect2.exe: error: ld returned 1 exit status

I haven't tried other platforms but if I've understood the semantics of static inline correctly I don't think this should break anything, it seems to me that static inline is the correct choice in exactly this case (function defined and used in the same translation unit and nowhere else).

As far as I understand it what gcc does here is when optimizations are turned off, it doesn't inline anything. And a bare inline without the static keyword indicates that the function may be defined externally so the inline version simply gets ignored. And because the function isn't declared elsewhere linking fails.

This fixes compilation failing on GCC 5.3.0 on MSYS2
when compiling with DEBUG defined.
Bill-Gray added a commit that referenced this pull request Mar 9, 2016
Changed INLINE definition from inline to static inline in pdcscrn.c
@Bill-Gray Bill-Gray merged commit 03eafc5 into Bill-Gray:master Mar 9, 2016
@Bill-Gray
Copy link
Owner

Bill-Gray commented Mar 9, 2016

Hi Ilkka,

Thank you. I've learned a few things here.

Somewhere, I got the idea that "inline" always meant "static"; that
is, you couldn't write an inline function in file1.c and use it in file2.c.
And the clang compiler appears to behave in that manner. I just wrote
a very simple program using a non-static inline function. With clang,
the compile failed. And I'm pretty sure it would fail with Visual C++
as well; I think that's where I got the idea that you couldn't have
non-static inline functions.

But with gcc, my simple example compiled and ran correctly. These
"INLINE" functions are only used within pdcscrn.c. The only reason I
didn't declare them to be 'static inline' was that I assumed that would
be redundant. Apparently, at least in gcc, it isn't.

Cross-compiling from Linux using MinGW, I don't get those undefined
references. I don't know why. But applying your change causes no problems,
will fix the errors you're getting, and makes it explicitly clear that
the functions really are only used within 'pdcscrn.c'. So I'm happy to
take this change.

Thanks! -- Bill

On 2016-03-09 14:21, Ilkka Halila wrote:

This fixes compilation failing on GCC 5.3.0 on MSYS2 when compiling with DEBUG defined.

The errors I received were as follows:

|gcc -g -Wall -DPDCDEBUG -I.. -mwindows -otestcurs.exe ../demos/testcurs.c pdcurses.a
-lgdi32 -lcomdlg32 pdcurses.a(pdcscrn.o): In function final_cleanup': ..\PDCurses\win32a/../win32a/pdcscrn.c:109: undefined reference to set_default_sizes_from_registry' pdcurses.a(pdcscrn.o): In function
rectangle_from_chars_to_pixels': ...\PDCurses\win32a/../win32a/pdcscrn.c:779: undefined reference tosort_out_rect' pdcurses.a(pdcscrn.o): In function WndProc': ...\PDCurses\win32a/../win32a/pdcscrn.c:1793: undefined reference toHandleSizing'
...\PDCurses\win32a/../win32a/pdcscrn.c:1839: undefined reference to HandleBlockCopy' ...\PDCurses\win32a/../win32a/pdcscrn.c:1994: undefined reference toHandleMenuToggle'
...\PDCurses\win32a/../win32a/pdcscrn.c:2006: undefined reference to show_mouse_rect' ...\PDCurses\win32a/../win32a/pdcscrn.c:2022: undefined reference to milliseconds_since_1970' pdcurses.a(pdcscrn.o): In function PDC_scr_open': ...\PDCurses\win32a/../win32a/pdcscrn.c:2305: undefined reference toset_up_window'
collect2.exe: error: ld returned 1 exit status |

I haven't tried other platforms but if I've understood the semantics of static inline
correctly I don't think this should break anything, it seems to me that static inline is
the correct choice in exactly this case (function defined and used in the same
translation unit and nowhere else).

As far as I understand it what gcc does here is when optimizations are turned off, it
doesn't inline anything. And a bare inline without the static keyword indicates that the
function may be defined externally so the inline version simply gets ignored. And
because the function isn't declared elsewhere linking fails.


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

https://github.com/Bill-Gray/PDCurses/pull/2

    Commit Summary


Reply to this email directly or view it on GitHub
https://github.com/Bill-Gray/PDCurses/pull/2.

@ihalila
Copy link
Contributor Author

ihalila commented Mar 9, 2016

Great, glad to be of help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants