[libcxx-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

Thu Jul 14 21:15:21 PDT 2022

JDevlieghere added a subscriber: teemperor.
JDevlieghere added a comment.

In D112374#3653702 <https://reviews.llvm.org/D112374#3653702>, @mizvekov wrote:

> @JDevlieghere I spent a lot of time trying to get this test running on my machine to no avail. I think lldb build and test setup is quite convoluted, fragile and antiquated. It uses many deprecated CMake features, It fails to properly link to system libraries it needs like librt. And I just kept patching these problems up and more kept coming. At some point I just gave up.

I'm sorry to hear you're having trouble building LLDB. The LLDB website has quite an elaborate guide with instructions in how to build LLDB: https://lldb.llvm.org/resources/build.html, including specific instructions on Windows. Windows is not my main platform, but I've successfully built LLDB there in the past following those instructions. I'm not sure what you feel is "convoluted, fragile and antiquated" about our build and test setup, as it's fairly similar to the rest of LLVM. I'd be happy to hear your suggestions on how we could improve things.

> And even this test itself requires the whole libcxx it seems, which is another difficult thing to get building outside of CI.
>
> I think pre-commit CI is essential here.
>
> So I would be happy to help sort this lldb bug, but you have to help me help you.

I'm happy to help out. I personally don't know if we should go with (1) or (2), both sound reasonable in their own way. I'd like @teemperor, who's the author of the feature and the affected tests, to weigh in.

> I think reverting it with no prior notification was unreasonable, and landing this back as it was, no changes, is reasonable.
>
> Do you agree? Please let me know soon otherwise.

I don't. I think reverting your change was well within the guidelines outlined by LLVM's patch reversion policy: https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy

Additionally, I think you could've given me a little bit more time to catch up on the discussion here. The code review policy and practices (https://llvm.org/docs/CodeReview.html#code-reviews-speed-and-reciprocity) recommend pinging every few days to once per week depending on how urgent the patch is.

By relanding, you broke the bots again (https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45354/#showFailuresLink) and I'm forced to revert this change a second time. Please refrain from landing this again until we've settled on a way forward.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374