<div dir="ltr">I think this makes more sense for now. If we stumble upon bugs requiring the fix for the other operators, we still have your other patch.<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 3, 2014 at 10:58 PM, Johannes Kapfhammer <span dir="ltr"><<a href="mailto:jkapfham@ethz.ch" target="_blank" class="cremed">jkapfham@ethz.ch</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Sun, Mar 30, 2014 at 07:45:51PM +0200, Daniel Jasper wrote:<br>
> All in all, I think a much easier solution for the mentioned bug would be<br>
> to treat "<<" similar to how we treat assignments, i.e. if we find a <<, we<br>
> assume the RHS to be an expression (that's the very first "if" in<br>
> determineTokenType()).<br>
><br>
</div>Not sure if that's the right place. The "if" branch you are talking<br>
about will set all preceding stars (on the same level) to<br>
TT_PointerOrReference:<br>
<br>
Before: cout << a *a << b *b << c *c;<br>
After: cout << a *a << b *b << c * b;<br>
My Fix: cout << a * a << b * b << c * c;<br>
<br>
So we need the special care for operator<< at least in a new<br>
"else if". I've added a "light" version of this patch below, note<br>
that it only handles operator<< and doesn't work with other operators.<br>
Doing it the way I did in the last mail has the additional benefit<br>
that it works for all operators and not just "<<" (in particular<br>
operator+ is hard to get otherwise). But we probably won't need this.<br>
<br>
--<br>
<br>
>From 16d81e7e3a0dedd5d51f963a134b26eb9c63b777 Mon Sep 17 00:00:00 2001<br>
<div class="">From: Kapfhammer <<a href="mailto:kapf@student.ethz.ch" class="cremed">kapf@student.ethz.ch</a>><br>
Date: Sun, 30 Mar 2014 12:04:17 +0200<br>
</div>Subject: [PATCH] Use binary operators as an indicator of for an expression and<br>
<div class=""> extend the test suite.<br>
<br>
This solves the issue where the star was too often interpreted as a<br>
pointer, e.g "cout<<a*b;" was formatted to "cout << a *b;" instead of<br>
"cout << a * b;". By marking statements more often an expression, the<br>
function determineStarAmpUsage() is more reliable.<br>
<br>
The test suite was extended.<br>
---<br>
</div> lib/Format/TokenAnnotator.cpp | 5 +++++<br>
unittests/Format/FormatTest.cpp | 15 +++++++++++++++<br>
2 files changed, 20 insertions(+)<br>
<br>
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp<br>
index 0034235..7fbbdae 100644<br>
--- a/lib/Format/TokenAnnotator.cpp<br>
+++ b/lib/Format/TokenAnnotator.cpp<br>
@@ -681,6 +681,11 @@ private:<br>
Previous->Type = TT_PointerOrReference;<br>
}<br>
}<br>
+ } else if (Current.is(tok::lessless) &&<br>
+ !Line.First->isOneOf(tok::kw_template, tok::kw_using) &&<br></blockquote><div><br></div><div>How does this change the behavior in any way?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ (!Current.Previous ||<br>
+ Current.Previous->isNot(tok::kw_operator))) {<br></blockquote><div><br></div><div>I wonder whether we can just use:</div><div><br></div><div>} else if (Current.is(tok::lessless) && !Line.MustBeDeclaration) { ..</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ Contexts.back().IsExpression = true;<br>
} else if (Current.isOneOf(tok::kw_return, tok::kw_throw)) {<br>
Contexts.back().IsExpression = true;<br>
} else if (Current.is(tok::l_paren) && !Line.MustBeDeclaration &&<br>
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp<br>
index 5395fd9..7478a23 100644<br>
--- a/unittests/Format/FormatTest.cpp<br>
+++ b/unittests/Format/FormatTest.cpp<br>
@@ -4625,6 +4625,21 @@ TEST_F(FormatTest, UnderstandsRvalueReferences) {<br>
<div class=""> verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");<br>
}<br>
<br>
+TEST_F(FormatTest, FormatsMultiplicationOperator) {<br>
+ verifyFormat("operator*(type *a)");<br>
+ verifyFormat("operator<<(type *a)");<br>
+ verifyFormat("{ cout << (a * b); }");<br>
+ verifyFormat("{ cout << a * b; }");<br>
</div>+ verifyFormat("{ cout << a * b << c * d; }");<br>
<div class="HOEnZb"><div class="h5">+ verifyFormat("type (*f)(type *a)");<br>
+ verifyFormat("type (&f)(type *a)");<br>
+ verifyFormat("void f(type *a)");<br>
+ verifyFormat("using f = void (*)(a *b)");<br>
+ verifyFormat("template <typename T = void (*)(a *b)");<br>
+ verifyFormat("using f = void (c::*)(a *b)");<br>
+ verifyFormat("template <typename T = void (c::*)(a *b)>");<br></div></div></blockquote><div><br></div><div>There are many tests here that don't contain a "<<". What are they testing? </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
+}<span style="color:rgb(34,34,34)"> </span></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
+<br>
TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) {<br>
verifyFormat("void f() {\n"<br>
" x[aaaaaaaaa -\n"<br>
--<br>
1.9.1<br>
<br>
</div></div></blockquote></div><br></div></div>