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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
