<div class="gmail_quote">On Mon, Apr 30, 2012 at 5:18 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank">dgregor@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div></div><div class="h5"><br>
On Apr 25, 2012, at 12:49 PM, Kaelyn Uhrain <<a href="mailto:rikka@google.com">rikka@google.com</a>> wrote:<br>
<br>
> Author: rikka<br>
> Date: Wed Apr 25 14:49:54 2012<br>
> New Revision: 155580<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=155580&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=155580&view=rev</a><br>
> Log:<br>
> Add an error message with fixit hint for changing '.' to '->'.<br>
><br>
> This is mainly for attempting to recover in cases where a class provides<br>
> a custom operator-> and a '.' was accidentally used instead of '->' when<br>
> accessing a member of the object returned by the current object's<br>
> operator->.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> cfe/trunk/include/clang/Sema/Sema.h<br>
> cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
> cfe/trunk/test/FixIt/fixit.cpp<br>
> cfe/trunk/test/SemaCXX/arrow-operator.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=155580&r1=155579&r2=155580&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=155580&r1=155579&r2=155580&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 25 14:49:54 2012<br>
> @@ -3641,6 +3641,8 @@<br>
><br>
> def err_typecheck_incomplete_tag : Error<"incomplete definition of type %0">;<br>
> def err_no_member : Error<"no member named %0 in %1">;<br>
> +def err_no_member_overloaded_arrow : Error<<br>
> + "no member named %0 in %1; did you mean to use '->' instead of '.'?">;<br>
><br>
> def err_member_not_yet_instantiated : Error<<br>
> "no member %0 in %1; it has not yet been instantiated">;<br>
><br>
> Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=155580&r1=155579&r2=155580&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=155580&r1=155579&r2=155580&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 25 14:49:54 2012<br>
> @@ -2703,6 +2703,18 @@<br>
> const DeclarationNameInfo &NameInfo,<br>
> const TemplateArgumentListInfo *TemplateArgs);<br>
><br>
> + // This struct is for use by ActOnMemberAccess to allow<br>
> + // BuildMemberReferenceExpr to be able to reinvoke ActOnMemberAccess after<br>
> + // changing the access operator from a '.' to a '->' (to see if that is the<br>
> + // change needed to fix an error about an unknown member, e.g. when the class<br>
> + // defines a custom operator->).<br>
> + struct ActOnMemberAccessExtraArgs {<br>
> + Scope *S;<br>
> + UnqualifiedId &Id;<br>
> + Decl *ObjCImpDecl;<br>
> + bool HasTrailingLParen;<br>
> + };<br>
> +<br>
> ExprResult BuildMemberReferenceExpr(Expr *Base, QualType BaseType,<br>
> SourceLocation OpLoc, bool IsArrow,<br>
> const CXXScopeSpec &SS,<br>
> @@ -2710,7 +2722,8 @@<br>
> NamedDecl *FirstQualifierInScope,<br>
> LookupResult &R,<br>
> const TemplateArgumentListInfo *TemplateArgs,<br>
> - bool SuppressQualifierCheck = false);<br>
> + bool SuppressQualifierCheck = false,<br>
> + ActOnMemberAccessExtraArgs *ExtraArgs = 0);<br>
><br>
> ExprResult PerformMemberExprBaseConversion(Expr *Base, bool IsArrow);<br>
> ExprResult LookupMemberExpr(LookupResult &R, ExprResult &Base,<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=155580&r1=155579&r2=155580&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=155580&r1=155579&r2=155580&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Wed Apr 25 14:49:54 2012<br>
> @@ -813,8 +813,9 @@<br>
> SourceLocation TemplateKWLoc,<br>
> NamedDecl *FirstQualifierInScope,<br>
> LookupResult &R,<br>
> - const TemplateArgumentListInfo *TemplateArgs,<br>
> - bool SuppressQualifierCheck) {<br>
> + const TemplateArgumentListInfo *TemplateArgs,<br>
> + bool SuppressQualifierCheck,<br>
> + ActOnMemberAccessExtraArgs *ExtraArgs) {<br>
> QualType BaseType = BaseExprType;<br>
> if (IsArrow) {<br>
> assert(BaseType->isPointerType());<br>
> @@ -835,6 +836,32 @@<br>
> ? computeDeclContext(SS, false)<br>
> : BaseType->getAs<RecordType>()->getDecl());<br>
><br>
> + if (ExtraArgs) {<br>
> + ExprResult RetryExpr;<br>
> + if (!IsArrow && BaseExpr) {<br>
> + SFINAETrap Trap(*this);<br>
<br>
</div></div>I think you want<br>
<br>
SFINAETrap Trap(*this, true);<br>
<br>
to SFINAE'ify access checking as well.<br></blockquote><div><br></div><div>Good point. Added this in r155871 (screwed it up in r155870 because I managed to read the email wrong and saw a "false" instead of a "true").</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> + ParsedType ObjectType;<br>
> + bool MayBePseudoDestructor = false;<br>
> + RetryExpr = ActOnStartCXXMemberReference(getCurScope(), BaseExpr,<br>
> + OpLoc, tok::arrow, ObjectType,<br>
> + MayBePseudoDestructor);<br>
> + if (RetryExpr.isUsable() && !Trap.hasErrorOccurred()) {<br>
> + CXXScopeSpec TempSS(SS);<br>
> + RetryExpr = ActOnMemberAccessExpr(<br>
> + ExtraArgs->S, RetryExpr.get(), OpLoc, tok::arrow, TempSS,<br>
> + TemplateKWLoc, ExtraArgs->Id, ExtraArgs->ObjCImpDecl,<br>
> + ExtraArgs->HasTrailingLParen);<br>
> + }<br>
<br>
</div>It makes me a bit nervous to be recursing like this, because it would be very easy for a later refactor to introduce a "->" -> to "." replacement and get into an infinite loop. I'd rather that we at least pass down a bit that says "we're recovering; don't try to recover further".<br>
</blockquote><div><br></div><div>As long as ActOnMemberAccessExpr doesn't pass false for IsArrow when calling BuildMemberReferenceExpr when ActOnMemberAccessExpr was called with tok::arrow as the OpKind, it won't recurse more than once. I think the introduction of a "->" to "." replacement within ActOnMemberAccessExpr would be time enough to introduce yet another flag to indicate whether the recovery has already been attempted (this wouldn't be the first place where introducing a particular kind of replacement for recovering from errors would introduce an infinite loop).</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks for working on this!<br></blockquote><div><br></div><div>You're welcome! :)</div><div></div></div><br><div>Cheers,</div><div>Kaelyn</div>