Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review how alias data-types are used #344

Closed
jougs opened this issue May 18, 2016 · 4 comments
Closed

Review how alias data-types are used #344

jougs opened this issue May 18, 2016 · 4 comments
Assignees

Comments

@jougs
Copy link
Contributor

jougs commented May 18, 2016

Currently, we have a lot of alias datatypes that make the code less readable to normal C++ programmers and only help little for NEST experts. Because the aliases are just typedefs, they do not provide any additional type safety.

The following list of aliases is defined in nest_types.h:

index -> size_t
tic_t -> longlong or long (longest integer type available)
syn_index -> unsigned char
targetindex -> unsigned short
thread -> int_t
rport -> long_t
port -> long_t
weight -> double_t
delay -> long_t 

The task is to review these types and replace them with standard types where they don't make sense.

This was known as trac90. See also #343.

@heplesser
Copy link
Contributor

These type aliases are informative, but since they are just aliases of plain C++ datatypes, the compiler performs no type checking. So they can create a false sense of safety on the part of the programmer.

@heplesser
Copy link
Contributor

In the Open NEST Developer VC today, we concluded that replacing the alias types with proper classes with explicit constructors would will not incur run-time penalties and increase type safety.
We will do a check to see how many changes in NEST code this would require and return to the issue in the next Developer VC.

@apeyser
Copy link
Contributor

apeyser commented Aug 8, 2016

I've looked at it and thought about it:

To add type-safety without too many problems (such as handling constexpr's, explicit cast expressions, ..) would require c++11.

On the other hand, with c++ 11, we could simply require curly-braced construction/initialization {} to solve all the problems of implicit narrowing, and if we wanted further safety it would be fairly trivial to construct these classes.

So this is essentially a pre-11 problem that is not solvable without producing more problems than are solved currently. +1 to moving onto c++ 11.

@heplesser
Copy link
Contributor

@apeyser Thanks for the quick check! Based on that and the arguments raised during the discussion today, I would then suggest to leave things as they are.

Would you open a new issue concerning a potential transition to C++11? The main issue here is compiler support, given that cutting-edge HPC hardware typically seems to be anything but cutting edge when it comes to compilers ...

@apeyser apeyser mentioned this issue Aug 8, 2016
@apeyser apeyser closed this as completed Aug 8, 2016
@apeyser apeyser reopened this Aug 8, 2016
@apeyser apeyser closed this as completed Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants