TApi in definition of ALPAKA_UNIFORM_CUDA_HIP_CHECK

[chillenzer]

For unrelated reasons, I've just been skimming through alpaka and found this line:

# define ALPAKA_UNIFORM_CUDA_HIP_RT_CHECK_IMPL(cmd, throw, ...)                                                    \
// ...
::alpaka::uniform_cuda_hip::detail::rtCheckLastError<TApi, throw>(                                        \
// ...

Do I see it correctly that this preprocessor macro uses a template argument name TApi it never defines? That sounds borderline suicidal. I really don't want to be that pitiful person unconsciously pulling the trigger on this beautifully designed footgun by using T_Api (or anything else) for my template parameter.

[psychocoderHPC]

With C++20 we can most likely remove all macros because we can get the line and file via native C++.

[chillenzer]

That would be awesome! Let's wait for that then.

[fwyzard]

Feel free not to use internal implementation details in your code.

[chillenzer]

It might be an implementation detail but I'm not concerned with a user messing it up.

$  grep -r "ALPAKA_UNIFORM_CUDA_HIP_RT_CHECK" | wc -l
100

That's 100 opportunities for any developer involved in this to mess something up that will be horrible to debug. Not saying that this needs an immediate fix but raising awareness might already save us a few developer hours.

[fwyzard]

Then I would suggest to use less triggering language than

That sounds borderline suicidal.

and

I really don't want to be that pitiful person unconsciously pulling the trigger on this beautifully designed footgun by using T_Api (or aything else) for my template parameter.

Not saying that this needs an immediate fix

(Un)fortunately macros cannot be namespaced or templated, so either we assume that the template parameter is TApi, or we pass it as an extra argument to the macro.

If you want to look into it, @mehmetyusufoglu has written an alternative version as part of a recent PR that takes the latter.

I would argue against using it in the general case, because I think it would make the code more verbose for no real gain, but your opinion may be different.

[chillenzer]

You are right. I'm very sorry. I enjoy colourful and pictorial language occasionally sacrificing precision for the sake of (what I consider) funny expressions. I didn't consider the effect of my language on other people. I promise to use more neutral language in the future.

(Un)fortunately macros cannot be namespaced or templated, so either we assume that the template parameter is TApi, or we pass it as an extra argument to the macro.

Yes, it's a string manipulation language that's in no way related to the semantics of the C++ core language. And my point is that we currently have an (at least) two-level string manipulation that couples the naming convention in one arbitrary location of the code base to 100 other places which (considered in isolation) have no way of inferring the existence of this coupling.

Having it as an extra argument is the obvious way to resolve this and is definitely the technically correct way to do it. But I could imagine that the trade-off of "if it ain't broken, ain't fix it" and we guard against a (potentially unlikely) future mistake might still be in favour of the former. I'm not yet involved enough into the development of alpaka to decide that.

If you want to look into it, @mehmetyusufoglu has written an alternative version as part of a recent PR that takes the latter.

Would you be so kind to provide a link?

[mehmetyusufoglu]

I can explain, I guess I had squashed but I know what I did. Mehmet chillenzer ***@***.***>, 26 Mar 2024 Sal, 15:20 tarihinde şunu yazdı:

…

You are right. I'm very sorry. I enjoy colourful and pictorial language occasionally sacrificing precision for the sake of (what I consider) funny expressions. I didn't consider the effect of my language on other people. I promise to use more neutral language in the future. (Un)fortunately macros cannot be namespaced or templated, so either we assume that the template parameter is TApi, or we pass it as an extra argument to the macro. Yes, it's a string manipulation language that's in no way related to the semantics of the C++ core language. And my point is that we currently have an (at least) two-level string manipulation that couples the naming convention in one arbitrary location of the code base to 100 other places which (considered in isolation) have no way of inferring the existence of this coupling. Having it as an extra argument is the obvious way to resolve this and is definitely the technically correct way to do it. But I could imagine that the trade-off of "if it ain't broken, ain't fix it" and we guard against a (potentially unlikely) future mistake might still be in favour of the former. I'm not yet involved enough into the development of alpaka to decide that. If you want to look into it, @mehmetyusufoglu <https://github.com/mehmetyusufoglu> has written an alternative version as part of a recent PR that takes the latter. Would you be so kind to provide a link? — Reply to this email directly, view it on GitHub <#2252 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUYXIJKFP2HMTNWHL6Q43L3Y2F7ZHAVCNFSM6AAAAABFIPBJQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRQGU3DCMRYGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[chillenzer]

Okay. Let's talk it over in the office then.

[fwyzard]

I enjoy colourful and pictorial language occasionally sacrificing precision for the sake of (what I consider) funny expressions.

Understood... next time I'll take in that spirit!