On Tue, Nov 5, 2013 at 9:56 AM, Kaelyn Uhrain <[email protected]> wrote:
> On Mon, Nov 4, 2013 at 6:37 PM, Richard Smith <[email protected]>wrote: > >> >> On 4 Nov 2013 11:05, "Kaelyn Uhrain" <[email protected]> wrote: >> > >> > Author: rikka >> > Date: Mon Nov 4 12:59:34 2013 >> > New Revision: 194002 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=194002&view=rev >> > Log: >> > Try to correct a mistyped "-" or ">" to "->" for some C++ cases. >> > >> > Similar C code isn't caught as it seems to hit a different code path. >> > Also, as the check is only done for record pointers, cases involving >> > an overloaded operator-> are not handled either. Note that the reason >> > this check is done in the parser instead of Sema is not related to >> > having enough knowledge about the current state as it is about being >> > able to fix up the parser's state to be able to recover and traverse the >> > correct code paths. >> > >> > Modified: >> > cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> > cfe/trunk/lib/Parse/ParseExpr.cpp >> > cfe/trunk/test/SemaCXX/member-expr.cpp >> > >> > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> > URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=194002&r1=194001&r2=194002&view=diff >> > >> ============================================================================== >> > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) >> > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Nov 4 >> 12:59:34 2013 >> > @@ -499,6 +499,9 @@ def ext_abstract_pack_declarator_parens >> > def err_function_is_not_record : Error< >> > "unexpected '%select{.|->}0' in function call; perhaps remove the " >> > "'%select{.|->}0'?">; >> > +def err_mistyped_arrow_in_member_access : Error< >> > + "use of undeclared identifier %0; did you mean '->' instead of " >> > + "'%select{-|>}1'?">; >> > >> > // C++ derived classes >> > def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">; >> > >> > Modified: cfe/trunk/lib/Parse/ParseExpr.cpp >> > URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=194002&r1=194001&r2=194002&view=diff >> > >> ============================================================================== >> > --- cfe/trunk/lib/Parse/ParseExpr.cpp (original) >> > +++ cfe/trunk/lib/Parse/ParseExpr.cpp Mon Nov 4 12:59:34 2013 >> > @@ -166,6 +166,46 @@ ExprResult Parser::ParseAssignmentExpres >> > ExprResult LHS = ParseCastExpression(/*isUnaryExpression=*/false, >> > /*isAddressOfOperand=*/false, >> > isTypeCast); >> > + >> > + // Check for a possible typo of "-" or ">" instead of "->" after a >> > + // pointer to a struct or class, while recovery is still possible. >> > + if (LHS.isUsable() && (Tok.is(tok::minus) || Tok.is(tok::greater))) { >> > + QualType LHSType = LHS.get()->getType(); >> > + const RecordType *Pointee = >> > + LHSType->isPointerType() >> > + ? LHSType->getPointeeType()->getAsStructureType() >> > + : 0; >> > + const RecordDecl *RD = Pointee ? Pointee->getDecl() : 0; >> > + const Token &NextTok = NextToken(); >> > + if (RD && NextTok.is(tok::identifier)) { >> > + UnqualifiedId Name; >> > + CXXScopeSpec ScopeSpec; >> > + SourceLocation TemplateKWLoc; >> > + NoTypoCorrectionCCC NoTCValidator; >> > + Name.setIdentifier(NextTok.getIdentifierInfo(), >> NextTok.getLocation()); >> > + Sema::SFINAETrap Trap(Actions); >> > + ExprResult Res = >> > + Actions.ActOnIdExpression(getCurScope(), ScopeSpec, >> TemplateKWLoc, >> > + Name, false, false, >> &NoTCValidator); >> > + if (Res.isInvalid()) { >> >> What happens if the next token is ( or :: or similar, and this identifier >> is not an id-expression by itself? For instance, p-f(x) might find f by adl. >> > Assuming (1) "f" is both the name of a member of "p" and something not > found through a simple lookup but through adl or similar, and (2) the > pointer arithmetic/comparison on an object pointer is intended, then yes > this will probably do the wrong thing. And while it's not great, working > around that should be pretty trivial (e.g. adding parentheses around the > rhs of the "-" or ">"). > OK, so this means we reject valid code; that's not OK -- please fix or revert. Here's one example of valid code this patch rejects: struct S { int f(...); }; namespace X { struct T {}; int f(T); } S *g(S *p) { return p - f(X::T()); } This also produces a diagnostic with garbage in it: error: use of undeclared identifier '������'; did you mean '->' instead of '-'? Also, in the case where this succeeds, we should annotate the produced >> expression onto the token stream to avoid reparsing it later. >> > I'm not sure what you're saying here... care to explain? > If you're going to the effort to produce an Expr from the identifier, please replace the tokens you used with a tok::annot_primary_expr token so that we won't need to redo the lookup and recreate the Expr when we actually parse the RHS. > > + Token OpTok = Tok; >> > + Tok.setKind(tok::arrow); >> > + PP.EnableBacktrackAtThisPos(); >> > + Res = ParsePostfixExpressionSuffix(LHS); >> > + if (Res.isUsable()) { >> > + LHS = Res; >> > + PP.CommitBacktrackedTokens(); >> > + Diag(OpTok, diag::err_mistyped_arrow_in_member_access) >> > + << NextTok.getIdentifierInfo() << OpTok.is(tok::greater) >> > + << FixItHint::CreateReplacement(OpTok.getLocation(), >> "->"); >> > + } else { >> > + Tok = OpTok; >> > + PP.Backtrack(); >> > + } >> > + } >> > + } >> > + } >> > + >> > return ParseRHSOfBinaryExpression(LHS, prec::Assignment); >> > } >> > >> > >> > Modified: cfe/trunk/test/SemaCXX/member-expr.cpp >> > URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-expr.cpp?rev=194002&r1=194001&r2=194002&view=diff >> > >> ============================================================================== >> > --- cfe/trunk/test/SemaCXX/member-expr.cpp (original) >> > +++ cfe/trunk/test/SemaCXX/member-expr.cpp Mon Nov 4 12:59:34 2013 >> > @@ -224,3 +224,16 @@ namespace pr16676 { >> > .i; // expected-error {{member reference type 'pr16676::S *' >> is a pointer; maybe you meant to use '->'}} >> > } >> > } >> > + >> > +namespace PR9054 { >> > +struct Foo { >> > + void bar(int); >> > + int fiz; >> > +}; >> > + >> > +int test(struct Foo *foo) { >> > + foo-bar(5); // expected-error {{use of undeclared identifier 'bar'; >> did you mean '->' instead of '-'?}} >> > + foo>baz(4); // expected-error-re {{use of undeclared identifier >> 'baz'$}} >> > + return foo>fiz; // expected-error {{use of undeclared identifier >> 'fiz'; did you mean '->' instead of '>'?}} >> > +} >> > +} >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
