[cfe-commits] r151794 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseExpr.cpp test/CXX/expr/expr.ass/p9-cxx11.cpp

[Sean Silva]

I think those error messages are a bit confusing.

The wording seems a little too "absolute". To me at least, "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.

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.

Sorry if that sounds nitpicky, but I legitimately understood it as that at
first.

--Sean Silva

On Wed, Feb 29, 2012 at 10:24 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> On Wed, Feb 29, 2012 at 6:59 PM, Richard Smith
> <richard-llvm at metafoo.co.uk> wrote:
> > Author: rsmith
> > Date: Wed Feb 29 20:59:17 2012
> > New Revision: 151794
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=151794&view=rev
> > Log:
> > Reject 'a = {0} = {0}' rather than parsing it as '(a = {0}) = {0}'. Also
> > improve the diagnostics for some attempts to use initializer lists in
> > expressions.
> >
> > Modified:
> >    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> >    cfe/trunk/lib/Parse/ParseExpr.cpp
> >    cfe/trunk/test/CXX/expr/expr.ass/p9-cxx11.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=151794&r1=151793&r2=151794&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Feb 29
> 20:59:17 2012
> > @@ -229,6 +229,8 @@
> >  def warn_cxx98_compat_generalized_initializer_lists : Warning<
> >   "generalized initializer lists are incompatible with C++98">,
> >   InGroup<CXX98Compat>, DefaultIgnore;
> > +def err_init_list_bin_op : Error<"initializer list cannot be used on
> the "
> > +  "%select{left|right}0 hand side of operator '%1'">;
> >  def warn_cxx98_compat_trailing_return_type : Warning<
> >   "trailing return types are incompatible with C++98">,
> >   InGroup<CXX98Compat>, DefaultIgnore;
> >
> > Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=151794&r1=151793&r2=151794&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
> > +++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Feb 29 20:59:17 2012
> > @@ -282,6 +282,12 @@
> >     Token OpToken = Tok;
> >     ConsumeToken();
> >
> > +    if (!LHS.isInvalid() && isa<InitListExpr>(LHS.get())) {
> > +      Diag(OpToken, diag::err_init_list_bin_op)
> > +        << /*LHS*/0 << PP.getSpelling(OpToken) <<
> LHS.get()->getSourceRange();
> > +      LHS = ExprError();
> > +    }
> > +
> >     // Special case handling for the ternary operator.
> >     ExprResult TernaryMiddle(true);
> >     if (NextTokPrec == prec::Conditional) {
> > @@ -353,22 +359,16 @@
> >     // Therefore we need some special-casing here.
> >     // Also note that the third operand of the conditional operator is
> >     // an assignment-expression in C++, and in C++11, we can have a
> > -    // braced-init-list on the RHS of an assignment.
> > +    // braced-init-list on the RHS of an assignment. For better
> diagnostics,
> > +    // parse as if we were allowed braced-init-lists everywhere, and
> check that
> > +    // they only appear on the RHS of assignments later.
> >     ExprResult RHS;
> > -    if (getLang().CPlusPlus0x && MinPrec == prec::Assignment &&
> > -        Tok.is(tok::l_brace)) {
> > -      Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
> > +    if (getLang().CPlusPlus0x && Tok.is(tok::l_brace))
> >       RHS = ParseBraceInitializer();
> > -      if (LHS.isInvalid() || RHS.isInvalid())
> > -        return ExprError();
> > -      // A braced-init-list can never be followed by more operators.
> > -      return Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),
> > -                                OpToken.getKind(), LHS.take(),
> RHS.take());
> > -    } else if (getLang().CPlusPlus && NextTokPrec <= prec::Conditional)
> {
> > +    else if (getLang().CPlusPlus && NextTokPrec <= prec::Conditional)
> >       RHS = ParseAssignmentExpression();
> > -    } else {
> > +    else
> >       RHS = ParseCastExpression(false);
> > -    }
> >
> >     if (RHS.isInvalid())
> >       LHS = ExprError();
> > @@ -387,6 +387,11 @@
> >     // more tightly with RHS than we do, evaluate it completely first.
> >     if (ThisPrec < NextTokPrec ||
> >         (ThisPrec == NextTokPrec && isRightAssoc)) {
> > +      if (!LHS.isInvalid() && isa<InitListExpr>(LHS.get())) {
> > +        Diag(OpToken, diag::err_init_list_bin_op)
> > +          << /*LHS*/0 << PP.getSpelling(OpToken) <<
> LHS.get()->getSourceRange();
> > +        LHS = ExprError();
> > +      }
>
> We normally try to avoid the Parser using AST methods directly...
> granted, the distinction between Parser and Sema has started to blur a
> bit, but I think it's still useful to try to maintain.
>
> -Eli
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120229/a9ac61d5/attachment.html>