Quuxplusone added inline comments.

================
Comment at: clang/unittests/Format/FormatTest.cpp:7652
 
+  verifyFormat("test < a - 1 >> 1;");
   verifyFormat("test >> a >> b;");
----------------
Quuxplusone wrote:
> IMO you should use `"test < a | b >> c;"` as your test case here, to reassure 
> the reader that it doesn't depend on the fact that `... 1;` is visibly not a 
> variable declaration.
> Personally I'd also like to see `"test<test<a | b>> c;"` tested on the very 
> next line, to show off the intended difference between the two. (Assuming 
> that I understand the intent of this patch correctly.)
> (I also switched to a bitwise operator just for the heck of it; that makes 
> this expression just a //very tiny bit// less implausible — but still highly 
> implausible, to the point where I question why we're special-casing it.)
Btw, a much-bigger-scope way to fix this would be to teach clang-format about 
"input encoding" versus "output encoding." The only time clang-format should 
//ever// be inserting space in the middle of `>>` is if it's translating 
C++11-encoded input into C++03-encoded output. If the input is known to already 
be C++03-encoded, then breaking up an `>>` token into a pair of `> >` tokens is 
//guaranteed// to introduce a bug.
Right now, my impression is that clang-format has a concept of "output 
encoding" (i.e. "language mode") but has no way of knowing the "input encoding."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to