[PATCH] D62202: Work around a Visual C++ bug

Tue May 21 09:45:46 PDT 2019

dblaikie added a comment.

In D62202#1510449 <https://reviews.llvm.org/D62202#1510449>, @probinson wrote:

> In D62202#1510414 <https://reviews.llvm.org/D62202#1510414>, @dblaikie wrote:
>
> > Technically this violates the LLVM style guide which says "make anonymous namespaces as small as possible, and only use them for class declarations." (preferring static for functions) - https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
>
>
> Which argues for flagging it somehow.

Yeah, if we're going this way I'd certainly advocate having a comment of some kind explaining why it's this way so it doesn't regress.

>> But making code work for the compilers we say we support seems reasonable to me.
>> 
>> What's the compiler bug - can't handle static functions as non-type template parameters in general? Are there other workarounds to consider?
> 
> Correct, static function was not permitted as a non-type template parameter.
>  https://developercommunity.visualstudio.com/content/problem/25334/error-code-c2971-when-specifying-a-function-as-the.html
>  I'm not aware of other workarounds, but I don't claim to be deeply familiar with the darker corners of C++.

I'm wondering if the code itself could be changed somewhat. Non-type template parameters, especially pointer typed ones (especially especially function pointer typed ones), are pretty rare/unusual. It'd be nice, for instance, to be able to pass a(n ideally stateless) lambda but there's no way to default construct them, or get a constexpr function pointer for them until C++17, unfortunately. (& even then you can't use lambdas directly in a template parameter expression, which is unfortunate... probably because mangling?)

Another possibility to keep these helper functions close to their use would be to put them in local classes inside the functions they're helping - and make the utility function a static member of that class? (it's awkward, but it's no more lines than wrapping each one in an anonymous namespace, etc)

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62202/new/

https://reviews.llvm.org/D62202