[clang] [Sema] Fix computations of "unexpanded packs" in substituted lambdas (PR #99882)

via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 23 07:08:52 PDT 2024


cor3ntin wrote:

On Tue, Jul 23, 2024, 16:04 Ilya Biryukov ***@***.***> wrote:

> My main concern is this line which seems very artificial
> https://github.com/llvm/llvm-project/pull/99882/files#diff-10901a3bb08d903a57820f09c2eb5f40cb4cd47c54ca3cad472106814c4bf15dR359
>
> Yeah, I can see your point. My north-start vision for that issue is that
> is should all be structural and even
> LambdaScopeInfo::ContainsUnexpandedPack should go away in favor of
> storing ContainsUnexpandedPack bit inside Stmt.
>

This would be excellent, but I think we would still need that just because
building a lambda is an horrible state machine.

> We could actually get away without having this line, but it's there
> precisely to make sure that parsing and transform get the same outputs. The
> bugs we are trying to fix are an evidence of the fact that parsing is
> tested more rigorously and sharing the same inputs with transform gives
> more confidence that bugs won't show up.
>
> If we both agree that you would rather centralize everything during
> transform, I think it's fine but during parsing we already have the
> information available so we should try to use it - unless I am missing
> something?
>
> Technically, we *also* have this information during transform in the same
> way, but, unfortunately, it requires a duplicate code path. I think my
> objections to a flag in LambdaScopeInfo would be much weaker if we
> rewrote the code in a way that was guaranteed to be shared across parsing
> and transform (putting it in computeDependence is just one way to do it).
>
That would also work for me, I think.

I'm looking forward to what @zyn0217 <https://github.com/zyn0217> comes up
> with, I'm sure we'll be in a better state when his patch lands.
>
> I will close this PR in the meantime to put more focus on #86265
> <https://github.com/llvm/llvm-project/pull/86265>.
>
>> Reply to this email directly, view it on GitHub
> <https://github.com/llvm/llvm-project/pull/99882#issuecomment-2245351993>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKX76YPCO6D6VGJ7FSNE5LZNZPHVAVCNFSM6AAAAABLIOJLFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBVGM2TCOJZGM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>


https://github.com/llvm/llvm-project/pull/99882


More information about the cfe-commits mailing list