Re: [PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-02-01 Thread Hans Wennborg via cfe-commits 
On Tue, Jan 31, 2017 at 3:23 PM, Richard Smith  wrote:
> On 31 January 2017 at 14:49, Hans Wennborg  wrote:
>>
>> Richard, what do you think? I don't believe this fixes a 3.9
>> regression, but the fix looks small, so it might be worth it.
>
>
> I think on the whole the benefit of this change outweighs the risk of it
> regressing something.

Merged in r293782.

Thanks,
Hans

>
>>
>> On Tue, Jan 31, 2017 at 12:07 PM, Akira Hatanaka 
>> wrote:
>> > Thanks for the review. r293678.
>> >
>> > Should this be merged to 4.0?
>> >
>> >> On Jan 31, 2017, at 12:04 PM, Akira Hatanaka via Phabricator via
>> >> cfe-commits  wrote:
>> >>
>> >> This revision was automatically updated to reflect the committed
>> >> changes.
>> >> Closed by commit rL293678: [Sema] Transform a templated name before
>> >> looking it up in (authored by ahatanak).
>> >>
>> >> Changed prior to commit:
>> >>  https://reviews.llvm.org/D24969?vs=82134=86474#toc
>> >>
>> >> Repository:
>> >>  rL LLVM
>> >>
>> >> https://reviews.llvm.org/D24969
>> >>
>> >> Files:
>> >>  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> >>  cfe/trunk/lib/Sema/TreeTransform.h
>> >>  cfe/trunk/test/SemaCXX/destructor.cpp
>> >>
>> >>
>> >> Index: cfe/trunk/test/SemaCXX/destructor.cpp
>> >> ===
>> >> --- cfe/trunk/test/SemaCXX/destructor.cpp
>> >> +++ cfe/trunk/test/SemaCXX/destructor.cpp
>> >> @@ -431,3 +431,23 @@
>> >>
>> >> // The constructor definition should not have errors
>> >> Invalid::~Invalid() {}
>> >> +
>> >> +namespace PR30361 {
>> >> +template 
>> >> +struct C1 {
>> >> +  ~C1() {}
>> >> +  operator C1* () { return nullptr; }
>> >> +  void foo1();
>> >> +};
>> >> +
>> >> +template
>> >> +void C1::foo1() {
>> >> +  C1::operator C1*();
>> >> +  C1::~C1();
>> >> +}
>> >> +
>> >> +void foo1() {
>> >> +  C1 x;
>> >> +  x.foo1();
>> >> +}
>> >> +}
>> >> Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> >> ===
>> >> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> >> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> >> @@ -4990,8 +4990,12 @@
>> >> NamedDecl *Result = nullptr;
>> >> // FIXME: If the name is a dependent name, this lookup won't
>> >> necessarily
>> >> // find it. Does that ever matter?
>> >> -if (D->getDeclName()) {
>> >> -  DeclContext::lookup_result Found =
>> >> ParentDC->lookup(D->getDeclName());
>> >> +if (auto Name = D->getDeclName()) {
>> >> +  DeclarationNameInfo NameInfo(Name, D->getLocation());
>> >> +  Name = SubstDeclarationNameInfo(NameInfo,
>> >> TemplateArgs).getName();
>> >> +  if (!Name)
>> >> +return nullptr;
>> >> +  DeclContext::lookup_result Found = ParentDC->lookup(Name);
>> >>   Result = findInstantiationOf(Context, D, Found.begin(),
>> >> Found.end());
>> >> } else {
>> >>   // Since we don't have a name for the entity we're looking for,
>> >> Index: cfe/trunk/lib/Sema/TreeTransform.h
>> >> ===
>> >> --- cfe/trunk/lib/Sema/TreeTransform.h
>> >> +++ cfe/trunk/lib/Sema/TreeTransform.h
>> >> @@ -8966,12 +8966,18 @@
>> >>   // base (and therefore couldn't do the check) and a
>> >>   // nested-name-qualifier (and therefore could do the lookup).
>> >>   NamedDecl *FirstQualifierInScope = nullptr;
>> >> +  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
>> >> +  if (MemberNameInfo.getName()) {
>> >> +MemberNameInfo =
>> >> getDerived().TransformDeclarationNameInfo(MemberNameInfo);
>> >> +if (!MemberNameInfo.getName())
>> >> +  return ExprError();
>> >> +  }
>> >>
>> >>   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
>> >> E->isArrow(),
>> >> QualifierLoc,
>> >> TemplateKWLoc,
>> >> -E->getMemberNameInfo(),
>> >> +MemberNameInfo,
>> >> Member,
>> >> FoundDecl,
>> >> (E->hasExplicitTemplateArgs()
>> >>
>> >>
>> >> ___
>> >> cfe-commits mailing list
>> >> cfe-commits@lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-31 Thread Richard Smith via cfe-commits 
On 31 January 2017 at 14:49, Hans Wennborg  wrote:

> Richard, what do you think? I don't believe this fixes a 3.9
> regression, but the fix looks small, so it might be worth it.
>

I think on the whole the benefit of this change outweighs the risk of it
regressing something.


> On Tue, Jan 31, 2017 at 12:07 PM, Akira Hatanaka 
> wrote:
> > Thanks for the review. r293678.
> >
> > Should this be merged to 4.0?
> >
> >> On Jan 31, 2017, at 12:04 PM, Akira Hatanaka via Phabricator via
> cfe-commits  wrote:
> >>
> >> This revision was automatically updated to reflect the committed
> changes.
> >> Closed by commit rL293678: [Sema] Transform a templated name before
> looking it up in (authored by ahatanak).
> >>
> >> Changed prior to commit:
> >>  https://reviews.llvm.org/D24969?vs=82134=86474#toc
> >>
> >> Repository:
> >>  rL LLVM
> >>
> >> https://reviews.llvm.org/D24969
> >>
> >> Files:
> >>  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >>  cfe/trunk/lib/Sema/TreeTransform.h
> >>  cfe/trunk/test/SemaCXX/destructor.cpp
> >>
> >>
> >> Index: cfe/trunk/test/SemaCXX/destructor.cpp
> >> ===
> >> --- cfe/trunk/test/SemaCXX/destructor.cpp
> >> +++ cfe/trunk/test/SemaCXX/destructor.cpp
> >> @@ -431,3 +431,23 @@
> >>
> >> // The constructor definition should not have errors
> >> Invalid::~Invalid() {}
> >> +
> >> +namespace PR30361 {
> >> +template 
> >> +struct C1 {
> >> +  ~C1() {}
> >> +  operator C1* () { return nullptr; }
> >> +  void foo1();
> >> +};
> >> +
> >> +template
> >> +void C1::foo1() {
> >> +  C1::operator C1*();
> >> +  C1::~C1();
> >> +}
> >> +
> >> +void foo1() {
> >> +  C1 x;
> >> +  x.foo1();
> >> +}
> >> +}
> >> Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> ===
> >> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> @@ -4990,8 +4990,12 @@
> >> NamedDecl *Result = nullptr;
> >> // FIXME: If the name is a dependent name, this lookup won't
> necessarily
> >> // find it. Does that ever matter?
> >> -if (D->getDeclName()) {
> >> -  DeclContext::lookup_result Found = ParentDC->lookup(D->
> getDeclName());
> >> +if (auto Name = D->getDeclName()) {
> >> +  DeclarationNameInfo NameInfo(Name, D->getLocation());
> >> +  Name = SubstDeclarationNameInfo(NameInfo,
> TemplateArgs).getName();
> >> +  if (!Name)
> >> +return nullptr;
> >> +  DeclContext::lookup_result Found = ParentDC->lookup(Name);
> >>   Result = findInstantiationOf(Context, D, Found.begin(),
> Found.end());
> >> } else {
> >>   // Since we don't have a name for the entity we're looking for,
> >> Index: cfe/trunk/lib/Sema/TreeTransform.h
> >> ===
> >> --- cfe/trunk/lib/Sema/TreeTransform.h
> >> +++ cfe/trunk/lib/Sema/TreeTransform.h
> >> @@ -8966,12 +8966,18 @@
> >>   // base (and therefore couldn't do the check) and a
> >>   // nested-name-qualifier (and therefore could do the lookup).
> >>   NamedDecl *FirstQualifierInScope = nullptr;
> >> +  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
> >> +  if (MemberNameInfo.getName()) {
> >> +MemberNameInfo = getDerived().TransformDeclarationNameInfo(
> MemberNameInfo);
> >> +if (!MemberNameInfo.getName())
> >> +  return ExprError();
> >> +  }
> >>
> >>   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
> >> E->isArrow(),
> >> QualifierLoc,
> >> TemplateKWLoc,
> >> -E->getMemberNameInfo(),
> >> +MemberNameInfo,
> >> Member,
> >> FoundDecl,
> >> (E->hasExplicitTemplateArgs()
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-31 Thread Hans Wennborg via cfe-commits 
Richard, what do you think? I don't believe this fixes a 3.9
regression, but the fix looks small, so it might be worth it.

On Tue, Jan 31, 2017 at 12:07 PM, Akira Hatanaka  wrote:
> Thanks for the review. r293678.
>
> Should this be merged to 4.0?
>
>> On Jan 31, 2017, at 12:04 PM, Akira Hatanaka via Phabricator via cfe-commits 
>>  wrote:
>>
>> This revision was automatically updated to reflect the committed changes.
>> Closed by commit rL293678: [Sema] Transform a templated name before looking 
>> it up in (authored by ahatanak).
>>
>> Changed prior to commit:
>>  https://reviews.llvm.org/D24969?vs=82134=86474#toc
>>
>> Repository:
>>  rL LLVM
>>
>> https://reviews.llvm.org/D24969
>>
>> Files:
>>  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>  cfe/trunk/lib/Sema/TreeTransform.h
>>  cfe/trunk/test/SemaCXX/destructor.cpp
>>
>>
>> Index: cfe/trunk/test/SemaCXX/destructor.cpp
>> ===
>> --- cfe/trunk/test/SemaCXX/destructor.cpp
>> +++ cfe/trunk/test/SemaCXX/destructor.cpp
>> @@ -431,3 +431,23 @@
>>
>> // The constructor definition should not have errors
>> Invalid::~Invalid() {}
>> +
>> +namespace PR30361 {
>> +template 
>> +struct C1 {
>> +  ~C1() {}
>> +  operator C1* () { return nullptr; }
>> +  void foo1();
>> +};
>> +
>> +template
>> +void C1::foo1() {
>> +  C1::operator C1*();
>> +  C1::~C1();
>> +}
>> +
>> +void foo1() {
>> +  C1 x;
>> +  x.foo1();
>> +}
>> +}
>> Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> ===
>> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> @@ -4990,8 +4990,12 @@
>> NamedDecl *Result = nullptr;
>> // FIXME: If the name is a dependent name, this lookup won't necessarily
>> // find it. Does that ever matter?
>> -if (D->getDeclName()) {
>> -  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
>> +if (auto Name = D->getDeclName()) {
>> +  DeclarationNameInfo NameInfo(Name, D->getLocation());
>> +  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
>> +  if (!Name)
>> +return nullptr;
>> +  DeclContext::lookup_result Found = ParentDC->lookup(Name);
>>   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
>> } else {
>>   // Since we don't have a name for the entity we're looking for,
>> Index: cfe/trunk/lib/Sema/TreeTransform.h
>> ===
>> --- cfe/trunk/lib/Sema/TreeTransform.h
>> +++ cfe/trunk/lib/Sema/TreeTransform.h
>> @@ -8966,12 +8966,18 @@
>>   // base (and therefore couldn't do the check) and a
>>   // nested-name-qualifier (and therefore could do the lookup).
>>   NamedDecl *FirstQualifierInScope = nullptr;
>> +  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
>> +  if (MemberNameInfo.getName()) {
>> +MemberNameInfo = 
>> getDerived().TransformDeclarationNameInfo(MemberNameInfo);
>> +if (!MemberNameInfo.getName())
>> +  return ExprError();
>> +  }
>>
>>   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
>> E->isArrow(),
>> QualifierLoc,
>> TemplateKWLoc,
>> -E->getMemberNameInfo(),
>> +MemberNameInfo,
>> Member,
>> FoundDecl,
>> (E->hasExplicitTemplateArgs()
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-31 Thread Akira Hatanaka via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293678: [Sema] Transform a templated name before looking it 
up in (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D24969?vs=82134=86474#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24969

Files:
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/SemaCXX/destructor.cpp


Index: cfe/trunk/test/SemaCXX/destructor.cpp
===
--- cfe/trunk/test/SemaCXX/destructor.cpp
+++ cfe/trunk/test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4990,8 +4990,12 @@
 NamedDecl *Result = nullptr;
 // FIXME: If the name is a dependent name, this lookup won't necessarily
 // find it. Does that ever matter?
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -8966,12 +8966,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+  if (MemberNameInfo.getName()) {
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+if (!MemberNameInfo.getName())
+  return ExprError();
+  }
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()


Index: cfe/trunk/test/SemaCXX/destructor.cpp
===
--- cfe/trunk/test/SemaCXX/destructor.cpp
+++ cfe/trunk/test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4990,8 +4990,12 @@
 NamedDecl *Result = nullptr;
 // FIXME: If the name is a dependent name, this lookup won't necessarily
 // find it. Does that ever matter?
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -8966,12 +8966,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2017-01-09 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak added a comment.

Richard, does the updated patch look OK?


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak added inline comments.



Comment at: lib/Sema/TreeTransform.h:8766-8767
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo =
+  getDerived().TransformDeclarationNameInfo(E->getMemberNameInfo());
 

rsmith wrote:
> Likewise here, you should return `ExprError()` if the transformed name is 
> null (please also add an assert that the name is not null before the 
> transform, as in that case a null transformed name would not indicate an 
> error has been diagnosed).
I discovered that the name can be null when the member expression is a member 
of an anonymous union and therefore an assert would cause a few regression 
tests to fail. For example, the following code would assert:


```
struct T0 {
  union {
void *m0;
  };
};
template 
struct T1 : public T0 {
  void f0() {
m0 = 0;
  }
};

struct A : public T0 { };

void f1(T1 *S) { S->f0(); }
```


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak updated this revision to Diff 82134.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Add code for error recovery.


https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8763,12 +8763,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+  if (MemberNameInfo.getName()) {
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+if (!MemberNameInfo.getName())
+  return ExprError();
+  }
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4846,8 +4846,12 @@
 }
 
 NamedDecl *Result = nullptr;
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8763,12 +8763,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+  if (MemberNameInfo.getName()) {
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+if (!MemberNameInfo.getName())
+  return ExprError();
+  }
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4846,8 +4846,12 @@
 }
 
 NamedDecl *Result = nullptr;
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Richard Smith via Phabricator via cfe-commits 
rsmith added a comment.

Looks good, other than error recovery.




Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4851
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);

You should deal with the possibility of `SubstDeclarationNameInfo` failing 
(which will happen if substitution into the name creates an invalid type). If 
you get a null name back from the `Subst` call, just return null.



Comment at: lib/Sema/TreeTransform.h:8766-8767
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo =
+  getDerived().TransformDeclarationNameInfo(E->getMemberNameInfo());
 

Likewise here, you should return `ExprError()` if the transformed name is null 
(please also add an assert that the name is not null before the transform, as 
in that case a null transformed name would not indicate an error has been 
diagnosed).


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak updated this revision to Diff 82120.
ahatanak added a comment.

Call TransformDeclarationNameInfo unconditionally to transform NameInfo rather 
than doing so only for destructors. I think this is a cleaner fix.


https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8763,12 +8763,14 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo =
+  getDerived().TransformDeclarationNameInfo(E->getMemberNameInfo());
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4846,8 +4846,10 @@
 }
 
 NamedDecl *Result = nullptr;
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8763,12 +8763,14 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo =
+  getDerived().TransformDeclarationNameInfo(E->getMemberNameInfo());
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4846,8 +4846,10 @@
 }
 
 NamedDecl *Result = nullptr;
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
___
cfe-commits mailing list

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-12-12 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak updated this revision to Diff 81172.
ahatanak added a comment.

Rebase and ping.


https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8754,11 +8754,16 @@
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
 
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+
+  if (isa(FoundDecl))
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4843,7 +4843,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), 
TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8754,11 +8754,16 @@
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
 
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+
+  if (isa(FoundDecl))
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4843,7 +4843,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-11-03 Thread Akira Hatanaka via cfe-commits 
ahatanak updated this revision to Diff 76883.
ahatanak added a comment.

Rebase. Pass the destructor's DeclarationNameInfo to the call to 
RebuildMemberExpr rather than getting it inside RebuildMemberExpr.


https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8723,11 +8723,16 @@
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
 
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+
+  if (isa(FoundDecl))
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4842,7 +4842,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), 
TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8723,11 +8723,16 @@
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
 
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+
+  if (isa(FoundDecl))
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4842,7 +4842,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
___
cfe-commits mailing list

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-11 Thread Akira Hatanaka via cfe-commits 
ahatanak marked an inline comment as done.
ahatanak added a comment.

(I'm replying to the comment near TreeTransform.h:11967 because phab doesn't 
let me hit save)

It looks like RebuildCXXPseudoDestructorExpr isn't even called because a 
MemberExpr is created instead of a CXXPseudoDestructorExpr.




Comment at: lib/Sema/TreeTransform.h:2127
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);

rsmith wrote:
> I don't see any reason why this should be specific to destructors. But this 
> is the wrong place for this change; by the time we reach `Rebuild*` we should 
> have already transformed everything that we need to transform.
> 
> Perhaps the problem is instead...
If we want to do the transformation before entering RebuildMemberExpr, I think 
it's also possible to do it in TreeTransform::TransformMemberExpr.


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-11 Thread Akira Hatanaka via cfe-commits 
ahatanak updated this revision to Diff 74324.
ahatanak added a comment.

Added a call to operator C1* in test case.


https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -2119,6 +2119,11 @@
NamedDecl *FirstQualifierInScope) {
 ExprResult BaseResult = getSema().PerformMemberExprBaseConversion(Base,
   isArrow);
+DeclarationNameInfo DNI = MemberNameInfo;
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
 if (!Member->getDeclName()) {
   // We have a reference to an unnamed field.  This is always the
   // base of an anonymous struct/union member access, i.e. the
@@ -2136,7 +2141,7 @@
   Base = BaseResult.get();
   ExprValueKind VK = isArrow ? VK_LValue : Base->getValueKind();
   MemberExpr *ME = new (getSema().Context)
-  MemberExpr(Base, isArrow, OpLoc, Member, MemberNameInfo,
+  MemberExpr(Base, isArrow, OpLoc, Member, DNI,
  cast(Member)->getType(), VK, OK_Ordinary);
   return ME;
 }
@@ -2149,7 +2154,7 @@
 
 // FIXME: this involves duplicating earlier analysis in a lot of
 // cases; we should avoid this when possible.
-LookupResult R(getSema(), MemberNameInfo, Sema::LookupMemberName);
+LookupResult R(getSema(), DNI, Sema::LookupMemberName);
 R.addDecl(FoundDecl);
 R.resolveKind();
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4846,7 +4846,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), 
TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -2119,6 +2119,11 @@
NamedDecl *FirstQualifierInScope) {
 ExprResult BaseResult = getSema().PerformMemberExprBaseConversion(Base,
   isArrow);
+DeclarationNameInfo DNI = MemberNameInfo;
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
 if (!Member->getDeclName()) {
   // We have a reference to an unnamed field.  This is always the
   // base of an anonymous struct/union member access, i.e. the
@@ -2136,7 +2141,7 @@
   Base = BaseResult.get();
   ExprValueKind VK = isArrow ? VK_LValue : Base->getValueKind();
   MemberExpr *ME = new (getSema().Context)
-  MemberExpr(Base, isArrow, OpLoc, Member, MemberNameInfo,
+  MemberExpr(Base, isArrow, OpLoc, Member, DNI,
  cast(Member)->getType(), VK, OK_Ordinary);
   return ME;
 }
@@ -2149,7 +2154,7 @@
 
 // FIXME: this involves duplicating earlier analysis in a lot of
 // cases; we should avoid this when possible.
-LookupResult R(getSema(), MemberNameInfo, Sema::LookupMemberName);
+LookupResult R(getSema(), DNI, Sema::LookupMemberName);
 R.addDecl(FoundDecl);
 R.resolveKind();
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-11 Thread Richard Smith via cfe-commits 
rsmith added inline comments.



Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4849
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =

ahatanak wrote:
> rsmith wrote:
> > Do we need to do this for conversion function names too? (Eg, `operator 
> > C1*`)
> I added a definition of "operator C1*" and called it in C1::foo1. I didn't 
> see any crashes or asserts.
> 
> Calls to a destructor and a conversion function create different AST nodes 
> (the former creates a MemberExpr and the latter creates a 
> CXXDependentScopeMemberExpr) and are transformed by different functions of 
> TreeTransform.
Please also check in that testcase.



Comment at: lib/Sema/TreeTransform.h:2127
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);

I don't see any reason why this should be specific to destructors. But this is 
the wrong place for this change; by the time we reach `Rebuild*` we should have 
already transformed everything that we need to transform.

Perhaps the problem is instead...



Comment at: lib/Sema/TreeTransform.h:11967-11969
   TypeSourceInfo *DestroyedType = Destroyed.getTypeSourceInfo();
   DeclarationName Name(SemaRef.Context.DeclarationNames.getCXXDestructorName(
  SemaRef.Context.getCanonicalType(DestroyedType->getType(;

... that we fail to transform `DestroyedType` here. Can you try fixing this bug 
and see if that removes the need for your other changes?


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-04 Thread Akira Hatanaka via cfe-commits 
ahatanak added inline comments.


> rsmith wrote in SemaTemplateInstantiateDecl.cpp:4849
> Do we need to do this for conversion function names too? (Eg, `operator 
> C1*`)

I added a definition of "operator C1*" and called it in C1::foo1. I didn't 
see any crashes or asserts.

Calls to a destructor and a conversion function create different AST nodes (the 
former creates a MemberExpr and the latter creates a 
CXXDependentScopeMemberExpr) and are transformed by different functions of 
TreeTransform.

https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-04 Thread Richard Smith via cfe-commits 
rsmith added inline comments.


> SemaTemplateInstantiateDecl.cpp:4849
> +  DeclarationName Name = D->getDeclName();
> +  if (auto *DD = dyn_cast(D))
> +Name =

Do we need to do this for conversion function names too? (Eg, `operator C1*`)

https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-10-04 Thread Akira Hatanaka via cfe-commits 
ahatanak added a comment.

ping


https://reviews.llvm.org/D24969



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

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

2016-09-27 Thread Akira Hatanaka via cfe-commits 
ahatanak created this revision.
ahatanak added reviewers: doug.gregor, rsmith.
ahatanak added a subscriber: cfe-commits.

This fixes PR30361.

clang was failing to compile the test case because it was passing "~C1" 
instead of "~C1" to FindInstantiatedDecl and RebuildMemberExpr.

https://reviews.llvm.org/D24969

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/destructor.cpp

Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,21 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -2122,6 +2122,11 @@
NamedDecl *FirstQualifierInScope) {
 ExprResult BaseResult = getSema().PerformMemberExprBaseConversion(Base,
   isArrow);
+DeclarationNameInfo DNI = MemberNameInfo;
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
 if (!Member->getDeclName()) {
   // We have a reference to an unnamed field.  This is always the
   // base of an anonymous struct/union member access, i.e. the
@@ -2139,7 +2144,7 @@
   Base = BaseResult.get();
   ExprValueKind VK = isArrow ? VK_LValue : Base->getValueKind();
   MemberExpr *ME = new (getSema().Context)
-  MemberExpr(Base, isArrow, OpLoc, Member, MemberNameInfo,
+  MemberExpr(Base, isArrow, OpLoc, Member, DNI,
  cast(Member)->getType(), VK, OK_Ordinary);
   return ME;
 }
@@ -2152,7 +2157,7 @@
 
 // FIXME: this involves duplicating earlier analysis in a lot of
 // cases; we should avoid this when possible.
-LookupResult R(getSema(), MemberNameInfo, Sema::LookupMemberName);
+LookupResult R(getSema(), DNI, Sema::LookupMemberName);
 R.addDecl(FoundDecl);
 R.resolveKind();
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4845,7 +4845,11 @@
 
 NamedDecl *Result = nullptr;
 if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+  DeclarationName Name = D->getDeclName();
+  if (auto *DD = dyn_cast(D))
+Name =
+SubstDeclarationNameInfo(DD->getNameInfo(), 
TemplateArgs).getName();
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,


Index: test/SemaCXX/destructor.cpp
===
--- test/SemaCXX/destructor.cpp
+++ test/SemaCXX/destructor.cpp
@@ -431,3 +431,21 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -2122,6 +2122,11 @@
NamedDecl *FirstQualifierInScope) {
 ExprResult BaseResult = getSema().PerformMemberExprBaseConversion(Base,
   isArrow);
+DeclarationNameInfo DNI = MemberNameInfo;
+
+if (isa(FoundDecl))
+  DNI = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+
 if (!Member->getDeclName()) {
   // We have a reference to an unnamed field.  This is always the
   // base of an anonymous struct/union member access, i.e. the
@@ -2139,7 +2144,7 @@
   Base = BaseResult.get();
   ExprValueKind VK = isArrow ? VK_LValue : Base->getValueKind();
   MemberExpr *ME = new (getSema().Context)
-  MemberExpr(Base, isArrow, OpLoc, Member, MemberNameInfo,
+  MemberExpr(Base, isArrow, OpLoc, Member, DNI,
  cast(Member)->getType(), VK, OK_Ordinary);
   return ME;
 }
@@ -2152,7 +2157,7 @@
 
 // FIXME: this involves duplicating earlier analysis in a lot of
 // cases; we should avoid this when possible.
-LookupResult R(getSema(), MemberNameInfo, Sema::LookupMemberName);
+LookupResult R(getSema(), DNI, Sema::LookupMemberName);
 R.addDecl(FoundDecl);
 R.resolveKind();
 
Index: