Fix issue with (void)F;

[dinesh.d]

Summary

This will fix issues raise by Tobias
(http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140505/104690.html)

[djasper]

Please add a test case.

[dinesh.d]

added test cases

[djasper]

unittests/Format/FormatTest.cpp

4743

This test case is weird, is it taken from real code? I think clang-format might do very strange things to it (as it commonly splits unwrapped lines on the semicolon).

Also, it is missing a trailing semicolon and don't you need braces if there are multiple statements in a DEBUG invocation?

What are you trying to test?

4744

I prefer implicitly concatenating string literals after "\n". Makes the tests easier to read for me. I.e., turn this into:

verifyFormat("void foo() {\n"
             "  assert(t && \"assert msg\");\n"
             "  (void)F;\n"
             "}");

Also, why the function definition and assert?

[dinesh.d]

update test cases

unittests/Format/FormatTest.cpp

4743

Test string is not quite correct here. Sorry. Update test case.

Though I have looked for (void) in llvm code and tried to mimic
pattern used in test cases.

4744

updated.

I have looked for (void) in llvm code and tried to mimic
pattern used in test cases.

[djasper]

unittests/Format/FormatTest.cpp

4757–4765

IMO, these don't make sense. They are split into different unwrapped lines and your change can't possibly affect the three lines other than "(void)F;". This file (mostly) contains unit tests, i.e. you should try to test the smallest example that reproduces a specific example.

In this case, I think this should simply be:

verifyIndependentOfContext("(void)F;");

[dinesh.d]

unittests/Format/FormatTest.cpp

4757–4765

current clang-format does not handles just '(void)F;' (void) is treated as
cast only if this code is inside a block. verifyIndependentOfContext()
test for both the cases, first as stand-alone string and then, after wrapping
within a function. So above test case will fail for first.

Smallest test case may be

verifyFormat("{ (void)F; }");

Moreover, this is not bug, Tobias has raise issue about change in formatting
for some pattern. If current behaviour is what we want we do not have to do
anything else with this patch, we can restore previous behaviour.

before r207961:

if (test) {
  (void)WasFastQuery;
  assert(WasFastQuery && "More work to do after problem solved?");

  assert(Found && "Invalid reverse map!");
  (void)Found;

  (void)SimplifyICmpOperands(Cond, LHS, RHS);
}

After r207961:

if (test) {
  (void) WasFastQuery;
  assert(WasFastQuery && "More work to do after problem solved?");

  assert(Found && "Invalid reverse map!");
  (void) Found;

  (void) SimplifyICmpOperands(Cond, LHS, RHS);
}

[dinesh.d]

reduced test case

[djasper]

Looks good.

[dinesh.d]

committed as r208080.

Thanks for review and support.

lib/Format/TokenAnnotator.cpp

         if (Previous->Type == TT_BinaryOperator &&
             Previous->isOneOf(tok::star, tok::amp)) {
           Previous->Type = TT_PointerOrReference;
         }
       }
     } else if (Current.isOneOf(tok::kw_return, tok::kw_throw)) {
       Contexts.back().IsExpression = true;
     } else if (Current.is(tok::l_paren) && !Line.MustBeDeclaration &&
-               !Line.InPPDirective && Current.Previous &&
-               Current.Previous->isNot(tok::kw_decltype)) {
+               !Line.InPPDirective &&
+               (!Current.Previous ||
+                Current.Previous->isNot(tok::kw_decltype))) {
       bool ParametersOfFunctionType =
           Current.Previous && Current.Previous->is(tok::r_paren) &&
           Current.Previous->MatchingParen &&
           Current.Previous->MatchingParen->Type == TT_FunctionTypeLParen;
       bool IsForOrCatch = Current.Previous &&
                           Current.Previous->isOneOf(tok::kw_for, tok::kw_catch);
       Contexts.back().IsExpression = !ParametersOfFunctionType && !IsForOrCatch;
     } else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {

unittests/Format/FormatTest.cpp

   verifyFormat("int a = (int)++a;");
   verifyFormat("int a = (int)sizeof(int);");
   verifyFormat("int a = (int)+2;");
   verifyFormat("my_int a = (my_int)2.0f;");
   verifyFormat("my_int a = (my_int)sizeof(int);");
   verifyFormat("return (my_int)aaa;");
   verifyFormat("#define x ((int)-1)");
   verifyFormat("#define p(q) ((int *)&q)");
 
   verifyFormat("void f() { my_int a = (my_int)*b; }");
   verifyFormat("void f() { return P ? (my_int)*P : (my_int)0; }");
   verifyFormat("my_int a = (my_int)~0;");
   verifyFormat("my_int a = (my_int)++a;");
   verifyFormat("my_int a = (my_int)+2;");
   verifyFormat("my_int a = (my_int)1;");
   verifyFormat("my_int a = (my_int *)1;");
   verifyFormat("my_int a = (const my_int)-1;");
   verifyFormat("my_int a = (const my_int *)-1;");
 
   // FIXME: single value wrapped with paren will be treated as cast.
   verifyFormat("void f(int i = (kValue)*kMask) {}");
 
+  verifyFormat("void foo() {\n"
+               "  DEBUG(dbgs() << \"test msg\";);\n"
+               "  (void)F;\n"
+               "}");
+  verifyFormat("void foo() {\n"
+               "  assert(test && \"assert msg\");\n"
+               "  (void)F;\n"
+               "}");
+
   // Don't break after a cast's
   verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
                "    (aaaaaaaaaaaaaaaaaaaaaaaaaa *)(aaaaaaaaaaaaaaaaaaaaaa +\n"
                "                                   bbbbbbbbbbbbbbbbbbbbbb);");
 
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");