hokein added inline comments.

================
Comment at: clang-tools-extra/pseudo/unittests/BracketTest.cpp:103
+TEST(Bracket, SimplePair) {
+  verifyBrackets("^{ ^[ ^( ^)  ^( ^) ^] ^}");
+  verifyBrackets(") ^{ ^[ ^] ^} (");
----------------
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
```


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

Reply via email to