[PATCH] D123909: [Clang] Use of decltype(capture) in parameter-declaration-clause

Corentin Jabot via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 18 06:01:53 PDT 2022


cor3ntin added inline comments.


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:700
+
+  if (Tok.is(tok::identifier) && NextToken().is(tok::r_paren)) {
+    SourceLocation TemplateKWLoc;
----------------
aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > cor3ntin wrote:
> > > > aaron.ballman wrote:
> > > > > This feels surprisingly restrictive to me, but it's sufficient for getting libstdc++ working again.
> > > > I've can't think of another kind of unqualified-id that could refer to a capturable entity here.
> > > > Do you know what would be missing?
> > > > Also, without being very restrictive, I'm not sure how this is implementable
> > > What I find restrictive is that it requires an unqualified-id *only* to kick in as being maybe mutable agnostic. e.g., `decltype(id)` and `decltype(+id)` (where the unary + may be an overloaded operator that depends on the mutability of the `this` object).
> > > 
> > > (I'm not seeing the principle that guides this restriction proposed in the core issue yet, but those can be hashed out later.)
> > I do not think there is a general answer to "given these captures and this arbitrarily complex expression", would the mutability of the lambda affect the declaration of the parameters.
> > In this case we know it doesn't and that happen  to fix existing code, which make the solution tractable.
> > If core was keen to add any more complexity here, I think they should consider mandating a look-ahead of the `mutable` keyword even if that sounds rather complex. I don't think it will come to that, the proposed resolution will be good enough to reduce the blast radius of the paper, and I don't see a strong motivation to add even more complexity here.
> Eh, I'm less convinced. The changes in P2036R3 were a band-aid to fix mistakes. The core issue "fixing" it is another band-aid over the top of P2036R3. My experience is that band-aids over band-aids rarely leads to a good feature that users can make sense of. The fact that I can't make heads or tails of what the actual language design rule is here also means this is painful for us supporting our own extensions (like `alignof` on an object instead of a type, `__typeof__` instead of `decltype`, etc).
> 
> Personally, I'd prefer the core issue was resolved in a more clean fashion than carving out a few exceptions under the hope that it's "good enough". However, that's something for us to fight about in Core when this comes up. For the moment, I'm happy enough with this because it gets libstdc++ unstuck.
> The changes in P2036R3 were a band-aid to fix mistakes. The core issue "fixing" it is another band-aid over the top of P2036R3. 

Agreed, but, the question is how many people will run into scenario not covered by the band-aid in existing code?
And this is why I'm not a fan of generalizing to `sizeof`/`noexcept` because it' sounds arbitrary and unmotivated, and if we have more than one bandaid we should consider a general solution.

Basically either that or `mutable` look-ahead, i don't see an in-between solution as being viable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123909



More information about the cfe-commits mailing list