search

codeplea / genann

simple neural network library in ANSI C
https://codeplea.com/genann
zlib License
1.99k stars 237 forks source link

Removed inline function specifiers #35

Closed jflopezfernandez closed 5 years ago

jflopezfernandez commented 5 years ago

This pull request fixes issue #28, which was unrelated to the build environment the user was building on. Instead, the inline function specifiers meant that a build of only one header and one source file would build correctly, even in the case of a library, but since the current master branch has no library build system, users were including the files genann.c and genann.h to their projects.

This meant that projects were trying to link to an inline function definition that they did not have access to because it was defined in a different compilation unit. As stated in the commit message, I thought about moving the functions to the header, but I figured the best way to move forward for now is to simply remove the inline specifier while we profile builds both with and without them, and see what the results say.

The alternative would have required moving a significant number of macros to the header file as well, which could have had a waterfall effect on other builds, since anyone linking to the library would have needed the header, which was now full of new macros. Another option would have been to remove the unused specifiers on the variables, but then there would be no point in having the macros in the first place.

This option was the simplest one, and until we profile the builds, I think it's the one that makes the most sense.

codeplea commented 5 years ago

If you want to rebase this PR so it only removes the inlines, and nothing else, I'll merge it.

jflopezfernandez commented 5 years ago

Sure, I'll do it tonight. That's actually what I originally meant to do, but I forgot I had already merged my first pull request to master.

jflopezfernandez commented 5 years ago

As I mentioned in the pull request I just created, #36, I was having trouble rebasing this branch, so I'm closing this pull request. The newer request is rebased to master as you requested and changes only the inline function specifications for the four functions which had them; they are now removed.

I did not push any changes to the documentation, as you did say 'and nothing else', but if you want me to add an update/heads up for any people who may have run into this problem, I don't mind it at all. I think it would definitely be helpful.

I'll also close the other pull request for the build system refactor.