mittinatten / freesasa

C-library for calculating Solvent Accessible Surface Areas
http://freesasa.github.io/
MIT License
107 stars 36 forks source link

Allow forward declaration of freesasa_parameters #64

Closed mlund closed 3 years ago

mlund commented 3 years ago

This allows freesasa_parameters to be forward declared in C++ programs. It is currently a typedef to an unnamed struct which cannot be forward declared as discussed at

https://stackoverflow.com/questions/10249548/forward-declaring-a-typedef-of-an-unnamed-struct

~The modification works for my purpose but I'm unable to run the tests as the configure script seems to be missing from the current main branch (mismatch with build instructions in the README.md). If you can direct me to how to build these I'm happy to do more testing.~ Downloading the latest release tar-ball with the configure script, I detected that struct needs to be inserted in all further instances of freesasa_parameters. Is this an acceptable solution, or could something else be done to remedy the issue?

mittinatten commented 3 years ago

Hi, I am on vacation, away from my computer, for a few more weeks, so can’t help much right now. But, have you run autoreconf? You have to run it to generate the configure script if you clone the repo. (It is mentioned in the Readme).

I think your solution will break the C code, the typedef needs to be there, but there should be some way that works for both C and C++. (Like what was done in the original question on SO)

Simon

tis 20 juli 2021 kl. 23:20 skrev Mikael Lund @.***>:

This allows freesasa_parameters to be forward declared in C++ programs. It is currently a typedef to an unnamed struct which cannot be forward declared as discussed at

https://stackoverflow.com/questions/10249548/forward-declaring-a-typedef-of-an-unnamed-struct

The modification works for my purpose but I'm unable to run the tests as the configure script seems to be missing from the current main branch (mismatch with build instructions in the README.md). If you can direct me to how to build these I'm happy to do more testing.

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

https://github.com/mittinatten/freesasa/pull/64 Commit Summary

  • Remove typedef to unnamed struct

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mittinatten/freesasa/pull/64, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVV4HHIK6ED3NSN6UTCR2TTYXSA3ANCNFSM5AWT7XLA .

mlund commented 3 years ago

C code compiles if struct is added before each use which I seems quite tedious. I don't know if there's another way that would allow simultaneous C and C++ forward declarations, but I now use the following in my code:

.h

struct freesasa_parameters_fwd; // workaround for freesasa unnamed struct that cannot be forward declared

.cpp

#include <freesasa.h>
struct freesasa_parameters_fwd : public freesasa_parameters {};
mittinatten commented 3 years ago

A bit difficult to do this from the phone 😊, can’t run any code, but what about something like:

typedef struct _freesasa_parameters { … } freesasa_parameters;

As in the SO question (with underscore instead of “tag”). That shouldn’t break anything.

Perhaps the C++ variant could even be included with some compiler directives in the same header?

mån 26 juli 2021 kl. 11:49 skrev Mikael Lund @.***>:

C code compiles if struct is added before each use which I seems quite tedious. I don't know if there's another way that would allow simultaneous C and C++ forward declarations, but I now use the following in my code: .h

struct freesasa_parameters_fwd; // workaround for freesasa unnamed struct that cannot be forward declared

.cpp

include struct freesasa_parameters_fwd : public freesasa_parameters {};

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mittinatten/freesasa/pull/64#issuecomment-886550018, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVV4HCZ57X6WXKLPYQOPDDTZUVRLANCNFSM5AWT7XLA .

mittinatten commented 3 years ago

Sorry, forgot about this.

But I think I have a solution that works for both C and C++, at least the code compiles properly.

struct freesasa_parameters {
  /* ... */
};

#ifndef __cplusplus
typedef struct freesasa_parameters freesasa_parameters;
#endif

Can you verify that this works for you too and modify the PR?

I guess ideally we should do the same change for the other structs in freesasa.h.

mlund commented 3 years ago

Thanks for looking into this; I'll update the PR accordingly.

mittinatten commented 3 years ago

Great! Thanks for your contribution.

(Såg först nu att du också är Lundensare!)

mlund commented 3 years ago

Ahh, syntes nok dit github handle så meget svensk ud :-) Vi anvender freesasa som en del af energifunktionen i Monte Carlo for at beskrive hydrofobe og salt-specifikke effekter.

mittinatten commented 3 years ago

Intressant! Jag trodde inte den var snabb nog för att användas i simuleringer. Jag doktorerade hos Anders Irbäck i 2009, så Monte Carlo känner jag till :)

mlund commented 3 years ago

:-) vores modeller overlapper ganske meget med Anders'. Ja, det går ikke specielt hurtigt, men nogen gange gør det ikke så meget. Det kunne være spændende hvis freesasa kunne beregne sasa for et udvalg af partikler da vi jo i MC normalt ikke flytter alle partikler samtidigt. Et andet mindre problem er at vi kopierer alle positioner og radier mellem hver energi beregning, da vores positioner ligge i en class Particle.