[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Kian Moniri via Phabricator via cfe-commits]

kianm added a comment.

In D68896#1861048 , @rsmith wrote:

> In D68896#1861040 , @rsmith wrote:
>
> > In D68896#1778193 , @kianm wrote:
> >
> > > Hi, I am still seeing problems with this assertion. Could we please get a 
> > > fix? I've posted the reduced test case and reproducible command on this 
> > > Phabricator patch.
> >
> >
> > Are you still seeing problems?
>
>
> For future Phabricator visitors, this was fixed in rG2e48be09b 
> .


No I am not seeing problems, they were fixed in rG2e48be09b 
. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D68896#1861040 , @rsmith wrote:

> In D68896#1778193 , @kianm wrote:
>
> > Hi, I am still seeing problems with this assertion. Could we please get a 
> > fix? I've posted the reduced test case and reproducible command on this 
> > Phabricator patch.
>
>
> Are you still seeing problems?


For future Phabricator visitors, this was fixed in rG2e48be09b 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D68896#1778193 , @kianm wrote:

> Hi, I am still seeing problems with this assertion. Could we please get a 
> fix? I've posted the reduced test case and reproducible command on this 
> Phabricator patch.


Are you still seeing problems? If so, please can you file a bug report? I can't 
reproduce this failure with a recent Clang; I get

  :14:3: error: ambiguous partial specializations of 'Base'
Base::f();
^
  :7:7: note: partial specialization matches [with T = A]
  class Base { };
^
  :10:7: note: partial specialization matches [with T = A, I = 1]
  class Base { };
^

but no assertion failure for the posted testcase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith via cfe-commits]

On Tue, 10 Dec 2019 at 13:24, Kian Moniri via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> kianm added a comment.
>
> Hi, I am still seeing problems with this assertion. Could we please get a
> fix? I've posted the reduced test case and reproducible command on this
> Phabricator patch.
>

Thanks for the reminder, fixed in rG2e48be09b.


> Thanks.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68896/new/
>
> https://reviews.llvm.org/D68896
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Kian Moniri via Phabricator via cfe-commits]

kianm added a comment.

Hi, I am still seeing problems with this assertion. Could we please get a fix? 
I've posted the reduced test case and reproducible command on this Phabricator 
patch.

Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Kian Moniri via Phabricator via cfe-commits]

kianm added a comment.

Hi,

There is a problem with this commit because it asserts after errors have been 
recognized. I have attached a reduced test case. The assertion looks to occur 
here:

  clang: 
/home/kianm/llvm/dev/llvm-project/clang/lib/Parse/ParseExprCXX.cpp:585: 
clang::ExprResult clang::Parser::tryParseCXXIdExpression(clang::CXXScopeSpec &, 
bool, clang::Token &): Assertion `SS.isEmpty() && "undeclared non-type 
annotation should be unqualified"' failed.

The reproducible command is:

  clang -cc1 -x c++ reduced.c

F10590725: reduced.c 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7e8fe67f0e26: PR43080: Do not build context-sensitive 
expressions during name classification. (authored by Richard Smith 
richard-l...@metafoo.co.uk).

Changed prior to commit:
  https://reviews.llvm.org/D68896?vs=224765=224910#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CodeGenCXX/odr-use-lookahead.cpp
  clang/test/SemaCXX/lambda-invalid-capture.cpp

Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- clang/test/SemaCXX/lambda-invalid-capture.cpp
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -16,3 +16,10 @@
   auto q = [child]{};
   const int n = sizeof(q);
 }
+
+int pr43080(int i) { // expected-note {{declared here}}
+  return [] { // expected-note {{begins here}}
+return sizeof i <
+  i; // expected-error {{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
+  }();
+}
Index: clang/test/CodeGenCXX/odr-use-lookahead.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/odr-use-lookahead.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+namespace PR43080 {
+  int f(int i) { return sizeof igetName());
 if (Index.first) {
-  InsertOCLBuiltinDeclarationsFromTable(S, R, II, Index.first - 1,
+  InsertOCLBuiltinDeclarationsFromTable(*this, R, II, Index.first - 1,
 Index.second);
   return true;
 }
@@ -860,14 +860,14 @@
   if (unsigned BuiltinID = II->getBuiltinID()) {
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
 // library functions like 'malloc'. Instead, we'll just error.
-if ((S.getLangOpts().CPlusPlus || S.getLangOpts().OpenCL) &&
-S.Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
+if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
+Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
   return false;
 
-if (NamedDecl *D = S.LazilyCreateBuiltin((IdentifierInfo *)II,
- BuiltinID, S.TUScope,
- R.isForRedeclaration(),
- R.getNameLoc())) {
+if (NamedDecl *D = LazilyCreateBuiltin((IdentifierInfo *)II,
+   BuiltinID, TUScope,
+   R.isForRedeclaration(),
+   R.getNameLoc())) {
   R.addDecl(D);
   return true;
 }
@@ -1013,7 +1013,7 @@
 }
   }
 
-  if (!Found && DC->isTranslationUnit() && LookupBuiltin(S, R))
+  if (!Found && DC->isTranslationUnit() && S.LookupBuiltin(R))
 return true;
 
   if (R.getLookupName().getNameKind()
@@ -2011,7 +2011,7 @@
   // If we didn't find a use of this identifier, and if the identifier
   // corresponds to a compiler builtin, create the decl object for the builtin
   // now, injecting it into translation unit scope, and return it.
-  if (AllowBuiltinCreation && LookupBuiltin(*this, R))
+  if (AllowBuiltinCreation && LookupBuiltin(R))
 return true;
 
   // If we didn't find a use of this identifier, the ExternalSource
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2482,23 +2482,20 @@
   return BuildDeclarationNameExpr(SS, R, /* ADL */ false);
 }
 
-/// LookupInObjCMethod - The parser has read a name in, and Sema has
-/// detected that we're currently inside an ObjC method.  Perform some
-/// additional lookup.
+/// The parser has read a name in, and Sema has detected that we're currently
+/// inside an ObjC method. Perform some additional checks and determine if we
+/// should form a reference to an ivar.
 ///
 /// Ideally, most of this would be done by lookup, but there's
 /// actually quite a lot of extra work involved.
-///
-/// Returns a null sentinel to indicate trivial success.
-ExprResult
-Sema::LookupInObjCMethod(LookupResult , Scope *S,
- IdentifierInfo *II, bool AllowBuiltinCreation) {
+DeclResult Sema::LookupIvarInObjCMethod(LookupResult , Scope *S,
+IdentifierInfo *II) {
 

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM

> I'm inclined to defer doing that for now, if that's OK :)

Sure.




Comment at: lib/Sema/SemaDecl.cpp:1191
+  return NameClassification::ContextIndependentExpr(
+  BuildDeclarationNameExpr(SS, Result, ADL));
+}

rsmith wrote:
> efriedma wrote:
> > This doesn't depend on the context... because we're going to throw away the 
> > expression later anyway?  I guess that makes sense.
> Yes, basically; we don't do anything context-dependent right now when 
> building the `UnresolvedLookupExpr`, it's all delayed until we resolve the 
> overload set. I suppose I could make this more explicit by directly creating 
> the `UnresolvedLookupExpr` here, at the cost of duplicating a little of the 
> work done by `BuildDeclarationNameExpr`. WDYT?
I don't think duplicating the code really helps; the comment makes it clear 
enough what's happening here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 224765.
rsmith added a comment.

Add comments for various NC_ classifications. Remove the unused and broken
NC_NestedNameSpecifier classification and the code in ClassifyName that
tries to classify names as nested name specifiers.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896

Files:
  include/clang/Basic/TokenKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  test/CodeGenCXX/odr-use-lookahead.cpp
  test/SemaCXX/lambda-invalid-capture.cpp

Index: test/SemaCXX/lambda-invalid-capture.cpp
===
--- test/SemaCXX/lambda-invalid-capture.cpp
+++ test/SemaCXX/lambda-invalid-capture.cpp
@@ -16,3 +16,10 @@
   auto q = [child]{};
   const int n = sizeof(q);
 }
+
+int pr43080(int i) { // expected-note {{declared here}}
+  return [] { // expected-note {{begins here}}
+return sizeof i <
+  i; // expected-error {{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
+  }();
+}
Index: test/CodeGenCXX/odr-use-lookahead.cpp
===
--- /dev/null
+++ test/CodeGenCXX/odr-use-lookahead.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+namespace PR43080 {
+  int f(int i) { return sizeof igetName());
 if (Index.first) {
-  InsertOCLBuiltinDeclarationsFromTable(S, R, II, Index.first - 1,
+  InsertOCLBuiltinDeclarationsFromTable(*this, R, II, Index.first - 1,
 Index.second);
   return true;
 }
@@ -860,14 +860,14 @@
   if (unsigned BuiltinID = II->getBuiltinID()) {
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
 // library functions like 'malloc'. Instead, we'll just error.
-if ((S.getLangOpts().CPlusPlus || S.getLangOpts().OpenCL) &&
-S.Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
+if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
+Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
   return false;
 
-if (NamedDecl *D = S.LazilyCreateBuiltin((IdentifierInfo *)II,
- BuiltinID, S.TUScope,
- R.isForRedeclaration(),
- R.getNameLoc())) {
+if (NamedDecl *D = LazilyCreateBuiltin((IdentifierInfo *)II,
+   BuiltinID, TUScope,
+   R.isForRedeclaration(),
+   R.getNameLoc())) {
   R.addDecl(D);
   return true;
 }
@@ -1013,7 +1013,7 @@
 }
   }
 
-  if (!Found && DC->isTranslationUnit() && LookupBuiltin(S, R))
+  if (!Found && DC->isTranslationUnit() && S.LookupBuiltin(R))
 return true;
 
   if (R.getLookupName().getNameKind()
@@ -2011,7 +2011,7 @@
   // If we didn't find a use of this identifier, and if the identifier
   // corresponds to a compiler builtin, create the decl object for the builtin
   // now, injecting it into translation unit scope, and return it.
-  if (AllowBuiltinCreation && LookupBuiltin(*this, R))
+  if (AllowBuiltinCreation && LookupBuiltin(R))
 return true;
 
   // If we didn't find a use of this identifier, the ExternalSource
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2482,23 +2482,20 @@
   return BuildDeclarationNameExpr(SS, R, /* ADL */ false);
 }
 
-/// LookupInObjCMethod - The parser has read a name in, and Sema has
-/// detected that we're currently inside an ObjC method.  Perform some
-/// additional lookup.
+/// The parser has read a name in, and Sema has detected that we're currently
+/// inside an ObjC method. Perform some additional checks and determine if we
+/// should form a reference to an ivar.
 ///
 /// Ideally, most of this would be done by lookup, but there's
 /// actually quite a lot of extra work involved.
-///
-/// Returns a null sentinel to indicate trivial success.
-ExprResult
-Sema::LookupInObjCMethod(LookupResult , Scope *S,
- IdentifierInfo *II, bool AllowBuiltinCreation) {
+DeclResult Sema::LookupIvarInObjCMethod(LookupResult , Scope *S,
+IdentifierInfo *II) {
   SourceLocation Loc = Lookup.getNameLoc();
   ObjCMethodDecl *CurMethod = getCurMethodDecl();
 
   // Check for error condition which is already reported.
   if (!CurMethod)
-return ExprError();
+   

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added a comment.

In D68896#1706870 , @efriedma wrote:

> Would it make sense to always use ClassifyName from the parser, instead of 
> using ActOnIdExpression?


I like that idea, at least in principle. We'd need to generalize `ClassifyName` 
to operate on an arbitrary `UnqualifiedId` instead of only on an identifier, 
and we'd still need something to cover the various current calls to 
`ActOnIdExpression` from within `Sema`, so I'm not sure whether it'll actually 
work out cleaner in practice. I'm inclined to defer doing that for now, if 
that's OK :)




Comment at: lib/Sema/SemaDecl.cpp:1191
+  return NameClassification::ContextIndependentExpr(
+  BuildDeclarationNameExpr(SS, Result, ADL));
+}

efriedma wrote:
> This doesn't depend on the context... because we're going to throw away the 
> expression later anyway?  I guess that makes sense.
Yes, basically; we don't do anything context-dependent right now when building 
the `UnresolvedLookupExpr`, it's all delayed until we resolve the overload set. 
I suppose I could make this more explicit by directly creating the 
`UnresolvedLookupExpr` here, at the cost of duplicating a little of the work 
done by `BuildDeclarationNameExpr`. WDYT?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Hmm, the ObjC changes were simpler than I expected.  And you managed to avoid 
making changes to overload sets.

The changes related to IsAddressOfOperand are a nice simplification.

Would it make sense to always use ClassifyName from the parser, instead of 
using ActOnIdExpression?




Comment at: include/clang/Sema/Sema.h:1865
+NC_DependentNonType,
+NC_ContextIndependentExpr,
 NC_NestedNameSpecifier,

This enum could probably use brief comments explaining what the different 
results mean.



Comment at: lib/Sema/SemaDecl.cpp:1191
+  return NameClassification::ContextIndependentExpr(
+  BuildDeclarationNameExpr(SS, Result, ADL));
+}

This doesn't depend on the context... because we're going to throw away the 
expression later anyway?  I guess that makes sense.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68896/new/

https://reviews.llvm.org/D68896



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: efriedma.
Herald added a project: clang.

We don't know what context to use until the classification result is
consumed by the parser, which could happen in a different semantic
context.

This covers everything except C++ implicit class member access, which
is a little awkward to handle properly in the face of the protected
member access check. But it at least fixes all the currently-filed
instances of PR43080.


Repository:
  rC Clang

https://reviews.llvm.org/D68896

Files:
  include/clang/Basic/TokenKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  test/CodeGenCXX/odr-use-lookahead.cpp
  test/SemaCXX/lambda-invalid-capture.cpp

Index: test/SemaCXX/lambda-invalid-capture.cpp
===
--- test/SemaCXX/lambda-invalid-capture.cpp
+++ test/SemaCXX/lambda-invalid-capture.cpp
@@ -16,3 +16,10 @@
   auto q = [child]{};
   const int n = sizeof(q);
 }
+
+int pr43080(int i) { // expected-note {{declared here}}
+  return [] { // expected-note {{begins here}}
+return sizeof i <
+  i; // expected-error {{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
+  }();
+}
Index: test/CodeGenCXX/odr-use-lookahead.cpp
===
--- /dev/null
+++ test/CodeGenCXX/odr-use-lookahead.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+namespace PR43080 {
+  int f(int i) { return sizeof igetName());
 if (Index.first) {
-  InsertOCLBuiltinDeclarationsFromTable(S, R, II, Index.first - 1,
+  InsertOCLBuiltinDeclarationsFromTable(*this, R, II, Index.first - 1,
 Index.second);
   return true;
 }
@@ -860,14 +860,14 @@
   if (unsigned BuiltinID = II->getBuiltinID()) {
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
 // library functions like 'malloc'. Instead, we'll just error.
-if ((S.getLangOpts().CPlusPlus || S.getLangOpts().OpenCL) &&
-S.Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
+if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
+Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
   return false;
 
-if (NamedDecl *D = S.LazilyCreateBuiltin((IdentifierInfo *)II,
- BuiltinID, S.TUScope,
- R.isForRedeclaration(),
- R.getNameLoc())) {
+if (NamedDecl *D = LazilyCreateBuiltin((IdentifierInfo *)II,
+   BuiltinID, TUScope,
+   R.isForRedeclaration(),
+   R.getNameLoc())) {
   R.addDecl(D);
   return true;
 }
@@ -1013,7 +1013,7 @@
 }
   }
 
-  if (!Found && DC->isTranslationUnit() && LookupBuiltin(S, R))
+  if (!Found && DC->isTranslationUnit() && S.LookupBuiltin(R))
 return true;
 
   if (R.getLookupName().getNameKind()
@@ -2011,7 +2011,7 @@
   // If we didn't find a use of this identifier, and if the identifier
   // corresponds to a compiler builtin, create the decl object for the builtin
   // now, injecting it into translation unit scope, and return it.
-  if (AllowBuiltinCreation && LookupBuiltin(*this, R))
+  if (AllowBuiltinCreation && LookupBuiltin(R))
 return true;
 
   // If we didn't find a use of this identifier, the ExternalSource
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2482,23 +2482,20 @@
   return BuildDeclarationNameExpr(SS, R, /* ADL */ false);
 }
 
-/// LookupInObjCMethod - The parser has read a name in, and Sema has
-/// detected that we're currently inside an ObjC method.  Perform some
-/// additional lookup.
+/// The parser has read a name in, and Sema has detected that we're currently
+/// inside an ObjC method. Perform some additional checks and determine if we
+/// should form a reference to an ivar.
 ///
 /// Ideally, most of this would be done by lookup, but there's
 /// actually quite a lot of extra work involved.
-///
-/// Returns a null sentinel to indicate trivial success.
-ExprResult
-Sema::LookupInObjCMethod(LookupResult , Scope *S,
- IdentifierInfo *II, bool AllowBuiltinCreation) {
+DeclResult Sema::LookupIvarInObjCMethod(LookupResult , Scope *S,
+IdentifierInfo *II) {
   SourceLocation Loc = Lookup.getNameLoc();
   ObjCMethodDecl