Action register

[Iximiel]

Description

• added to ActionRegister.h a new template class ActionRegistration that works like the, PLUMED_REGISTER_ACTION without the need of the preprocessor.
• changed PLUMED_REGISTER_ACTION in ActionRegister.h: now is a shortcut to a declaration: #define PLUMED_REGISTER_ACTION(classname,directive) ActionRegistration<classname> directive##Registerer(#directive); instead of a whole class definition+declaration
• changed PLUMED_REGISTER_ACTION in the sources with: for a in */; do sed -r 's%(PLUMED_REGISTER_ACTION\(.*,.*)\"([A-Z_0-9]*)\"(\))%\1\2\3%g' "$a"/*cpp -i; done the string constant is automatically stringified by the "#" operator in the preprocessor
  • it is a sed away to be completely exchanged by the explicit template tool
  • even if, I think that keeping the macro will keep the operation less verbose

In the case of multiple definitions, like XDistances and XYTorsion, the compiler will only declare one specialized ActionRegistration, but with multiple differently named instances without errors, for example

ActionRegistration<XDistances> xDistancesRegister("XDISTANCES");
ActionRegistration<XDistances> yDistancesRegister("YDISTANCES");
ActionRegistration<XDistances> zDistancesRegister("ZDISTANCES");

Target release

I would like my code to appear in the C++17 release (it uses a inline constexpr variable).

Type of contribution

• changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
• changes to a module not authored by you
• new module contribution or edit of a module authored by you

Copyright

• [C] I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.

• the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.

Tests

• I added a new regtest or modified an existing regtest to validate my changes.
• I verified that all regtests are passed successfully on GitHub Actions.

[GiovanniBussi]

@Iximiel why this additional open brace after the comment?

[Iximiel]

I'm not a vim user, but is what I understood to do from this

[GiovanniBussi]

That's only for macros. Here the braces are already available, vim can match them.

[GiovanniBussi]

@Iximiel I do not understand the removal of the quotes. Is it to allow multiple names for multiple registrations?

Notice that this has two negative effects:

1. It requires an extensive change on the code base, including contributed modules, with possible issues when merging or rebasing from one version to another
2. To me the syntax of the registration is actually less intuitive. If it's a string, one expect to pass a string.

I think using the line number to distinguish the two names (as it is now) is a cleaner solution.

[GiovanniBussi]

We usually use capital letters for classes (PlumedRegisterer)

[Iximiel]

i corrected that in the fourth commit, is that not up?

[GiovanniBussi]

Maybe I was reviewing an older commit sorry for this

[Iximiel]

Yes, I wanted to allow multiple names by using the wanted directive as a variable name and use the # to get the cstring for the directive

I suspected that removing the quotes was a too heavy change

If you prefer, I'll reverting that and substituting:
#define PLUMED_REGISTER_ACTION(classname,directive) ActionRegistration<classname> directive##Registerer(#directive);

with
#define PLUMED_REGISTER_ACTION(classname,directive) ActionRegistration<classname> classname##__LINE__##Registerer(directive);
as I said is just a sed (or a git rebase) away
so there will be no virtual changes in the other files

[GiovanniBussi]

Yes I think it's cleaner thanks!!!

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.05 ⚠️

Comparison is base (03032e2) 85.78% compared to head (25df5e7) 85.73%.

❗ Current head 25df5e7 differs from pull request most recent head 7016c11. Consider uploading reports for the commit 7016c11 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##            c++17     #935      +/-   ##
==========================================
- Coverage   85.78%   85.73%   -0.05%     
==========================================
  Files         600      600              
  Lines       54724    54437     -287     
==========================================
- Hits        46945    46673     -272     
+ Misses       7779     7764      -15     

Impacted Files	Coverage Δ
src/core/ActionRegister.h	100.00% <100.00%> (ø)

... and 272 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[GiovanniBussi]

You eliminated an indirection. I honestly do not remember anymore why it was there. Are you sure this is ok?

[GiovanniBussi]

I think it's correct. You are just using PLUMED_CONCATENATE below..

[GiovanniBussi]

maybe this can be a standalone commit?

[Iximiel]

removed from this PR