ilya-biryukov added a comment.
A few ideas for tests:
- Static operators. Technically disallowed by the standard, but Clang seems to
recover without dropping the member, so it seems we can actually look it up.
struct X {
bool operator ==(X const&) const;
static bool operator !=(X const&, X const&);
};
this could potentially lead to a crash if the parameter list for the operator
is empty:
struct X {
bool operator ==(X const&) const;
static bool operator !=();
};
- template functions with non-matching template heads (so they don't
"correspond")
bool operator ==(X, X);
================
Comment at: clang/include/clang/Sema/Sema.h:3768
NamedDecl *Dest = nullptr);
-
+ // bool HasSameExclaimEqual(const FunctionDecl *FD, SourceLocation OpLoc,
+ // Expr *RHSArg);
----------------
This looks like a leftover from previous version. Remove?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:887
-bool OverloadCandidateSet::OperatorRewriteInfo::shouldAddReversed(
- OverloadedOperatorKind Op) {
----------------
Why do we need to move this from `OperatorRewriteInfo` to
`OverloadCandidateSet`?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:892
+static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
+ ArrayRef<Expr *> Args,
+ const FunctionDecl *EqEqFD) {
----------------
We only use `Args[1]`, let's pass `Expr*` instead of an array.
The standard calls this expression "first operand", so let's sync the naming
and accept the `Expr *FirstOperand`?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:893
+ ArrayRef<Expr *> Args,
+ const FunctionDecl *EqEqFD) {
+ assert(EqEqFD->getOverloadedOperator() ==
----------------
NIT: maybe rename to `EqFD`?
`EqEqFD` is somewhat hard to grasp and I don't think there any possibility for
confusion with assignment here.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:912
+ QualType RHS = Args[1]->getType();
+ if (auto *RHSRec = RHS->getAs<RecordType>()) {
+ LookupResult Members(S, NotEqOp, OpLoc,
----------------
NIT: use [early
exits](https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code):
```
auto *RHSRec = RHS->getAs<RecordType>();
if (!RHSRec)
return true;
// <else branch with reduced nesting>
```
================
Comment at: clang/lib/Sema/SemaOverload.cpp:918
+ for (NamedDecl *Op : Members)
+ if (S.Context.hasSameUnqualifiedType(
+ MD->getParamDecl(0)->getType(),
----------------
Could we implement the "corresponds" check from
[(basic.scope.scope)p4](https://eel.is/c++draft/basic.scope.scope) directly?
This should address the existing FIXMEs about `const` members and template
functions.
================
Comment at:
clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp:141
+namespace P2468R2 {
+//=============Problem cases prior to P2468R2 but now intentionally
rejected=============
+namespace no_more_problem_cases {
----------------
NIT: the rest of the file uses plain comments. Maybe stick to existing style
and avoid heading padded with `=====`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134529/new/
https://reviews.llvm.org/D134529
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
