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

[Dinesh Dwivedi]

================
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;
----------------
Daniel Jasper wrote:
> Please add:
> 
>   // FIXME: Pull cast detection into its own function.
added comment

================
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;
----------------
Daniel Jasper wrote:
> Please use {} for the if, if there are {} for the else.
updated

================
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)) &&
----------------
Daniel Jasper wrote:
> 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.
updated. I am setting IsCast to true and then if it is not identified as cast, it is getting resetted. 
"(Prev && .." was there to keep IsCast becomming true as it will not get reset later.

I am not sure if I have to check for Prev or can assume that it will always be non-null here.

================
Comment at: lib/Format/TokenAnnotator.cpp:806
@@ +805,3 @@
+
+            // only tokens allowed inside () is const and some identifier
+            // isSimpleTypeSpecifier case is already handled above
----------------
Daniel Jasper wrote:
> What about:
> - * (pointers)
> - [] (arrays)
> - <> (templates)
> 
> ?
I did not get it. Do you mean something like *(my_int_ptr)+2 etc. I will look for other
casting example and try to handle them in patch. 

If you have anything specific in mind, which should be handled, please suggest.

================
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)) {
----------------
Daniel Jasper wrote:
> 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.
updated

http://reviews.llvm.org/D3576