Misleading header file in C extension (cmontecarlo1.h)

[yeganer]

For some reason there is a header file called cmontecarlo1.h declaring a single function ( which would produce compiler errors if the order of import in cmontecarlo.c was changed).

I think this function declaration should be moved to cmontecarlo.h and in that process I suggest to check if there are other potential errors.

I also suggest adding missing declarations from functions in cmontecarlo.c to it's header.

[wkerzendorf]

sure - I would also like to know how to convert an issue to a PR. I think this issue is a good candidate.

[yeganer]

I haven't found any new information but it seems this feature isn't available anymore.
See discussion here: mislav/hub@4f70dd1

When i am done with some implementation i'll create a PR. That seems the best approach.

[orbitfold]

I'd ask whoever added it why they did it. It's easy to assume people made a
mistake but sometimes there are reasons you may not be aware of
On 12 Feb 2016 18:01, "yeganer" [email protected] wrote:

I haven't found any new information but it seems this feature isn't
available anymore.
See discussion here: mislav/hub@4f70dd1
mislav/hub@4f70dd1

When i am done with some implementation i'll create a PR. That seems the
best approach.

—
Reply to this email directly or view it on GitHub
#489 (comment).

[yeganer]

When i noticed it I tried to think of a reason and didn't find one. I moved the declaration and deleted cmontecarlo1.h and it compiled and ran without errors.

I'm happy to hear a reason and be proven wrong.

[orbitfold]

You should see when it was added. I think it was added as part of an effort
to improve performance around this September. By someone from mpa
On 12 Feb 2016 18:45, "yeganer" [email protected] wrote:

When i noticed it I tried to think of a reason and didn't find one. I
moved the declaration and deleted cmontecarlo1.h and it compiled and ran
without errors.

I'm happy to hear a reason and be proven wrong.

—
Reply to this email directly or view it on GitHub
#489 (comment).

[yeganer]

It was added by @mreinecke during the performance improvements.
But i don't see a reason for that function to have it's own header.
To me it looks like an artifact from some test or something like that.

[wkerzendorf]

okay @mreinecke is there a reason for cmontecarlo1.h

[orbitfold]

I'm curious too actually

[mreineck]

The reason for this curious construct was (if I remember correctly) that the function line_search() is defined (not only declared) in cmontecarlo.h, but it does not have the "static" attribute. As a consequence, if cmontecarlo.h is included by two different C files, the function line_search() turns up in each of the .o files compiled from these, and the linker will most likely complain about a function being defined twice.
There are different ways to avoid this:

• declare line_search() to be "static"
• move the definition of line_search() to montecarlo.c, but leave the declaration in montecarlo.h
• play confusing games with the header files like I did.

I must admit that I don't remember why I went for this awkward solution. There probably was a reason, but maybe not a very good one.

[orbitfold]

Also @yeganer we need to be sure the code compiles on both gcc and clang. In the past there were some issues with incompatibilities between the two.

[mreineck]

I can help with compatibility problems ... just let me know if anything turns up!