[PATCH] Some heuristics to identify c style casting (PR18937)

[Daniel Jasper]

Thank you for working on this!

================
Comment at: lib/Format/TokenAnnotator.cpp:757
@@ -756,3 +756,3 @@
           Current.Type = TT_BlockComment;
       } else if (Current.is(tok::r_paren)) {
         FormatToken *LeftOfParens = NULL;
----------------
Please add:

  // FIXME: Pull cast detection into its own function.

================
Comment at: lib/Format/TokenAnnotator.cpp:790
@@ +789,3 @@
+                 LeftOfParens->Type != TT_TemplateCloser && Current.Next) {
+          if (Current.Next->isOneOf(tok::identifier, tok::numeric_constant))
+            IsCast = true;
----------------
Please use {} for the if, if there are {} for the else.

================
Comment at: lib/Format/TokenAnnotator.cpp:792
@@ +791,3 @@
+            IsCast = true;
+          else { // Some heuristics to identify c style casting
+            FormatToken *Prev = Current.Previous;
----------------
I'd prefer to put this comment on its own line.

And please make all comments here full sentences.

================
Comment at: lib/Format/TokenAnnotator.cpp:799
@@ +798,3 @@
+            // it is likely a cast
+            if (Prev && (Current.Next->isUnaryOperator() ||
+                         Current.Next->isOneOf(tok::amp, tok::star)) &&
----------------
I think we need to check that Current.Next is not null. Otherwise, this might crash if a line ends in a right parenthesis.

On the other hand, I don't think "Prev &&" changes the behavior.

How about writing this as:

  if (Current.Next && Current.Next->Next) {
    bool NextIsUnary = Current.Next->isUnaryOperator() ||
                       Current.Next->isOneOf(tok::amp, tok::star);
    IsCast = NextIsUnary && Current.Next->Next->isOneOf(
                                tok::identifier, tok::numeric_constant);
  }

I think then it is almost self-explanatory and you don't need the comment.

================
Comment at: lib/Format/TokenAnnotator.cpp:806
@@ +805,3 @@
+
+            // only tokens allowed inside () is const and some identifier
+            // isSimpleTypeSpecifier case is already handled above
----------------
What about:
- * (pointers)
- [] (arrays)
- <> (templates)

?

================
Comment at: lib/Format/TokenAnnotator.cpp:808
@@ +807,3 @@
+            // isSimpleTypeSpecifier case is already handled above
+            while (Prev != Current.MatchingParen) {
+              if (!Prev || !Prev->isOneOf(tok::kw_const, tok::identifier)) {
----------------
Further up in this function, we are using a for-loop for this, i.e.

  for ( ; Prev != Current.MatchingParen; Prev = Prev->Previous) { ..

I think we should be consistent.

http://reviews.llvm.org/D3576