[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:5592
+bool Parser::isConstructorDeclarator(bool IsUnqualified, bool DeductionGuide,
+                                     bool IsFriend) {
   TentativeParsingAction TPA(*this);
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > shafik wrote:
> > > > Instead of adding yet another `bool` flag maybe we can consider using something like `enum isFriend : bool {No, Yes}`.
> > > > 
> > > > I am sure @aaron.ballman will want to chime in here as well.
> > > Heh, so this is where I get worried about the scalability of using enums for these. We really want to use three different enums here, but do we really want to *add* three different enums? I'm unconvinced.
> > > 
> > > However, if we can come up with some template magic to allow for named bool parameters as a generic interface, that would be valuable to use.
> > I prefer enums over bools TBH, even if we end up with a million of then somewhere.
> > 
> > That said, what about:
> > 
> > https://godbolt.org/z/Kz6jdjobj
> > 
> > ```
> > template<typename SpecificThing>
> > class Is {
> >     Is(bool v) : value(v){}
> >   public:
> >   bool value;
> >   static const Is Yes() { return Is{true};}
> >   static const Is No() { return Is{false};}
> > 
> >   operator bool() { return value; }
> > };
> > 
> > class Friend{}; // #1
> >  
> > void foo(Is<Friend> f) {
> >     if (f) {
> >         ///...
> >     }
> > }
> > 
> > void baz() {
> >     foo(Is<Friend>::Yes());
> > }
> > ```
> > 
> > Adding a 'new' thing is as simple as just adding #1 for anything we care about.  We might want to put them in a namespace of some sort, but perhaps not awful?
> Yeah, this is along the lines of what I was thinking of! However, I'm still concerned about that approach because it involves adding a new type for every situation we have a bool. Empty classes to use as a tag definitely works, but I was hoping we could use a string literal rather than a tag type so that we don't have the extra compile time overhead of adding hundreds of new empty classes. e.g.,
> ```
> void foo(Is<"Friend"> f) {
>   if (f) {
>      // ...
>   }
> }
> 
> void baz() {
>   foo(Is<"Friend">::Yes); // Yay
>   foo(Is<"Enemy">::Yes); // Error for type mismatch with Is<"Friend">
> }
> ```
> However, that might require compiling with C++20 (I don't recall), so it may not be a viable idea.
Yeah, referring to stringliterals is troublesome in C++17.  However, we COULD do that like:

```
void foo(Is<"Friend"_Is> f) {
  if (f) {
     // ...
  }
}

void baz() {
  foo(Is<"Friend"_Is>::Yes); // Yay
  foo(Is<"Enemy"_Is>::Yes); // Error for type mismatch with Is<"Friend">
}```

by making operator _Is return an integer_sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847