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

base/mpi.h: also instantiate for signed long long int #16527

Merged
merged 1 commit into from Jan 24, 2024

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Jan 23, 2024

Vaguely in reference to #16493

@tamiko tamiko added this to the Release 9.6 milestone Jan 23, 2024
@tamiko
Copy link
Member Author

tamiko commented Jan 23, 2024

/rebuild

Copy link
Member

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about MPI_DEVICE_SCALARS for consistency?

@tamiko
Copy link
Member Author

tamiko commented Jan 23, 2024

@masterleinad Good point. Updated.

@tamiko tamiko force-pushed the add_missing_instantiation_2 branch from 469d14c to c3647f8 Compare January 23, 2024 21:13
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, it is true that on all systems we work on, two out of int, long int, and long long int are the same. Does the compiler not complain in those cases?

Should we instead instantiate for int and int64_t?

@tamiko
Copy link
Member Author

tamiko commented Jan 23, 2024

@bangerth I think according to the standard int, long int and long long int are distinct types, meaning:

template <typename C> void foo(){};
template void foo<int>();
template void foo<long int>();  // ok
template void foo<long long int>(); // ok
template void foo<std::int64_t>(); // error: duplicate instantiation

@drwells
Copy link
Member

drwells commented Jan 24, 2024

It would be deeply weird if

static_assert(std::is_same_v<long int, long long int>);

succeeded on some platforms but failed on others.

@bangerth
Copy link
Member

Not weird at all. std::is_same<int,std::int16_t> is also true on some platforms and false on others. The length of integers has always been platform dependent. On some platforms, long int is 32 bit and on others it is 64 bit. long long int is pretty generally 64 bit, but I don't think the standard guarantees that it is.

@tamiko
Copy link
Member Author

tamiko commented Jan 24, 2024

The difference is that std::intN_t is allowed to be a typedef whereas int, long, and long long must be different types. Even if they are functionally the same.

@drwells
Copy link
Member

drwells commented Jan 24, 2024

It's an interesting question and I had to look it up to be sure. In 17.4.2 all the cstdint types (int16_t et al) are declared with using, whereas in 6.8.2 signed char, short int, etc are "the five standard signed integer types".

@tamiko tamiko merged commit cf47f8b into dealii:master Jan 24, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants