cor3ntin added inline comments.

================
Comment at: clang/test/SemaCXX/overloaded-operator-decl.cpp:64
+class E {};
+void operator+(E, ...) {} // expected-error{{overloaded 'operator+' cannot be 
variadic}}
+void d() { E() + E(); }
----------------
aaron.ballman wrote:
> Fznamznon wrote:
> > aaron.ballman wrote:
> > > I think it might make sense to extend the test coverage for the other 
> > > operators you can overload, just to demonstrate we diagnose them all 
> > > consistently. WDYT?
> > Okay, while trying to add more test cases I discovered that following
> > ```
> > class E {};
> > bool operator<(const E& lhs, ...);
> > auto operator<=>(const E& lhs, ...);
> > 
> > void d() {
> >   E() < E();
> > }
> > ```
> > crashes even with the patch since there is code searching for best overload 
> > candidate that doesn't consider possibility for them making variadic.
> > The code around overloading is actually pretty inconsistent, somewhere 
> > invalid candidates are considered, and somewhere not, so I spent some time 
> > not knowing what to do.
> > I'm now inclined that we just shouldn't consider invalid candidates like 
> > @shafik 
> > suggests. WDYY?
> > 
> Overload resolution doesn't need to produce a candidate that's viable to 
> call; C++ lets you resolve to the "best viable function" only to say "and 
> that one wasn't good enough either." e.g., 
> http://eel.is/c++draft/over#match.general-3
> 
> I've not yet spotted anything in http://eel.is/c++draft/over that says 
> invalid declarations should/should not be added to the initial candidate set. 
> I *think* the intention is that if name lookup can find the name, it goes 
> into the candidate set. Then that set is processed to remove functions that 
> are not viable (http://eel.is/c++draft/over.match.viable). Then we find the 
> best viable function from that set.
> 
> I think we should be keeping the function in the candidate set so long as it 
> matches the rules in http://eel.is/c++draft/over.match.viable even if the 
> function is otherwise not viable. Otherwise, correcting an unrelated issue 
> might change overload resolution to find a completely different function. 
> e.g., in my example above, we'd select `void overloaded(int);` as the best 
> viable function, but when the user corrects the `float` function, we'd change 
> to call that instead. I think it's easier to understand what's going on when 
> picking the `float` overload to begin with and saying "but we can't call that 
> because it's busted".
> 
> CC @cor3ntin @hubert.reinterpretcast @rsmith for some extra opinions, as I'm 
> not certain if I'm interpreting the standard correctly or not.
I'm not sure how much the standardese matter here as the declaration of the 
operators are ill-formed anyway.
But from a QOI perspective, i think keeping in the set everything the users 
might reasonably expect to be considered makes sense to me, as it *should* lead 
to better diagnostics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156244

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to