aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041
+def err_static_mutable_lambda : Error<
+ "lambda cannot be both mutable and static">;
+def err_static_lambda_captures : Error<
+ "a static lambda cannot have any captures">;
+def note_lambda_captures : Note<"captures declared here">;
----------------
cor3ntin wrote:
> royjacobson wrote:
> > aaron.ballman wrote:
> > > royjacobson wrote:
> > > > aaron.ballman wrote:
> > > > > royjacobson wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > These are semantic errors, not parsing ones. This means these
> > > > > > > will be diagnosed when parsing the lambda rather than when
> > > > > > > instantiating it. I don't think that matters for the cast of
> > > > > > > combining `mutable` and `static`, but I'm less certain about
> > > > > > > "have any captures" because of cases like:
> > > > > > > ```
> > > > > > > template <typename... Types>
> > > > > > > auto func(Types... Ts) {
> > > > > > > return [Ts...] { return 1; };
> > > > > > > }
> > > > > > >
> > > > > > > int main() {
> > > > > > > auto lambda = func();
> > > > > > > }
> > > > > > > ```
> > > > > > > I'm pretty sure that lambda has no captures for that call, but it
> > > > > > > could have captures depending on the instantiation.
> > > > > > >
> > > > > > > Actually, from some off-list discussion with @erichkeane, even
> > > > > > > mutable and static are a problem in code like:
> > > > > > > ```
> > > > > > > template <typename Ty>
> > > > > > > void func(T t) {
> > > > > > > if constexpr (Something<T>) {
> > > > > > > [](){};
> > > > > > > } else {
> > > > > > > [t](){};
> > > > > > > }
> > > > > > > ```
> > > > > > > where the lambda is in a discarded statement.
> > > > > > >
> > > > > > > So I think these might need to change to be Sema diagnostics (and
> > > > > > > we should add some additional test coverage).
> > > > > > From https://eel.is/c++draft/expr.prim.lambda.general#4
> > > > > >
> > > > > > > If the lambda-specifier-seq contains static, there shall be no
> > > > > > > lambda-capture
> > > > > >
> > > > > > So this should be a parsing error. Or maybe I don't understand what
> > > > > > you're saying. There are no static lambdas in your examples so I'm
> > > > > > not sure how they're related.
> > > > > >
> > > > > > So this should be a parsing error. Or maybe I don't understand what
> > > > > > you're saying.
> > > > >
> > > > > Parsing errors are where the grammar disallows something, generally.
> > > > > The rest are semantic diagnostics (e.g., we can parse the construct
> > > > > just fine, but we diagnose when turning it into an AST node because
> > > > > that's the point at which we have complete information about what
> > > > > we've parsed).
> > > > >
> > > > > That said, my concern was mostly around SFINAE situations. My
> > > > > recollection is that SFINAE traps do not cover parsing errors only
> > > > > type substitution errors. So for my first example, I would expect
> > > > > there to be no parsing error despite specifying a capture list
> > > > > because that capture list can be empty when the pack is empty, but we
> > > > > would get a SFINAE diagnostic when rebuilding declaration during
> > > > > template instantiation if the pack was not empty.
> > > > >
> > > > > > There are no static lambdas in your examples so I'm not sure how
> > > > > > they're related.
> > > > >
> > > > > Sorry, I was being lazy with my examples and showing more about the
> > > > > capture list. Consider:
> > > > > ```
> > > > > template <typename... Types>
> > > > > auto func(Types... Ts) {
> > > > > return [Ts...] static { return 1; };
> > > > > }
> > > > >
> > > > > int main() {
> > > > > auto lambda = func();
> > > > > }
> > > > > ```
> > > > >
> > > > Like I said,
> > > >
> > > > > If the lambda-specifier-seq contains static, there shall be no
> > > > > lambda-capture
> > > >
> > > > `Ts...` is still a syntactic `lambda-capture`, even if it's
> > > > instantiated as an empty pack. I don't see how that's not a parsing
> > > > error.
> > > Ah, I think I maybe see where the confusion is coming in, now: you think
> > > my example should be diagnosed and I think my example should be accepted.
> > >
> > > Based on the standards wording, I think you're right. The standard
> > > specifically uses "lambda-capture" as a grammar term, so it *is* a
> > > parsing error at that point.
> > >
> > > Based on my understanding of the intent behind the feature, I think I'm
> > > right and there's a core issue here. I don't think we intended to
> > > prohibit empty packs from being captured as non-SFINAEable error (and in
> > > off-list talks with @erichkeane, he agrees). as that allows different
> > > specializations of the function containing the lambda, which could be of
> > > use. However, I'm not 100% sure on the intent. CC @hubert.reinterpretcast
> > > as the C++ standards conformance code owner to see if he has an opinion
> > > or other recollections here.
> > I can see something like
> >
> > ```
> > [Ts...] static(sizeof...(Types) == 0) {}
> > ```
> >
> > being eventually useful, but as long as the `static` is non-conditional
> > allowing a capture list with an empty pack doesn't make sense to me. I
> > think you can always do something like
> > ```
> > []<typename = std::enable_if<sizeof...(Types) == 0>::value> {}
> > ```
> > and that should have the same functionality.
> >
> > Do you or Erich plan to open a CWG issue about this?
> >
> FYI I agree with @royjacobson on the current wording interpretation.
> I don't think sfinae on empty pack offer much benefit - versus the complexity
> (and the cost of not being able to diagnose non-empty packs until
> instanciations) so I don't think there is a core issue here either. But until
> such core issue emerges and is resolved, I don't think we need to change what
> is done here.
> Do you or Erich plan to open a CWG issue about this?
I was contemplating it, but after thinking on it more overnight, I think we
should accept this patch as-is. I believe it would be safe and plausible to
lift this restriction later if CWG or EWG decides to move in this space, but
the amount of utility gained by supporting it now is pretty minimal.
Thanks for the discussion around this!
================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:698
Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
+ Builder.defineMacro("__cpp_static_call_operator", "202207L");
}
----------------
Because we're exposing the feature as a language extension in older language
modes, I think we probably want to define the macro in older language modes as
well, right?
================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1174
+ auto ConsumeLocation = [&P, &DeclEndLoc](SourceLocation &SpecifierLoc,
+ int diag_index) {
+ if (SpecifierLoc.isValid()) {
----------------
coding style nit.
================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265
+ if (Intro.hasLambdaCapture())
+ P.Diag(StaticLoc, diag::err_static_lambda_captures);
+}
----------------
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?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15391-15393
+ if (Invoker != CallOp) {
+ L->CompletedImplicitDefinition(Invoker);
+ }
----------------
================
Comment at: clang/test/Parser/cxx2b-lambdas-ext-warns.cpp:1-3
+// RUN: %clang_cc1 -std=c++20 %s -verify=expected,cxx20
+// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b
+// RUN: %clang_cc1 -std=c++2b -Wpre-c++2b-compat %s -verify=expected,precxx2b
----------------
You weren't using the `expected` prefix in the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133659/new/
https://reviews.llvm.org/D133659
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
