[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-27 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

```c++
void CXXMethodDecl::addOverriddenMethod(const CXXMethodDecl *MD) {
  assert(MD->isCanonicalDecl() && "Method is not canonical!");
  assert(!MD->getParent()->isDependentContext() &&
 "Can't add an overridden method to a class template!");
```

It seems to me that the second assertion is not really a requirement. @mizvekov 
, can you please comment on this statement?

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-27 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

Such an example causes a failure in my patch

```c++
template  
class X {
public:
  X() = default;
  virtual ~X() = default;

  virtual int foo(int x, int y, T &entry) = 0;

  void bar() {
struct Processor : public X {
  Processor() : X() {}

  int foo(int, int, T &) override {
return 42;
  }
};
  }
};
```

The output 
```
llvm-project/clang/lib/AST/DeclCXX.cpp:2508: void 
clang::CXXMethodDecl::addOverriddenMethod(const CXXMethodDecl *): Assertion 
`!MD->getParent()->isDependentContext() && "Can't add an overridden method to a 
class template!"' failed.
```

The problem case is class definition nested to the method. 
I'm working on investigation and fixing this issue  

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Erich Keane via cfe-commits

erichkeane wrote:

Hmm, that compile time regression is unfortunate.  @vbe-sc as a part of your 
next version of this patch, can you do some sort of analysis why this would 
result in further lookups/instantiation/etc?  I could comprehend that perhaps 
we're skipping the 'current instantiation' now for these bases so now there is 
more work to do in instantiations (of which there are more of those than the 
base, sort of out of necessity), but would like some sort of 
analysis/confirmation that is the case/what is happening in some of those 
'worst' comparisons.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Nikita Popov via cfe-commits

nikic wrote:

FYI this seems to have significant cost for some types of C++ code, in 
particular clang build time regresses by 0.35% 
(https://llvm-compile-time-tracker.com/compare.php?from=4a7b56e6e7dd0f83c379ad06b6e81450bc691ba6&to=486644723038555a224fd09d462bb5099e64809e&stat=instructions:u).

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Anton Sidorenko via cfe-commits

asi-sc wrote:

@vbe-sc , reverted as you asked me offline 
https://github.com/llvm/llvm-project/pull/117727 . Feel free to create a new PR 
which includes the original commit and the fix for problem.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread LLVM Continuous Integration via cfe-commits

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `clangd-ubuntu-tsan` 
running on `clangd-ubuntu-clang` while building `clang` at step 6 
"test-build-clangd-clangd-index-server-clangd-in...".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/134/builds/9282


Here is the relevant piece of the build log for the reference

```
Step 6 (test-build-clangd-clangd-index-server-clangd-in...) failure: test 
(failure)
 TEST 'Clangd :: target_info.test' FAILED 

Exit Code: 66

Command Output (stderr):
--
RUN: at line 5: rm -rf 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
 && mkdir -p 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
+ rm -rf 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
+ mkdir -p 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir
RUN: at line 7: echo '[{"directory": 
"/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir",
 "command": 
"/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/armv7-clang
 -x c++ the-file.cpp -v", "file": "the-file.cpp"}]' > 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/compile_commands.json
+ echo '[{"directory": 
"/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir",
 "command": 
"/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir/armv7-clang
 -x c++ the-file.cpp -v", "file": "the-file.cpp"}]'
RUN: at line 9: sed -e 
"s|INPUT_DIR|/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir|g"
 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/llvm-project/clang-tools-extra/clangd/test/target_info.test
 > 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1
+ sed -e 
's|INPUT_DIR|/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.dir|g'
 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/llvm-project/clang-tools-extra/clangd/test/target_info.test
RUN: at line 12: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1
 > 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test
+ sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test.1
RUN: at line 14: clangd -lit-test < 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test
 2>&1 | /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/bin/FileCheck 
-strict-whitespace 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test
+ clangd -lit-test
+ /vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/bin/FileCheck 
-strict-whitespace 
/vol/worker/clangd-ubuntu-clang/clangd-ubuntu-tsan/build/tools/clang/tools/extra/clangd/test/Output/target_info.test.tmp.test

--




```



https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread via cfe-commits

https://github.com/cor3ntin closed 
https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Vladislav Belov via cfe-commits


@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers 
`_).
 
+- Clang now make correct name lookup when dependent base class is the current 
instantiation.

vbe-sc wrote:

Could we merge this? 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Vladislav Belov via cfe-commits


@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers 
`_).
 
+- Clang now make correct name lookup when dependent base class is the current 
instantiation.

vbe-sc wrote:

Fixed, thanks

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 6564e20ae7fc69849b6787afa12f68b712f4df12 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/docs/ReleaseNotes.rst  |  3 ++
 clang/lib/AST/CXXInheritance.cpp | 18 
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 clang/www/cxx_dr_status.html |  2 +-
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a8830a5658c7da..76ad2e2cf6b641 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers 
`_).
 
+- Fix name lookup for a dependent base class that is the current 
instantiation.  
+  (`CWG591: When a dependent base class is the current instantiation 
`_).
+
 C Language Changes
 --
 
diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..6d71cb206aa188 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -135,7 +135,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback 
BaseMatches) const {
 return false;
 
   CXXRecordDecl *Base =
-cast_or_null(Ty->getDecl()->getDefinition());
+  cast_if_present(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_if_present(
+  BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
  

[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread via cfe-commits

https://github.com/cor3ntin approved this pull request.

LGTM (modulo small wording nit in the release note)

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread via cfe-commits


@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers 
`_).
 
+- Clang now make correct name lookup when dependent base class is the current 
instantiation.

cor3ntin wrote:

```suggestion
- Fix name lookup for a dependent base class that is the current instantiation.
```

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-26 Thread via cfe-commits

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-24 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@cor3ntin could you, please, re-review this patch and resolve the comments if 
everything is ok?

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-21 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@sdkrystian, can you, please, re-review this patch? It seems like I fixed your 
test

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-19 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

> Can you add a release note

I added a release note as you asked

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-19 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From d17588af72f845b758a03637c84752dde733200c Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/docs/ReleaseNotes.rst  |  3 ++
 clang/lib/AST/CXXInheritance.cpp | 18 
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 clang/www/cxx_dr_status.html |  2 +-
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a8830a5658c7da..b9f06f2047539a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
   by default.
   (`CWG2521: User-defined literals and reserved identifiers 
`_).
 
+- Clang now make correct name lookup when dependent base class is the current 
instantiation.
+  (`CWG591: When a dependent base class is the current instantiation 
`_).
+
 C Language Changes
 --
 
diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..6d71cb206aa188 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -135,7 +135,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback 
BaseMatches) const {
 return false;
 
   CXXRecordDecl *Base =
-cast_or_null(Ty->getDecl()->getDefinition());
+  cast_if_present(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_if_present(
+  BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C:

[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread Vladislav Belov via cfe-commits


@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_or_null(

vbe-sc wrote:

Yes, you are right. Fixed here and in the other part of the code because there 
is the same case.



https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From e2e490717b1abe440556f2d8c439b3ee18b31642 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 18 
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 clang/www/cxx_dr_status.html |  2 +-
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..6d71cb206aa188 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -135,7 +135,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback 
BaseMatches) const {
 return false;
 
   CXXRecordDecl *Base =
-cast_or_null(Ty->getDecl()->getDefinition());
+  cast_if_present(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_if_present(
+  BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 186f7cc0ace546..c773c58fac4d0f 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -3599,7 +3599,7 @@ C++ defect report implementation 
status
 https://cplusplus.github.io/CWG/issues/591.html";>591
 CD4
 When a dependent base class is the current instantiation
-No
+Yes
   
   
 https://cplusplus.github.io/CWG/issues/592.html";>592

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.l

[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 2f7c7634ae22e53c5ffd645c5d544b36d108a25b Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 18 
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 clang/www/cxx_dr_status.html |  2 +-
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..590f3b0499d120 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -135,7 +135,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback 
BaseMatches) const {
 return false;
 
   CXXRecordDecl *Base =
-cast_or_null(Ty->getDecl()->getDefinition());
+  cast_if_present(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_or_null(
+  BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 186f7cc0ace546..c773c58fac4d0f 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -3599,7 +3599,7 @@ C++ defect report implementation 
status
 https://cplusplus.github.io/CWG/issues/591.html";>591
 CD4
 When a dependent base class is the current instantiation
-No
+Yes
   
   
 https://cplusplus.github.io/CWG/issues/592.html";>592

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm

[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread via cfe-commits

https://github.com/cor3ntin edited 
https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread via cfe-commits


@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_or_null(

cor3ntin wrote:

```suggestion
  if (auto *BaseRecord = cast_if_present(
```

Can this actually be null?

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-18 Thread via cfe-commits

https://github.com/cor3ntin commented:

Can you add a release note and update cxx_dr_status?
Thanks

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-17 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From f3b8ee95f1b3759f7d0dbc12f856a13ca9f01e0f Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 16 +++
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..3edc1f54837deb 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord =
+  cast(BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 5a48960ab73890b10381b4e7ebd32d3f66246bdd Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 16 +++
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..7961dbc0c7c8a3 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -170,13 +170,21 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = isa(BaseType);
+if (!isCurrentInstantiation) {
+  if (auto *BaseRecord = cast_or_null(
+  BaseSpec.getType()->getAsRecordDecl()))
+isCurrentInstantiation = BaseRecord->isDependentContext() &&
+ BaseRecord->isCurrentInstantiation(Record);
+}
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +252,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +272,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 11c24e5592bfdf8849bb415580d37a6f545e4952 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 14 ++
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..b2e77037c10c72 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -170,13 +170,19 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = false;
+if (auto *BaseRecord =
+cast(BaseSpec.getType()->getAsRecordDecl()))
+  isCurrentInstantiation = BaseRecord->isDependentContext() &&
+   BaseRecord->isCurrentInstantiation(Record);
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +250,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +270,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 11c24e5592bfdf8849bb415580d37a6f545e4952 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 14 ++
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..b2e77037c10c72 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -170,13 +170,19 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = false;
+if (auto *BaseRecord =
+cast(BaseSpec.getType()->getAsRecordDecl()))
+  isCurrentInstantiation = BaseRecord->isDependentContext() &&
+   BaseRecord->isCurrentInstantiation(Record);
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +250,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +270,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 0de2ff271efd672bae0b4974c404149da79ff747 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 14 ++
 clang/test/CXX/drs/cwg5xx.cpp| 48 ++--
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..b2e77037c10c72 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -170,13 +170,19 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
 
+bool isCurrentInstantiation = false;
+if (auto *BaseRecord =
+cast(BaseSpec.getType()->getAsRecordDecl()))
+  isCurrentInstantiation = BaseRecord->isDependentContext() &&
+   BaseRecord->isCurrentInstantiation(Record);
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
 //   if a base class of the class template depends on a template-parameter,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +250,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +270,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..0d53a9d07d76de 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,61 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
+};
+  };
+
+  template struct G {
+struct B {
+  typedef int M;
+  struct C {
+typedef void M;
+struct D;
+  };
+};
+  };
+
+  template struct H {
+template struct B {
+  typedef int M;
+  template struct C {
+typedef void M;
+struct D;
+struct P;
+  };
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct G::B::C::D : B {
+M m;
+  };
+
+  template
+  template
+  template
+  struct H::B::C::D : B {
+M m;
+  };
+
+  template struct A::B::D : A {
+M m;
+// expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
+  };
+
+  template
+  template
+  template
+  struct H::B::C::P : B {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

> I don't think this patch fixes the following case:
> 
> ```c++
> template 
> struct A 
> {
> struct B 
> {
> using X = int;
> 
> struct C
> {
> using X = void;
> 
> struct D;
> };
> };
> };
> 
> template 
> struct A::B::C::D : B
> {
> X x; // error: field has incomplete type 'X' (aka 'void')
> };
> ```
> 
> Regardless, this patch should probably include this as a test.

Many thanks for your answer! You're right, this test failed with this patch, 
but I've already fixed it (see the new changes).
Moreover, I have added more tests that should and shouldn't compile.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-16 Thread via cfe-commits

kolobabka wrote:

> I don't think this patch fixes the following case:
> 
> ```c++
> template 
> struct A 
> {
> struct B 
> {
> using X = int;
> 
> struct C
> {
> using X = void;
> 
> struct D;
> };
> };
> };
> 
> template 
> struct A::B::C::D : B
> {
> X x; // error: field has incomplete type 'X' (aka 'void')
> };
> ```
> 
> Regardless, this patch should probably include this as a test.

Many thanks for your answer! Yor're right, this test failed with this patch, 
but I've fixed it yet (see new changes). 

Moreover, I have added more tests to verify cases that should compile and those 
that shouldn't.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-15 Thread Krystian Stasiowski via cfe-commits

https://github.com/sdkrystian edited 
https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-15 Thread Krystian Stasiowski via cfe-commits

https://github.com/sdkrystian commented:

I don't think this patch fixes the following case:
```cpp
template 
struct A 
{
struct B 
{
using X = int;

struct C
{
using X = void;

struct D;
};
};
};

template 
struct A::B::C::D : B
{
X x; // error: field has incomplete type 'X' (aka 'void')
};
```
Regardless, this patch should probably this as a test.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-15 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

> Please wait for a week before pinging someone again.
> 
> I think this is fine to merge as is, if you want to go ahead. We can always 
> do post commit review later.

Sorry, I didn't know.
Let's do it this way you want 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-15 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

Please wait for a week before pinging someone again.

I think this is fine to merge as is, if you want to go ahead. We can always do 
post commit review later.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-14 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@sdkrystian, could you, please, take a look? We are about to merge this 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-12 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@sdkrystian ping

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-11 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

> @vbe-sc I can later today or tomorrow.

That's would be great, thank you!

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-11 Thread Krystian Stasiowski via cfe-commits

sdkrystian wrote:

@vbe-sc I can later today or tomorrow.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-10 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@sdkrystian, could you please take a look? 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-08 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov approved this pull request.

LGTM, but give some time for @sdkrystian to take a look, since he is actively 
working on this area.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-08 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@sdkrystian, please, could you take a look?

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-07 Thread Vladislav Belov via cfe-commits


@@ -265,8 +268,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+if (auto *RT = BaseSpec.getType()->getAs())
+  BaseRecord = cast(RT->getDecl());

vbe-sc wrote:

This is almost same. To be correct, we need to replace the first variant with 
```c++
BaseRecord = cast(BaseSpec.getType()->getAsRecordDecl());
```

All in all, thanks for this comment. Fixed

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-07 Thread Vladislav Belov via cfe-commits


@@ -169,14 +169,18 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = false;
+if (auto *TST = BaseSpec.getType()->getAs())
+  isCurrentInstantiation = TST->isCurrentInstantiation();

vbe-sc wrote:

You are right, thanks.
I've accepted your changes 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-07 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 66f3465737a99a002310b707f7783698adef253a Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 10 +-
 clang/test/CXX/drs/cwg5xx.cpp|  8 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..9b1bcaa88e129e 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -169,6 +169,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = isa(BaseType);
 
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
@@ -176,7 +177,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +246,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +266,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+BaseRecord = 
cast(BaseSpec.getType()->getAsRecordDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..b283684aef2f7e 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,21 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct A::B::D : A {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Matheus Izvekov via cfe-commits


@@ -169,14 +169,18 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = false;
+if (auto *TST = BaseSpec.getType()->getAs())
+  isCurrentInstantiation = TST->isCurrentInstantiation();

mizvekov wrote:

This relies on a TemplateSpecializationType which is a type sugar to make this 
determination, and type sugar could be lost, for example with template 
instantiation.

Might be worth double checking and adding a test case where the base type is 
formed from instantiation.

This test would be equivalent to checking if the type is canonically an 
InjectedClassNameType:
```suggestion
bool isCurrentInstantiation = isa(BaseType);
```
This is reliable as it only depends on the canonical type.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Matheus Izvekov via cfe-commits


@@ -265,8 +268,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+if (auto *RT = BaseSpec.getType()->getAs())
+  BaseRecord = cast(RT->getDecl());

mizvekov wrote:

This should be equivalent to:
```suggestion
BaseRecord = BaseSpec.getType()->getAsRecordDecl();
```

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> > > @Endilll, please, could you take a look?
> > 
> > 
> > My knowledge of wording about templates is not deep enough, unfortunately.
> 
> Could you, please, suggest someone who can review this part?

I've already added several people who should be able to, but mind that we're 
very short on reviewers, so this will likely take time.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

> > @Endilll, please, could you take a look?
> 
> My knowledge of wording about templates is not deep enough, unfortunately.

Could you, please, suggest someone who can review this part?

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> @Endilll, please, could you take a look?

My knowledge of wording about templates is not deep enough, unfortunately.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

> Also, do you have any ideas why the Win pipeline fails with checkout failure? 
> How can I fix it?

It seems to be unrelated to you. Might be fixed when you'll push a new commit.

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-06 Thread Vladislav Belov via cfe-commits

vbe-sc wrote:

@Endilll, please, could you take a look? 
Also, do you have any ideas why the Win pipeline fails with checkout failure? 
How can I fix it? 

https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-05 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc updated 
https://github.com/llvm/llvm-project/pull/114978

>From 0c2363cb2ab24d0b4d9998943772f9105437579e Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 13 -
 clang/test/CXX/drs/cwg5xx.cpp|  8 ++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..049532f942d051 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -169,6 +169,9 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = false;
+if (auto *TST = BaseSpec.getType()->getAs())
+  isCurrentInstantiation = TST->isCurrentInstantiation();
 
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
@@ -176,7 +179,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +248,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +268,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+if (auto *RT = BaseSpec.getType()->getAs())
+  BaseRecord = cast(RT->getDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..b283684aef2f7e 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,21 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct A::B::D : A {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-05 Thread Vladislav Belov via cfe-commits

https://github.com/vbe-sc created 
https://github.com/llvm/llvm-project/pull/114978

Currently the following example is a compilation failure: 
```
template struct A {
typedef int M;
struct B {
  typedef void M;
  struct C;
};
};

template struct A::B::C : A {
M m; // void or int ?
};
```

According to the point 13.8.3.2

```
A dependent base class is a base class that is a dependent type and is not the 
current instantiation.
Note 2 : A base class can be the current instantiation in the case of a nested 
class naming an enclosing class as a base.
```

The base class `A` is the current instantiation, because `C` is a nested class 
for an enclosing class `A`, it's is the not-dependent base class and we need 
to search the names through its scope.

This patch makes this example compile

>From 4eb7be671dee117f8185423b177c014edd310b48 Mon Sep 17 00:00:00 2001
From: vb-sc 
Date: Tue, 5 Nov 2024 15:46:57 +0300
Subject: [PATCH] [clang] Fix name lookup for dependent bases

---
 clang/lib/AST/CXXInheritance.cpp | 13 -
 clang/test/CXX/drs/cwg5xx.cpp|  8 ++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..049532f942d051 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -169,6 +169,9 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = false;
+if (auto *TST = BaseSpec.getType()->getAs())
+  isCurrentInstantiation = TST->isCurrentInstantiation();
 
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
@@ -176,7 +179,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +248,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +268,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+if (auto *RT = BaseSpec.getType()->getAs())
+  BaseRecord = cast(RT->getDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..b283684aef2f7e 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,21 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct A::B::D : A {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

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


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-05 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll edited 
https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Fix name lookup for dependent bases (PR #114978)

2024-11-05 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Vladislav Belov (vbe-sc)


Changes

Currently the following example is a compilation failure: 
```
template struct A {
typedef int M;
struct B {
  typedef void M;
  struct C;
};
};

template struct A::B::C : A {
M m; // void or int ?
};
```

According to the point 13.8.3.2

```
A dependent base class is a base class that is a dependent type and is not the 
current instantiation.
Note 2 : A base class can be the current instantiation in the case of a nested 
class naming an enclosing class as a base.
```

The base class `A` is the current instantiation, because `C` is a nested class 
for an enclosing class `A`, it's is the not-dependent base class and 
we need to search the names through its scope.

This patch makes this example compile

---
Full diff: https://github.com/llvm/llvm-project/pull/114978.diff


2 Files Affected:

- (modified) clang/lib/AST/CXXInheritance.cpp (+8-5) 
- (modified) clang/test/CXX/drs/cwg5xx.cpp (+6-2) 


``diff
diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp
index eb265a872c1259..049532f942d051 100644
--- a/clang/lib/AST/CXXInheritance.cpp
+++ b/clang/lib/AST/CXXInheritance.cpp
@@ -169,6 +169,9 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 // Find the record of the base class subobjects for this type.
 QualType BaseType =
 Context.getCanonicalType(BaseSpec.getType()).getUnqualifiedType();
+bool isCurrentInstantiation = false;
+if (auto *TST = BaseSpec.getType()->getAs())
+  isCurrentInstantiation = TST->isCurrentInstantiation();
 
 // C++ [temp.dep]p3:
 //   In the definition of a class template or a member of a class template,
@@ -176,7 +179,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 //   the base class scope is not examined during unqualified name lookup
 //   either at the point of definition of the class template or member or
 //   during an instantiation of the class tem- plate or member.
-if (!LookupInDependent && BaseType->isDependentType())
+if (!LookupInDependent &&
+(BaseType->isDependentType() && !isCurrentInstantiation))
   continue;
 
 // Determine whether we need to visit this base class at all,
@@ -244,9 +248,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 return FoundPath;
   }
 } else if (VisitBase) {
-  CXXRecordDecl *BaseRecord;
+  CXXRecordDecl *BaseRecord = nullptr;
   if (LookupInDependent) {
-BaseRecord = nullptr;
 const TemplateSpecializationType *TST =
 BaseSpec.getType()->getAs();
 if (!TST) {
@@ -265,8 +268,8 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 BaseRecord = nullptr;
 }
   } else {
-BaseRecord = cast(
-BaseSpec.getType()->castAs()->getDecl());
+if (auto *RT = BaseSpec.getType()->getAs())
+  BaseRecord = cast(RT->getDecl());
   }
   if (BaseRecord &&
   lookupInBases(Context, BaseRecord, BaseMatches, LookupInDependent)) {
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index ed0c7159dfc889..b283684aef2f7e 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1178,17 +1178,21 @@ namespace cwg590 { // cwg590: yes
   template typename A::B::C A::B::C::f(A::B::C) {}
 }
 
-namespace cwg591 { // cwg591: no
+namespace cwg591 { // cwg591: yes
   template struct A {
 typedef int M;
 struct B {
   typedef void M;
   struct C;
+  struct D;
 };
   };
 
   template struct A::B::C : A {
-// FIXME: Should find member of non-dependent base class A.
+M m;
+  };
+
+  template struct A::B::D : A {
 M m;
 // expected-error@-1 {{field has incomplete type 'M' (aka 'void'}}
   };

``




https://github.com/llvm/llvm-project/pull/114978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits