[PATCH] D133659: [Clang] P1169R4: static operator()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Double-checking my understanding of the overload resolution changes: we added a new conversion sequence, but we don't expect that conversion sequence to cause a change in overload resolution in practice? (I'm wondering if there's test coverage we should be adding for those changes.)



================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265
+  if (Intro.hasLambdaCapture())
+    P.Diag(StaticLoc, diag::err_static_lambda_captures);
+}
----------------
aaron.ballman wrote:
> royjacobson wrote:
> > cor3ntin wrote:
> > > We might want to add a note showing where the capture list is.
> > I added a note, but the captures list is always just before the static specifier, so I'm not sure how useful it is. WDYT?
> > 
> FWIW, I'm not sure the note adds a whole lot of value. @cor3ntin, do you have a code example in mind where you think the note would be clarifying?
I think we should remove the note here -- we can add a note back when we run into a use case where it would be enlightening.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:7027-7029
+    ImplicitConversionSequence ICS;
+    ICS.setStaticObjectArgument();
+    Candidate.Conversions[FirstConvIdx] = ICS;
----------------
Can you replace this with: `Candidate.Conversions[FirstConvIdx].setStaticObjectArgument();` and get the same behavior?


================
Comment at: clang/test/Lexer/cxx-features.cpp:42-48
+#if check(if_consteval, 0, 0, 0, 0, 0, 202106)
+#error "wrong value for __cpp_if_consteval"
+#endif
+
+#if check(multidimensional_subscript, 0, 0, 0, 0, 0, 202110)
+#error "wrong value for __cpp_multidimensional_subscript"
+#endif
----------------
Unrelated changes? (Feel free to land as an NFC change separately though, they look like sensible changes.)


================
Comment at: clang/test/SemaCXX/lambda-unevaluated.cpp:125
+
+#if __cplusplus > 202002L
+namespace StaticLambdas {
----------------
We can remove this now, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659