I think those error messages are a bit confusing.<div><br></div><div>The wording seems a little too "absolute". To me at least, "<span style="background-color:rgb(255,255,255);font-family:arial,sans-serif;font-size:12px">initializer list cannot be used on the right hand side of operator '+'" at first reading seems to say that it is illegal, in general, to have an initializer list on the RHS of a +, where it seems like it would be clearer to say "illegal use of an initializer list in the right operand to '+'". Urgh, I'm not fully happy about that one either.</span><div>
<span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)">Maybe it would be better to just say "illegal use of an initializer list", and have the little ~~~ underneath the offending initializer list. In the end, saying "on the right hand side of operator '+'" is just location information which the ~~~ communicates better than words.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)">Sorry if that sounds nitpicky, but I legitimately understood it as that at first.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:12px;background-color:rgb(255,255,255)">--Sean Silva </span><br>
<br><div class="gmail_quote">On Wed, Feb 29, 2012 at 10:24 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Wed, Feb 29, 2012 at 6:59 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Wed Feb 29 20:59:17 2012<br>
> New Revision: 151794<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=151794&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=151794&view=rev</a><br>
> Log:<br>
> Reject 'a = {0} = {0}' rather than parsing it as '(a = {0}) = {0}'. Also<br>
> improve the diagnostics for some attempts to use initializer lists in<br>
> expressions.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
> cfe/trunk/lib/Parse/ParseExpr.cpp<br>
> cfe/trunk/test/CXX/expr/expr.ass/p9-cxx11.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=151794&r1=151793&r2=151794&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=151794&r1=151793&r2=151794&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Feb 29 20:59:17 2012<br>
> @@ -229,6 +229,8 @@<br>
> def warn_cxx98_compat_generalized_initializer_lists : Warning<<br>
> "generalized initializer lists are incompatible with C++98">,<br>
> InGroup<CXX98Compat>, DefaultIgnore;<br>
> +def err_init_list_bin_op : Error<"initializer list cannot be used on the "<br>
> + "%select{left|right}0 hand side of operator '%1'">;<br>
> def warn_cxx98_compat_trailing_return_type : Warning<<br>
> "trailing return types are incompatible with C++98">,<br>
> InGroup<CXX98Compat>, DefaultIgnore;<br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseExpr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=151794&r1=151793&r2=151794&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=151794&r1=151793&r2=151794&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseExpr.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Feb 29 20:59:17 2012<br>
> @@ -282,6 +282,12 @@<br>
> Token OpToken = Tok;<br>
> ConsumeToken();<br>
><br>
> + if (!LHS.isInvalid() && isa<InitListExpr>(LHS.get())) {<br>
> + Diag(OpToken, diag::err_init_list_bin_op)<br>
> + << /*LHS*/0 << PP.getSpelling(OpToken) << LHS.get()->getSourceRange();<br>
> + LHS = ExprError();<br>
> + }<br>
> +<br>
> // Special case handling for the ternary operator.<br>
> ExprResult TernaryMiddle(true);<br>
> if (NextTokPrec == prec::Conditional) {<br>
> @@ -353,22 +359,16 @@<br>
> // Therefore we need some special-casing here.<br>
> // Also note that the third operand of the conditional operator is<br>
> // an assignment-expression in C++, and in C++11, we can have a<br>
> - // braced-init-list on the RHS of an assignment.<br>
> + // braced-init-list on the RHS of an assignment. For better diagnostics,<br>
> + // parse as if we were allowed braced-init-lists everywhere, and check that<br>
> + // they only appear on the RHS of assignments later.<br>
> ExprResult RHS;<br>
> - if (getLang().CPlusPlus0x && MinPrec == prec::Assignment &&<br>
> - Tok.is(tok::l_brace)) {<br>
> - Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);<br>
> + if (getLang().CPlusPlus0x && Tok.is(tok::l_brace))<br>
> RHS = ParseBraceInitializer();<br>
> - if (LHS.isInvalid() || RHS.isInvalid())<br>
> - return ExprError();<br>
> - // A braced-init-list can never be followed by more operators.<br>
> - return Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),<br>
> - OpToken.getKind(), LHS.take(), RHS.take());<br>
> - } else if (getLang().CPlusPlus && NextTokPrec <= prec::Conditional) {<br>
> + else if (getLang().CPlusPlus && NextTokPrec <= prec::Conditional)<br>
> RHS = ParseAssignmentExpression();<br>
> - } else {<br>
> + else<br>
> RHS = ParseCastExpression(false);<br>
> - }<br>
><br>
> if (RHS.isInvalid())<br>
> LHS = ExprError();<br>
> @@ -387,6 +387,11 @@<br>
> // more tightly with RHS than we do, evaluate it completely first.<br>
> if (ThisPrec < NextTokPrec ||<br>
> (ThisPrec == NextTokPrec && isRightAssoc)) {<br>
> + if (!LHS.isInvalid() && isa<InitListExpr>(LHS.get())) {<br>
> + Diag(OpToken, diag::err_init_list_bin_op)<br>
> + << /*LHS*/0 << PP.getSpelling(OpToken) << LHS.get()->getSourceRange();<br>
> + LHS = ExprError();<br>
> + }<br>
<br>
</div></div>We normally try to avoid the Parser using AST methods directly...<br>
granted, the distinction between Parser and Sema has started to blur a<br>
bit, but I think it's still useful to try to maintain.<br>
<span class="HOEnZb"><font color="#888888"><br>
-Eli<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>