[clang-format] Recognize Java logical shift assignment operator

[bradfier]

Summary

At present, clang-format mangles Java containing logical right shift operators ('>>>=' or '>>>'), splitting them in two, resulting in invalid code:

 public class Minimal {
   public void func(String args) {
     int i = 42;
-    i >>>= 1;
+    i >> >= 1;
     return i;
   }
 }

This adds both forms of logical right shift to the FormatTokenLexer, so clang-format won't attempt to split them and insert bogus whitespace.

[bradfier]

Add more diff context, add klimek as reviewer.

[arphaman]

Are the <<< and >>> operators handled correctly?

[arphaman]

include/clang/Basic/LangOptions.def

95 ↗

(On Diff #94334)

I don't think we should have a Java lang option, since Clang is not actually a Java frontend. Maybe another option like LogicalShiftOperators will work better?

[bradfier]

In D31652#719990, @arphaman wrote:

Are the <<< and >>> operators handled correctly?

Yes, as a side-effect of CUDA kernel-call syntax which uses >>> and <<<, those tokens are recognised as punctuators.

[bradfier]

include/clang/Basic/LangOptions.def

95 ↗

(On Diff #94334)

That seems reasonable. C and C++ have >>> or <<< operators as part of CUDA extensions though, so maybe the full LogicalShiftAssignOperators is more precise?

[thakis]

include/clang/Basic/TokenKinds.def

230 ↗

(On Diff #94334)

I think this is the wrong approach to go about this. clang is a C compiler, and its tokenizer shouldn't have to know about Java. Instead, this should be handled in the formatter. See tryMergePreviousTokens() in lib/Format/FormatTokenLexer.cpp for how we do this for e.g. => or >>>= etc for JavaScript – just do the same for Java.

[bradfier]

Switch to a more appropriate (and much simpler) method of identifying these Java-specific operators.

Also removed any references to fictitious "logical left shifts", I think I made those up while tired...

[bradfier]

Thanks for that @thakis, that's a much better solution than the first attempt!

[bradfier]

Friendly ping.

[thakis]

Looks good. Do you have commit access?

[bradfier]

In D31652#723664, @thakis wrote:

Looks good. Do you have commit access?

Thanks! And no I don't have commit access.

[thakis]

Landed in r299952: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20170410/189882.html

lib/Format/FormatTokenLexer.cpp

if (tryMergeTokens(JSIdentity, TT_BinaryOperator))
       return;
     if (tryMergeTokens(JSNotIdentity, TT_BinaryOperator))
       return;
     if (tryMergeTokens(JSShiftEqual, TT_BinaryOperator))
       return;
     if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
       return;
   }
+
+  if (Style.Language == FormatStyle::LK_Java) {
+    static const tok::TokenKind JavaRightLogicalShift[] = {tok::greater,
+                                                           tok::greater,
+                                                           tok::greater};
+    static const tok::TokenKind JavaRightLogicalShiftAssign[] = {tok::greater,
+                                                                 tok::greater,
+                                                                 tok::greaterequal};
+    if (tryMergeTokens(JavaRightLogicalShift, TT_BinaryOperator))
+      return;
+    if (tryMergeTokens(JavaRightLogicalShiftAssign, TT_BinaryOperator))
+      return;
+  }
 }
 
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)
     return false;
 
   bool FourthTokenIsLess = false;

unittests/Format/FormatTestJava.cpp

EXPECT_EQ("/*\n"
             "void f() {}",
             format("  /*\n"
                    "   * Really multi-line\n"
                    "   * comment.\n"
                    "   */\n"
                    "  void f() {}"));
 }
 
+TEST_F(FormatTestJava, RetainsLogicalShifts) {
+    verifyFormat("void f() {\n"
+                 "  int a = 1;\n"
+                 "  a >>>= 1;\n"
+                 "}");
+    verifyFormat("void f() {\n"
+                 "  int a = 1;\n"
+                 "  a = a >>> 1;\n"
+                 "}");
+}
+
+
 } // end namespace tooling
 } // end namespace clang