[PATCH] D58998: Add declarations of explicit specializations and make primary template deleted

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 7 13:46:37 PST 2019


dblaikie added a comment.

In D58998#1422074 <https://reviews.llvm.org/D58998#1422074>, @tkoeppe wrote:

> In D58998#1421799 <https://reviews.llvm.org/D58998#1421799>, @dblaikie wrote:
>
> > In D58998#1419205 <https://reviews.llvm.org/D58998#1419205>, @jyknight wrote:
> >
> > > Is there any reason this needs to be a template -- can't these just be changed to function overloads, instead of template specializations?
> >
> >
> > I'll second this - and I'm happy to take ownership (if it's needed) of the decision. This code came in with this specialization approach, and I'm going to guess it wasn't for any particularly nuanced reason & is fine to change to overloads as would be more common here.
>
>
> The only reason I can think of is that we wanted to keep the overload set small (or trivial), but I don't know if that's significant. (Richard said he doesn't care either way.)
>
> What shall we do now? Would you like to take this change and then refactor, or immediately change this to overloads?


I'd just immediately switch to overloads as a means to fix the original issue rather than do a two-step.


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

https://reviews.llvm.org/D58998





More information about the llvm-commits mailing list