[PATCH] D105127: Implement P1401R5

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D105127#2850975 <https://reviews.llvm.org/D105127#2850975>, @mizvekov wrote:

> So I read the paper, downloaded this patch, played around with it a little bit, tried some different tests, like expressions with dependent types,
> classes with regular/explicit user-defined conversions, function names and some other examples that are mentioned in the paper.
> It works fine on these.
>
> However, I confirm that the failures in `CXX/except/except.spec/p1.cpp` detected by the buildbots are real.
> It fails for me with this DR, works on parent revision.

Yep, I need to look at that. I've ran the entire test suite locally without issue initially but maybe I broke something!



================
Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:43
 namespace ccce {
+struct S {
+} s;
----------------
mizvekov wrote:
> cor3ntin wrote:
> > mizvekov wrote:
> > > This is not consistently indented.
> > Unfortunately, it was put there by clang-format, should I move it manually?
> In general you should respect the formatting tips you get from the non-test code, as these will fail the pre-merge checks.
> But in the test code, our buildbot produces the clang-format patch but this is not really enforced when merging.
> 
> I think what you are doing here, keeping the existing style, is reasonable so you should disregard this particular tip from the format patch.
> The other option would be to format everything in a pre-work NFC commit. This should be fine for pure semantic tests like these, but you have to be careful with parser tests.
This is unfortunate, but I'll fix it, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127