r187504 - Improve the diagnostic experience, including adding recovery, for
David Blaikie
dblaikie at gmail.com
Wed Jul 31 11:00:23 PDT 2013
On Wed, Jul 31, 2013 at 10:38 AM, Kaelyn Uhrain <rikka at google.com> wrote:
> Author: rikka
> Date: Wed Jul 31 12:38:24 2013
> New Revision: 187504
>
> URL: http://llvm.org/viewvc/llvm-project?rev=187504&view=rev
> Log:
> Improve the diagnostic experience, including adding recovery, for
> changing '->' to '.' when there is no operator-> defined for a class.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaExprMember.cpp
> cfe/trunk/lib/Sema/SemaOverload.cpp
> cfe/trunk/test/SemaCXX/member-expr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 31 12:38:24 2013
> @@ -4233,6 +4233,8 @@ def err_typecheck_member_reference_sugge
> "member reference type %0 is %select{a|not a}1 pointer; maybe you meant to use '%select{->|.}1'?">;
> def note_typecheck_member_reference_suggestion : Note<
> "did you mean to use '.' instead?">;
> +def note_member_reference_arrow_from_operator_arrow : Note<
> + "'->' applied to return value of the operator->() declared here">;
> def err_typecheck_member_reference_type : Error<
> "cannot refer to type member %0 in %1 with '%select{.|->}2'">;
> def err_typecheck_member_reference_unknown : Error<
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Jul 31 12:38:24 2013
> @@ -2256,7 +2256,8 @@ public:
> SourceLocation RParenLoc);
>
> ExprResult BuildOverloadedArrowExpr(Scope *S, Expr *Base,
> - SourceLocation OpLoc);
> + SourceLocation OpLoc,
> + bool *NoArrowOperatorFound = 0);
>
> /// CheckCallReturnType - Checks that a call expression's return type is
> /// complete. Returns true on failure. The location passed in is the location
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 31 12:38:24 2013
> @@ -5076,15 +5076,44 @@ Sema::ActOnStartCXXMemberReference(Scope
> // [...] When operator->returns, the operator-> is applied to the value
> // returned, with the original second operand.
> if (OpKind == tok::arrow) {
> + bool NoArrowOperatorFound = false;
> + bool FirstIteration = true;
> + FunctionDecl *CurFD = dyn_cast<FunctionDecl>(CurContext);
> // The set of types we've considered so far.
> llvm::SmallPtrSet<CanQualType,8> CTypes;
> SmallVector<SourceLocation, 8> Locations;
> CTypes.insert(Context.getCanonicalType(BaseType));
>
> while (BaseType->isRecordType()) {
> - Result = BuildOverloadedArrowExpr(S, Base, OpLoc);
> - if (Result.isInvalid())
> + Result = BuildOverloadedArrowExpr(
> + S, Base, OpLoc,
> + // When in a template specialization and on the first loop iteration,
> + // potentially give the default diagnostic (with the fixit in a
> + // separate note) instead of having the error reported back to here
> + // and giving a diagnostic with a fixit attached to the error itself.
> + (FirstIteration && CurFD && CurFD->isFunctionTemplateSpecialization())
> + ? 0
> + : &NoArrowOperatorFound);
> + if (Result.isInvalid()) {
> + if (NoArrowOperatorFound) {
> + if (FirstIteration) {
> + Diag(OpLoc, diag::err_typecheck_member_reference_suggestion)
> + << BaseType << 1 << Base->getSourceRange()
> + << FixItHint::CreateReplacement(OpLoc, ".");
> + OpKind = tok::period;
> + break;
> + } else {
'else' after 'break', just drop the 'else' here.
> + Diag(OpLoc, diag::err_typecheck_member_reference_arrow)
> + << BaseType << Base->getSourceRange();
> + CallExpr *CE = dyn_cast<CallExpr>(Base);
> + if (Decl *CD = (CE ? CE->getCalleeDecl() : 0)) {
> + Diag(CD->getLocStart(),
> + diag::note_member_reference_arrow_from_operator_arrow);
> + }
> + }
> + }
> return ExprError();
> + }
> Base = Result.get();
> if (CXXOperatorCallExpr *OpCall = dyn_cast<CXXOperatorCallExpr>(Base))
> Locations.push_back(OpCall->getDirectCallee()->getLocation());
> @@ -5096,9 +5125,11 @@ Sema::ActOnStartCXXMemberReference(Scope
> Diag(Locations[i], diag::note_declared_at);
> return ExprError();
> }
> + FirstIteration = false;
> }
>
> - if (BaseType->isPointerType() || BaseType->isObjCObjectPointerType())
> + if (OpKind == tok::arrow &&
> + (BaseType->isPointerType() || BaseType->isObjCObjectPointerType()))
> BaseType = BaseType->getPointeeType();
> }
>
>
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Wed Jul 31 12:38:24 2013
> @@ -1136,10 +1136,8 @@ Sema::LookupMemberExpr(LookupResult &R,
> // foo->bar
> // This is actually well-formed in C++ if MyRecord has an
> // overloaded operator->, but that should have been dealt with
> - // by now.
> - Diag(OpLoc, diag::err_typecheck_member_reference_suggestion)
> - << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange()
> - << FixItHint::CreateReplacement(OpLoc, ".");
> + // by now--or a diagnostic message already issued if a problem
> + // was encountered while looking for the overloaded operator->.
> IsArrow = false;
> } else if (BaseType->isFunctionType()) {
> goto fail;
>
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Jul 31 12:38:24 2013
> @@ -11463,7 +11463,8 @@ Sema::BuildCallToObjectOfClassType(Scope
> /// (if one exists), where @c Base is an expression of class type and
> /// @c Member is the name of the member we're trying to find.
> ExprResult
> -Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc) {
> +Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
> + bool *NoArrowOperatorFound) {
> assert(Base->getType()->isRecordType() &&
> "left-hand side must have class type");
>
> @@ -11509,6 +11510,12 @@ Sema::BuildOverloadedArrowExpr(Scope *S,
> case OR_No_Viable_Function:
> if (CandidateSet.empty()) {
> QualType BaseType = Base->getType();
> + if (NoArrowOperatorFound) {
> + // Report this specific error to the caller instead of emitting a
> + // diagnostic, as requested.
> + *NoArrowOperatorFound = true;
> + return ExprError();
> + }
> Diag(OpLoc, diag::err_typecheck_member_reference_arrow)
> << BaseType << Base->getSourceRange();
> if (BaseType->isRecordType() && !BaseType->isPointerType()) {
>
> Modified: cfe/trunk/test/SemaCXX/member-expr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-expr.cpp?rev=187504&r1=187503&r2=187504&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/member-expr.cpp (original)
> +++ cfe/trunk/test/SemaCXX/member-expr.cpp Wed Jul 31 12:38:24 2013
> @@ -79,16 +79,13 @@ namespace test5 {
> };
>
> void test0(int x) {
> - x.A::foo<int>(); // expected-error {{'int' is not a structure or union}}
If you're removing these lines, you can probably remove these testN
functions entirely (& perhaps rename test2 to test0 or test1).
> }
>
> void test1(A *x) {
> - x.A::foo<int>(); // expected-error {{'test5::A *' is a pointer}}
> }
>
> void test2(A &x) {
> - x->A::foo<int>(); // expected-error {{'test5::A' is not a pointer}} \
> - // expected-note {{did you mean to use '.' instead?}}
> + x->A::foo<int>(); // expected-error {{'test5::A' is not a pointer; maybe you meant to use '.'?}}
> }
> }
>
> @@ -182,7 +179,36 @@ namespace PR15045 {
>
> int f() {
> Cl0 c;
> - return c->a; // expected-error {{member reference type 'PR15045::Cl0' is not a pointer}} \
> - // expected-note {{did you mean to use '.' instead?}}
> + return c->a; // expected-error {{member reference type 'PR15045::Cl0' is not a pointer; maybe you meant to use '.'?}}
> + }
> +
> + struct bar {
> + void func(); // expected-note {{'func' declared here}}
> + };
> +
> + struct foo {
> + bar operator->(); // expected-note 2 {{'->' applied to return value of the operator->() declared here}}
> + };
> +
> + template <class T> void call_func(T t) {
> + t->func(); // expected-error-re 2 {{member reference type 'PR15045::bar' is not a pointer$}} \
> + // expected-note {{did you mean to use '.' instead?}}
> + }
> +
> + void test_arrow_on_non_pointer_records() {
> + bar e;
> + foo f;
> +
> + // Show that recovery has happened by also triggering typo correction
Recovery has happened - but has the fixit actually been issued? We
have tests for fixit hints that either test the machine readable form
and/or rerun the compiler on the fixit'd code (I assume the machine
readable tests are better, but not sure what the general
consensus/plan is). See the tests under test/FixIt.
> + e->Func(); // expected-error {{member reference type 'PR15045::bar' is not a pointer; maybe you meant to use '.'?}} \
> + // expected-error {{no member named 'Func' in 'PR15045::bar'; did you mean 'func'?}}
> +
> + // Make sure a fixit isn't given in the case that the '->' isn't actually
> + // the problem (the problem is with the return value of an operator->).
> + f->func(); // expected-error-re {{member reference type 'PR15045::bar' is not a pointer$}}
> +
> + call_func(e); // expected-note {{in instantiation of function template specialization 'PR15045::call_func<PR15045::bar>' requested here}}
> +
> + call_func(f); // expected-note {{in instantiation of function template specialization 'PR15045::call_func<PR15045::foo>' requested here}}
> }
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list