[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 20 11:15:20 PDT 2022


MaskRay added a comment.

In D119136#3462579 <https://reviews.llvm.org/D119136#3462579>, @aaron.ballman wrote:

> In D119136#3462570 <https://reviews.llvm.org/D119136#3462570>, @MaskRay wrote:
>
>> Sorry but I've reverted this patch and all its fixups in c79e6007edef4b0044be93c4ffff64dc806ac687 <https://reviews.llvm.org/rGc79e6007edef4b0044be93c4ffff64dc806ac687> and 0f5dbfd29ae0df215a01aff80d29255bb799fed0 <https://reviews.llvm.org/rG0f5dbfd29ae0df215a01aff80d29255bb799fed0> .
>> See https://reviews.llvm.org/D123909#3461716 for another case not considered.
>
> Please double check with the patch author and reviewers before unilaterally reverting multiple commits with no notice and no failing build bots. I do not see a valid justification for these reverts -- the concerns have been true positives so far (or have generated core issues that WG21 is still discussing), and @cor3ntin has been highly responsive with addressing the fallout. This is a significant amount of churn that I don't think should have happened.

Reply to both this message and https://reviews.llvm.org/D123909#3462560:

I am sorry but this thread has gained reports from multiple independent parties about breakage, and I just thought the followups did not fix all issues.
I confess I do not really understand the subtle areas in the language.
My decision to revert was mostly driven by fixups' descriptions like "but did not properly handled dependant context,"

- which made me believe there were indeed bugs in the implementation and new corner cases just emerged.

I'd agree that there is invalid code which is not correctly rejected, but the fixups mixed with bugfixes made the whole picture unclear.

I apology if I did make wrong reverts.
The reverts have been made.
I hope that starting from a clean state is not too bad. I am grateful to @cor3ntin who is highly responsive with addressing the fallout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136



More information about the cfe-commits mailing list