aaron.ballman added a reviewer: hubert.reinterpretcast.
aaron.ballman added a subscriber: hubert.reinterpretcast.
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">;
----------------
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.


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

Reply via email to