aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Sema/DeclSpec.h:1806
+// typename is allowed (C++2a [temp.res]p5]).
+enum class ImplicitTypenameContext {
+ No,
----------------
ayzhao wrote:
> aaron.ballman wrote:
> > Not opposed to this construct, but I am worried about how well it will
> > scale. I don't know that we want to add a bunch of named enums that all
> > boil down to a bool. (If someone thinks they have good ideas here, that'd
> > be a good RFC topic for Discourse because we have a ton of interfaces that
> > take a bunch of bools.)
> Ack. IIRC this `enum` was created as a result of [this
> comment](https://reviews.llvm.org/D53847?id=198139#inline-545979) by @rsmith
> expressing concern over adding an additional `bool` parameter to a function
> with a lot of preexisting `bool` args.
> Ack. IIRC this enum was created as a result of this comment by @rsmith
> expressing concern over adding an additional bool parameter to a function
> with a lot of preexisting bool args.
Yeah, I spotted his suggestion (and am totally fine with implementing it here
as you've done). I was mostly thinking about the future when someone tries to
do this for every set of 2 or more bool parameters. :-D
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5592
+bool Parser::isConstructorDeclarator(bool IsUnqualified, bool DeductionGuide,
+ bool IsFriend) {
TentativeParsingAction TPA(*this);
----------------
erichkeane wrote:
> 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.
That's a really neat idea! If you want to work it up into something that could
be plausible to add to ADT, I think it's worth an RFC to add the interface. I'm
guessing the diagnostic behavior of that would be kind of gross, but once we
move to C++20 we'd be able to use string literal template arguments directly
and get better diagnostic behavior. The critical part is that the code is
readable and we get diagnostics when passing an argument to the wrong parameter.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53847/new/
https://reviews.llvm.org/D53847
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
