[PATCH] D151553: [clang] Fix consteval operators in template contexts

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Sema/TreeTransform.h:11988
 
-  return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
-                                                 E->getOperatorLoc(),
-                                                 Callee.get(),
-                                                 First.get(),
-                                                 Second.get());
+  bool RequiresADL = false;
+  Expr *Callee = E->getCallee();
----------------
I don't think this is useful, it is only used L11999 but not in L11994, and in either cases you can use `ULE->requiresADL()`


================
Comment at: clang/lib/Sema/TreeTransform.h:12015
+  return getDerived().RebuildCXXOperatorCallExpr(
+      E->getOperator(), E->getOperatorLoc(), Callee->getBeginLoc(), RequiresADL,
+      Functions, First.get(), Second.get());
----------------
here i think RequiresADL is always false


================
Comment at: clang/lib/Sema/TreeTransform.h:15216-15217
 
-  if (Op == OO_Subscript) {
-    SourceLocation LBrace;
-    SourceLocation RBrace;
-
-    if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Callee)) {
-      DeclarationNameLoc NameLoc = DRE->getNameInfo().getInfo();
-      LBrace = NameLoc.getCXXOperatorNameBeginLoc();
-      RBrace = NameLoc.getCXXOperatorNameEndLoc();
-    } else {
-      LBrace = Callee->getBeginLoc();
-      RBrace = OpLoc;
-    }
-
-    return SemaRef.CreateOverloadedArraySubscriptExpr(LBrace, RBrace,
-                                                      First, Second);
-  }
+  // FIXME: The following code for subscript expression is either never executed
+  // or not tested by check-clang.
+  if (Op == OO_Subscript)
----------------
Maybe it would be worth investigating that further before merging the PR? Although the pr is a clear improvement so I'll let you decide what to do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553