Problem 1257 - Request to remove "using" declarations from G4PhysicalConstants.hh and G4SystemOfUnits.hh
Summary: Request to remove "using" declarations from G4PhysicalConstants.hh and G4Syst...
Status: RESOLVED WONTFIX
Alias: None
Product: Geant4
Classification: Unclassified
Component: global/management (show other problems)
Version: 9.4
Hardware: All All
: P5 enhancement
Assignee: Gabriele Cosmo
URL:
Depends on:
Blocks:
 
Reported: 2011-10-18 05:39 CEST by jasondet
Modified: 2011-10-24 15:58 CEST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this problem.
Description jasondet 2011-10-18 05:39:38 CEST
G4PhysicalConstants.hh and G4SystemOfUnits.hh port the CLHEP system of units and physical constants into Geant4. They do so by #including the corresponding CLHEP header, and then listing "using" declarations for (perhaps) all of the variables in the CLHEP namespace. As has been discussed at length in numerous coding forums, the practice of employing "using" declarations and directives is widely frowned upon, especially in header files, as they bring the namespace variables into the global namespace. See for example

http://www.cplusplus.com/forum/beginner/25538/
http://www.inspirel.com/vera/ce/doc/rules/T018.html
http://drdobbs.com/184401782
http://stackoverflow.com/questions/7134403/using-namespace-std-in-header-file
http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-a-bad-practice-in-c
http://stackoverflow.com/questions/3186226/header-files-and-using-namespace
http://www.velocityreviews.com/forums/t286650-using-namespace-directive-in-header-file.html
http://www.velocityreviews.com/forums/t570222-header-file-and-using-directive.html

Worse yet, there is no method to "unuse" declarations once declared, so users must resort to special tricks to avoid cluttering their namespace, as discussed for example here:

http://stackoverflow.com/questions/167862/how-can-i-unuse-a-namespace

If the point of putting all of these declarations into the Geant4 global namespace is to save developers the trouble of having to writing their own using declarations and directives, then the arguments in the above webpages apply and it would be preferable to remove these and sweep the code and add using directives / declarations as applicable to the .cc files that need them.

If the point is to restrict the Geant4 dependencies on CLHEP to the smallest number of files possible, and "rebrand" the CLHEP constants as Geant4 variables, it would be better to nest the CLHEP namespace or simply rename it as one specific to Geant4 (maybe use "Geant4"). Users can then do, for example, "using namespace Geant4;" or "G4double length = 10*Geant4::m;" to maximize the forward-compatibility of their code.

As it is, the present list of using declarations cause an enormous amount of shadowing in our code and (more importantly) the libraries we link against, as variable names like "m" and "s" are quite common. We compile with the -Wshadow option in gcc to catch our developers making shadowing mistakes especially in derived classes. Sifting through the numerous warnings resulting from the Geant4 headers gives us a lot of extra work to do. It would be helpful if these headers could be updated to conform to more widely agreed-upon coding standards.
Comment 1 Gabriele Cosmo 2011-10-23 23:40:08 CEST
The use of physical constants and units as defined is part of the user's API since the beginning in Geant4, well before namespaces became part of the C++ standard and implemented in all compilers. Therefore, their use in the global namespace -cannot- be easily avoided without imposing massive migration in the user codes. We can try to reduce their use within the internal headers in Geant4, but cannot modify the API at this point in time; G4PhysicalConstants.hh and G4SystemOfUnits.hh are meant to allow users's API compatibility.
Comment 2 jasondet 2011-10-24 06:10:51 CEST
That's too bad. For the record, I would like to add the following arguments.

As a 10+ year user of G4, my experience is that migrating user code is an expected step when upgrading to any new G4 release. For this case, simple remedies could be introduced to ease the migration, e.g. by allowing users to add something like "using namespace Geant4;" at the top of files that need it. A CPP directive could also easily be implemented and set during configuration to keep these variables in the global namespace if a user desires. So I think that with a little effort this change could be implemented in a way that does not require "massive" user migration.

As for API compatibility, while this may have been implemented to "allow users' API compatibility" with G4, by putting variables in the global namespace it breaks compatibility with other APIs, making the Geant4 API less compatible and more painful to use. So that point really did not make sense to me.

Anyway thank you for considering my suggestion. Perhaps at least keep this in mind for future releases.

Sincerely,
Jason Detwiler
Comment 3 Gabriele Cosmo 2011-10-24 15:58:38 CEST
Hi Jason,

the current implementation makes sense as long as the original API for physical constants and units is _the_ valid one users should adopt.
I perfectly understand your concerns, as this issue has been matter of discussion when namespaces were introduced in CLHEP and was explicitly decided to keep the current API, following the input from the users community (particularly HEP experiments).
We may envisage at some point to move to a coherent (and perhaps selectable) use of the namespaces for this, in a way similar to what you suggest, but cannot promise this to happen in a too near future...

Cheers, Gabriele