sammccall marked 4 inline comments as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/pseudo/unittests/BracketTest.cpp:103
+TEST(Bracket, SimplePair) {
+ verifyBrackets("^{ ^[ ^( ^) ^( ^) ^] ^}");
+ verifyBrackets(") ^{ ^[ ^] ^} (");
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > For this simple case, just using `^` is fine.
> > >
> > > I think in the near future, we would need more to verify that e.g. which
> > > two brackets are paired. And since we're defining some some common
> > > functions used by the tests, I wonder what's the plan here (using
> > > something like `{ $1^( $1^)`?)
> > > I think in the near future, we would need more to verify that e.g. which
> > > two brackets are paired.
> >
> > I thought about the simplest way to specify these tests, and I think the
> > `^` is sufficient.
> >
> > The combination of:
> > a) the set of brackets that are paired
> > b) for each bracket, knowing whether it is paired forwards or backwards
> > c) certainty that brackets are well-nested
> > fully determines the bracket pairing.
> >
> > The test specifies a) and we check it, and b) and c) can be verified with
> > no extra information.
> >
> > Does this make sense? It's a little implicit, but makes the testcases much
> > more readable than having to specifiy `$1^` etc.
> > If so, I'll explain this in a comment.
> Sure, using `^` is sufficient, and agree that make test cases more readable.
>
> I was a bit worried about the error-repair cases. For these cases, I thought
> b) (e.g. we know this `{` should be paired with the last `}`, not the next
> one) is more important, it is probably better explicitly specify in the test,
> rather than doing it silently. I think we can use trailing comments to
> explain that while keeping `^`.
>
> ```
> class A ^{ // A
> void foo() {
> ^} // A
> ```
Added a comment explaining how it's sufficient, and that we should use comments
if needed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125911/new/
https://reviews.llvm.org/D125911
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
