[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

The issue is up: https://github.com/llvm/llvm-project/issues/80252

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

[clang] Users/mizvekov/bug/clang merge usingshadowdecl (PR #80245)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/80245

[clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit 
fd8634a09de71.

This problem could manifest itself as ODR check false positives when importing 
modules.

Fixes: #ISSUE_TO_BE_CREATED

>From 23a0d731cfe8593c338fc8ad7920f7aa9270afaa Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:07:16 -0300
Subject: [PATCH 1/2] [NFC] [clang] add test for merging of UsingShadowDecl

---
 clang/test/Modules/cxx20-decls.cppm | 44 +
 1 file changed, 44 insertions(+)
 create mode 100644 clang/test/Modules/cxx20-decls.cppm

diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
new file mode 100644
index 0..052c8e73be247
--- /dev/null
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_REDECL_ADDR:[^ ]*]] prev 0x[[BAZ_ADDR:[^ 
]*]] <{{.*}}> line:{{.*}} imported in A. hidden  baz
+// CHECK-NEXT: |-original Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_REDECL_ADDR:[^ ]*]] prev 
0x[[ALIAS_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} imported in A. hidden foo 
'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x{{[^ ]*}} first 0x[[USING_ADDR:[^ ]*]] <{{.*}}> 
col:{{.*}} imported in A. hidden baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_REDECL_ADDR]] 'baz'
+// FIXME: UsingShadowDecl should have been merged
+// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} <{{.*}}> 
col:{{.*}} imported in A. hidden implicit TypeAlias 
0x[[ALIAS_REDECL_ADDR]] 'foo'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} <{{.*}}> col:{{.*}} imported in 
A. hidden implicit TypeAlias 0x[[ALIAS_REDECL_ADDR]] 'foo'
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_ADDR]] <{{.*}}> line:{{.*}} baz
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_ADDR]] <{{.*}}> col:{{.*}} referenced 
foo 'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x[[USING_ADDR]] <{{.*}}> col:{{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT:  `-UsingShadowDecl 0x[[SHADOW_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} 
implicit TypeAlias 0x[[ALIAS_ADDR]] 'foo'

>From b44f1b2ee3d709c627c1679fe3a252ea564bdd3d Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:26:10 -0300
Subject: [PATCH 2/2] [clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit
fd8634a09de71.

This problem could manifest itself as ODR check false positives
when importing modules.

Fixes: #ISSUE_TO_BE_CREATED
---
 clang/docs/ReleaseNotes.rst | 3 +++
 clang/lib/AST/ASTContext.cpp| 2 +-
 clang/test/Modules/cxx20-decls.cppm | 6 ++
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b7b11ab3a6b2e..aa14b8e931101 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -181,6 +181,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix incorrect code generation caused by the object argument of ``static 
operator()`` and ``static operator[]`` calls not being evaluated.
   Fixes (`#67976 `_)
+- Fix incorrect merging of modules which contain using declarations which 
shadow
+  other declarations. This could manifest as ODR checker false positives.
+  Fixes (`#TODO `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 71c9c0003d18c..feeb350bf3e0c 100644
--- 

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

Hmm I have been doing user branches for a while, this saves me time in case I 
want to split the patch off in a separate MR.
It's already the case that this is split into test adding + fixing commits.

In any case, the user branch will be deleted as soon as this is merged.

Can we go ahead, and clarify the rules for the next time, since this is such a 
small patch and windows CI takes 3 hours?

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/80245

>From 23a0d731cfe8593c338fc8ad7920f7aa9270afaa Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:07:16 -0300
Subject: [PATCH 1/2] [NFC] [clang] add test for merging of UsingShadowDecl

---
 clang/test/Modules/cxx20-decls.cppm | 44 +
 1 file changed, 44 insertions(+)
 create mode 100644 clang/test/Modules/cxx20-decls.cppm

diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
new file mode 100644
index 0..052c8e73be247
--- /dev/null
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_REDECL_ADDR:[^ ]*]] prev 0x[[BAZ_ADDR:[^ 
]*]] <{{.*}}> line:{{.*}} imported in A. hidden  baz
+// CHECK-NEXT: |-original Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_REDECL_ADDR:[^ ]*]] prev 
0x[[ALIAS_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} imported in A. hidden foo 
'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x{{[^ ]*}} first 0x[[USING_ADDR:[^ ]*]] <{{.*}}> 
col:{{.*}} imported in A. hidden baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_REDECL_ADDR]] 'baz'
+// FIXME: UsingShadowDecl should have been merged
+// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} <{{.*}}> 
col:{{.*}} imported in A. hidden implicit TypeAlias 
0x[[ALIAS_REDECL_ADDR]] 'foo'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} <{{.*}}> col:{{.*}} imported in 
A. hidden implicit TypeAlias 0x[[ALIAS_REDECL_ADDR]] 'foo'
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_ADDR]] <{{.*}}> line:{{.*}} baz
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_ADDR]] <{{.*}}> col:{{.*}} referenced 
foo 'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x[[USING_ADDR]] <{{.*}}> col:{{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT:  `-UsingShadowDecl 0x[[SHADOW_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} 
implicit TypeAlias 0x[[ALIAS_ADDR]] 'foo'

>From 895c615a354d540c4e4404fa728bfad2a962c7bf Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:26:10 -0300
Subject: [PATCH 2/2] [clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit
fd8634a09de71.

This problem could manifest itself as ODR check false positives
when importing modules.

Fixes: #80252
---
 clang/docs/ReleaseNotes.rst | 3 +++
 clang/test/Modules/cxx20-decls.cppm | 6 ++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b7b11ab3a6b2e..a48672951de32 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -181,6 +181,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix incorrect code generation caused by the object argument of ``static 
operator()`` and ``static operator[]`` calls not being evaluated.
   Fixes (`#67976 `_)
+- Fix incorrect merging of modules which contain using declarations which 
shadow
+  other declarations. This could manifest as ODR checker false positives.
+  Fixes (`#80252 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
index 052c8e73be247..ee9f117278884 100644
--- a/clang/test/Modules/cxx20-decls.cppm
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -31,9 +31,7 @@ using xxx = baz::foo;
 // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
 // CHECK-NEXT: |-UsingDecl 0x{{[^ ]*}} first 0x[[USING_ADDR:[^ ]*]] <{{.*}}> 
col:{{.*}} imported in A. hidden baz::foo
 // CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_REDECL_ADDR]] 'baz'
-// FIXME: UsingShadowDecl should have been merged
-// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} <{{.*}}> 
col:{{.*}} imported in A. hidden 

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

@@ -181,6 +181,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix incorrect code generation caused by the object argument of ``static 
operator()`` and ``static operator[]`` calls not being evaluated.
   Fixes (`#67976 `_)
+- Fix incorrect merging of modules which contain using declarations which 
shadow
+  other declarations. This could manifest as ODR checker false positives.
+  Fixes (`#TODO `_)

mizvekov wrote:

The MR is split in two commits, the first one adds the test, if you click on 
the last commit, you can see the change in the expectation.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_REDECL_ADDR:[^ ]*]] prev 0x[[BAZ_ADDR:[^ 
]*]] <{{.*}}> line:{{.*}} imported in A. hidden  baz
+// CHECK-NEXT: |-original Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_REDECL_ADDR:[^ ]*]] prev 
0x[[ALIAS_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} imported in A. hidden foo 
'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x{{[^ ]*}} first 0x[[USING_ADDR:[^ ]*]] <{{.*}}> 
col:{{.*}} imported in A. hidden baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_REDECL_ADDR]] 'baz'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} prev 0x[[SHADOW_ADDR:[^ ]*]] 
<{{.*}}> col:{{.*}} imported in A. hidden implicit TypeAlias 
0x[[ALIAS_REDECL_ADDR]] 'foo'

mizvekov wrote:

While normally I would agree, in this case there are no existing AST tests for 
these other kinds of declarations, so I felt appropriate to broaden this test a 
little bit to cover those.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> IIRC, in #79959, you're saying the patch have the potential to fix #78850? Is 
> it true?

Yeah, looking at the similarities, there is a decent chance this is the same 
issue, it's worth trying it out.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

@@ -181,6 +181,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix incorrect code generation caused by the object argument of ``static 
operator()`` and ``static operator[]`` calls not being evaluated.
   Fixes (`#67976 `_)
+- Fix incorrect merging of modules which contain using declarations which 
shadow
+  other declarations. This could manifest as ODR checker false positives.
+  Fixes (`#TODO `_)

mizvekov wrote:

We usually do small fixups like this during review / post approval, this will 
not be merged like this, don't worry.
People on the LLVM community tend to be quite flexible on these things.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

I have rebased on top of the change which reenables GMF ODR checker on the test 
suite, so I added a new regression test that exercises the false positive ODR 
violation.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/80245

>From 8823a7ec248c7220395f53634467aa25f5007206 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:07:16 -0300
Subject: [PATCH 1/2] [NFC] [clang] add tests for merging of UsingShadowDecl

---
 clang/test/Modules/GH80252.cppm | 42 +++
 clang/test/Modules/cxx20-decls.cppm | 44 +
 2 files changed, 86 insertions(+)
 create mode 100644 clang/test/Modules/GH80252.cppm
 create mode 100644 clang/test/Modules/cxx20-decls.cppm

diff --git a/clang/test/Modules/GH80252.cppm b/clang/test/Modules/GH80252.cppm
new file mode 100644
index 0..3212404fbe14c
--- /dev/null
+++ b/clang/test/Modules/GH80252.cppm
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cppm -emit-module-interface -o 
%t/B.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/C.cpp -fmodule-file=A=%t/A.pcm 
-fmodule-file=B=%t/B.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- bar.h
+class bar {
+  bar(baz::foo);
+};
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+#include "bar.h"
+export module B;
+
+//--- C.cpp
+#include "foo.h"
+import A;
+#include "bar.h"
+import B;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = bar;
+// FIXME: This is a false positive ODR violation.
+// expected-error@bar.h:2 {{'bar' has different definitions in different 
modules; first difference is defined here found constructor with 1st parameter 
of type 'baz::foo' (aka 'char')}}
+// expected-note@bar.h:2 {{but in 'B.' found constructor with 1st 
parameter of type 'baz::foo' (aka 'char')}}
diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
new file mode 100644
index 0..052c8e73be247
--- /dev/null
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_REDECL_ADDR:[^ ]*]] prev 0x[[BAZ_ADDR:[^ 
]*]] <{{.*}}> line:{{.*}} imported in A. hidden  baz
+// CHECK-NEXT: |-original Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_REDECL_ADDR:[^ ]*]] prev 
0x[[ALIAS_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} imported in A. hidden foo 
'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x{{[^ ]*}} first 0x[[USING_ADDR:[^ ]*]] <{{.*}}> 
col:{{.*}} imported in A. hidden baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_REDECL_ADDR]] 'baz'
+// FIXME: UsingShadowDecl should have been merged
+// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} <{{.*}}> 
col:{{.*}} imported in A. hidden implicit TypeAlias 
0x[[ALIAS_REDECL_ADDR]] 'foo'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} <{{.*}}> col:{{.*}} imported in 
A. hidden implicit TypeAlias 0x[[ALIAS_REDECL_ADDR]] 'foo'
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl 0x[[BAZ_ADDR]] <{{.*}}> line:{{.*}} baz
+// CHECK-NEXT: |-TypeAliasDecl 0x[[ALIAS_ADDR]] <{{.*}}> col:{{.*}} referenced 
foo 'char'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'char'
+// CHECK-NEXT: |-UsingDecl 0x[[USING_ADDR]] <{{.*}}> col:{{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace 0x[[BAZ_ADDR]] 'baz'
+// CHECK-NEXT:  `-UsingShadowDecl 0x[[SHADOW_ADDR:[^ ]*]] <{{.*}}> col:{{.*}} 
implicit TypeAlias 0x[[ALIAS_ADDR]] 'foo'

>From f2e37f079cecdd95ba32912aaea739be68959f9e Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:26:10 -0300
Subject: [PATCH 2/2] [clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit
fd8634a09de71.

This problem could manifest itself as ODR check false positives
when importing modules.

Fixes: #80252
---
 clang/docs/ReleaseNotes.rst | 3 +++
 clang/lib/AST/ASTContext.cpp  

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/80245

>From f7340f3781a3e3094169c970aadb9b69414543f5 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:07:16 -0300
Subject: [PATCH 1/2] [NFC] [clang] add tests for merging of UsingShadowDecl

---
 clang/test/Modules/GH80252.cppm | 42 +++
 clang/test/Modules/cxx20-decls.cppm | 44 +
 2 files changed, 86 insertions(+)
 create mode 100644 clang/test/Modules/GH80252.cppm
 create mode 100644 clang/test/Modules/cxx20-decls.cppm

diff --git a/clang/test/Modules/GH80252.cppm b/clang/test/Modules/GH80252.cppm
new file mode 100644
index 0..3212404fbe14c
--- /dev/null
+++ b/clang/test/Modules/GH80252.cppm
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cppm -emit-module-interface -o 
%t/B.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/C.cpp -fmodule-file=A=%t/A.pcm 
-fmodule-file=B=%t/B.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- bar.h
+class bar {
+  bar(baz::foo);
+};
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+#include "bar.h"
+export module B;
+
+//--- C.cpp
+#include "foo.h"
+import A;
+#include "bar.h"
+import B;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = bar;
+// FIXME: This is a false positive ODR violation.
+// expected-error@bar.h:2 {{'bar' has different definitions in different 
modules; first difference is defined here found constructor with 1st parameter 
of type 'baz::foo' (aka 'char')}}
+// expected-note@bar.h:2 {{but in 'B.' found constructor with 1st 
parameter of type 'baz::foo' (aka 'char')}}
diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
new file mode 100644
index 0..28af7b4bc47b4
--- /dev/null
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl {{.*}} imported in A. {{.*}} baz
+// CHECK-NEXT: |-original Namespace {{.*}} 'baz'
+// CHECK-NEXT: |-TypeAliasDecl {{.*}} foo 'char'
+// CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+// CHECK-NEXT: |-UsingDecl {{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace {{.*}} 'baz'
+// FIXME: UsingShadowDecl should have been merged
+// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} {{.*}} imported 
in A. {{.*}} 'foo'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} {{.*}} imported in A. 
{{.*}} 'foo'
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl {{.*}} baz
+// CHECK-NEXT: |-TypeAliasDecl {{.*}} foo 'char'
+// CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+// CHECK-NEXT: |-UsingDecl {{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace {{.*}} 'baz'
+// CHECK-NEXT:  `-UsingShadowDecl 0x[[SHADOW_ADDR:[^ ]*]] {{.*}} 'foo'

>From 1e6827281750a7fe33b8fa1bc66524b6f52d9080 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:26:10 -0300
Subject: [PATCH 2/2] [clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit
fd8634a09de71.

This problem could manifest itself as ODR check false positives
when importing modules.

Fixes: #80252
---
 clang/docs/ReleaseNotes.rst | 3 +++
 clang/lib/AST/ASTContext.cpp| 2 +-
 clang/test/Modules/GH80252.cppm | 4 +---
 clang/test/Modules/cxx20-decls.cppm | 6 ++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 53040aa0f9074..0a9cd242f9d95 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix incorrect code generation caused by the object argument of ``static 
operator()`` 

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

I took a look at what sort of complexity unittest would entail here. I don't 
think it's a good compromise complexity wise, it's a lot of boilerplate just to 
test a few AST nodes are correctly linked.

On the other hand, these AST tests don't look particularly out of place 
compared to a lot of other AST tests we have.

However, I have seen that there are a lot of other related merging issues, so I 
will leave the tangentially related tests for another MR.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

I just pushed the change on this patch. What I meant is this patch will get, 
besides aforementioned regression test involving ODR checker failure, a simpler 
AST test that verifies just the UsingShadowDecl is correctly merged.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cppm -emit-module-interface -o 
%t/B.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/C.cpp -fmodule-file=A=%t/A.pcm 
-fmodule-file=B=%t/B.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- bar.h
+class bar {
+  bar(baz::foo);
+};
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;

mizvekov wrote:

Done, slightly different, to keep test simple and avoid introducing more 
shadowing declarations.

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

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/80245

>From 7bd920a14240c85442467706236e0644f320aa5c Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:07:16 -0300
Subject: [PATCH 1/2] [NFC] [clang] add tests for merging of UsingShadowDecl

---
 clang/test/Modules/GH80252.cppm | 45 +
 clang/test/Modules/cxx20-decls.cppm | 44 
 2 files changed, 89 insertions(+)
 create mode 100644 clang/test/Modules/GH80252.cppm
 create mode 100644 clang/test/Modules/cxx20-decls.cppm

diff --git a/clang/test/Modules/GH80252.cppm b/clang/test/Modules/GH80252.cppm
new file mode 100644
index 0..f4730a9874197
--- /dev/null
+++ b/clang/test/Modules/GH80252.cppm
@@ -0,0 +1,45 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cppm -emit-module-interface -o 
%t/B.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/C.cpp -fmodule-file=A=%t/A.pcm 
-fmodule-file=B=%t/B.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- bar.h
+class bar {
+  bar(baz::foo);
+};
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+export using AX = baz::foo;
+
+//--- B.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+#include "bar.h"
+export module B;
+export using BX = baz::foo;
+export using BY = bar;
+
+//--- C.cpp
+#include "foo.h"
+import A;
+#include "bar.h"
+import B;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = bar;
+// FIXME: This is a false positive ODR violation.
+// expected-error@bar.h:2 {{'bar' has different definitions in different 
modules; first difference is defined here found constructor with 1st parameter 
of type 'baz::foo' (aka 'char')}}
+// expected-note@bar.h:2 {{but in 'B.' found constructor with 1st 
parameter of type 'baz::foo' (aka 'char')}}
diff --git a/clang/test/Modules/cxx20-decls.cppm 
b/clang/test/Modules/cxx20-decls.cppm
new file mode 100644
index 0..28af7b4bc47b4
--- /dev/null
+++ b/clang/test/Modules/cxx20-decls.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -I %t %t/A.cppm -emit-module-interface -o 
%t/A.pcm -verify
+// RUN: %clang_cc1 -std=c++20 -I %t %t/B.cpp -fmodule-file=A=%t/A.pcm 
-fsyntax-only -verify -ast-dump-all -ast-dump-filter baz | FileCheck %s
+
+//--- foo.h
+namespace baz {
+  using foo = char;
+  using baz::foo;
+}
+
+//--- A.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module A;
+
+//--- B.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+// Since modules are loaded lazily, force loading by performing a lookup.
+using xxx = baz::foo;
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl {{.*}} imported in A. {{.*}} baz
+// CHECK-NEXT: |-original Namespace {{.*}} 'baz'
+// CHECK-NEXT: |-TypeAliasDecl {{.*}} foo 'char'
+// CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+// CHECK-NEXT: |-UsingDecl {{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace {{.*}} 'baz'
+// FIXME: UsingShadowDecl should have been merged
+// CHECK-NOT:  `-UsingShadowDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}} {{.*}} imported 
in A. {{.*}} 'foo'
+// CHECK-NEXT: `-UsingShadowDecl 0x{{[^ ]*}} {{.*}} imported in A. 
{{.*}} 'foo'
+
+// CHECK-LABEL: Dumping baz:
+// CHECK-NEXT: NamespaceDecl {{.*}} baz
+// CHECK-NEXT: |-TypeAliasDecl {{.*}} foo 'char'
+// CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+// CHECK-NEXT: |-UsingDecl {{.*}} baz::foo
+// CHECK-NEXT: | `-NestedNameSpecifier Namespace {{.*}} 'baz'
+// CHECK-NEXT:  `-UsingShadowDecl 0x[[SHADOW_ADDR:[^ ]*]] {{.*}} 'foo'

>From db966f66a148ab5b5ba26250cbf0dab6be5a2c29 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Thu, 1 Feb 2024 02:26:10 -0300
Subject: [PATCH 2/2] [clang] fix merging of UsingShadowDecl

Previously, when deciding if two UsingShadowDecls where mergeable,
we would incorrectly only look for both pointing to the exact redecla
ration, whereas the correct thing is to look for declarations to the
same entity.

This problem has existed as far back as 2013, introduced in commit
fd8634a09de71.

This problem could manifest itself as ODR check false positives
when importing modules.

Fixes: #80252
---
 clang/docs/ReleaseNotes.rst | 3 +++
 clang/lib/AST/ASTContext.cpp| 2 +-
 clang/test/Modules/GH80252.cppm | 4 +---
 clang/test/Modules/cxx20-decls.cppm | 6 ++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 53040aa0f9074..0a9cd242f9d95 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,9 @@ Bug Fixes to C++ Support
   and (`#79745 `_)
 - Fix 

[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

There is a lot of activity going on just in `clang/test/AST`, with multiple 
commits a day.
I think there is a time and place, and the right tradeoffs for all kinds of 
tests we have. Shall we get others opinions?
Shall we make an RFC so we get the community aligned on this new direction?

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov commented:

On the Implementation side, this looks good, just some concerns regarding 
documentation.

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

@@ -457,6 +457,28 @@ Note that **currently** the compiler doesn't consider 
inconsistent macro definit
 Currently Clang would accept the above example. But it may produce surprising 
results if the
 debugging code depends on consistent use of ``NDEBUG`` also in other 
translation units.
 
+Definitions consistency
+^^^
+
+The C++ language defines that same declarations in different translation units 
should have
+the same definition, as known as ODR (One Definition Rule). Prior to modules, 
the translation
+units don't dependent on each other and the compiler itself don't and can't 
perform a strong
+ODR violation check. Sometimes it is the linker does some jobs related to ODR, 
where the
+higher level semantics are missing. With the introduction of modules, now the 
compiler have
+the chance to perform ODR violations with language semantics across 
translation units.

mizvekov wrote:

```suggestion
The C++ language defines that same declarations in different translation units 
should have
the same definition, as known as ODR (One Definition Rule). Prior to modules, 
the translation
units don't dependent on each other and the compiler itself can't perform a 
strong
ODR violation check. With the introduction of modules, now the compiler have
the chance to perform ODR violations with language semantics across translation 
units.
```

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

@@ -3940,6 +3940,14 @@ static bool RenderModulesOptions(Compilation , const 
Driver ,
 Args.ClaimAllArgs(options::OPT_fmodules_disable_diagnostic_validation);
   }
 
+  // Don't check ODR violations for decls in the global module fragment.
+  // 1. To keep consistent behavior with MSVC, which don't check ODR violations
+  //in the global module fragment too.
+  // 2. Give users better using experience since most issue reports complains
+  //the false positive ODR violations diagnostic and the true positive ODR
+  //violations are rarely reported.
+  CmdArgs.push_back("-fskip-odr-check-in-gmf");

mizvekov wrote:

```suggestion
  // FIXME: We provisionally don't check ODR violations for decls in the global 
module fragment.
  CmdArgs.push_back("-fskip-odr-check-in-gmf");
```

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

@@ -457,6 +457,28 @@ Note that **currently** the compiler doesn't consider 
inconsistent macro definit
 Currently Clang would accept the above example. But it may produce surprising 
results if the
 debugging code depends on consistent use of ``NDEBUG`` also in other 
translation units.
 
+Definitions consistency
+^^^
+
+The C++ language defines that same declarations in different translation units 
should have
+the same definition, as known as ODR (One Definition Rule). Prior to modules, 
the translation
+units don't dependent on each other and the compiler itself don't and can't 
perform a strong
+ODR violation check. Sometimes it is the linker does some jobs related to ODR, 
where the
+higher level semantics are missing. With the introduction of modules, now the 
compiler have
+the chance to perform ODR violations with language semantics across 
translation units.
+
+However, in the practice we found the existing ODR checking mechanism may be 
too aggressive.
+In the many issue reports about ODR violation diagnostics, most of them are 
false positive
+ODR violations and the true positive ODR violations are rarely reported. Also 
MSVC don't
+perform ODR check for declarations in the global module fragment.

mizvekov wrote:

I think there are many kinds of issues here and the text might not be conveying 
the right idea.

1) There are some clang bugs where wrong semantic analysis occurs when building 
AST from pcm, where otherwise parsing from source produces correct results. 
This is not a problem in ODR checking per se, but it can manifest itself as 
that, and this ends up being helpful.
2) Bugs in the ODR checker itself, where two identical definitions, per the 
standard, are mistaken as different, or the opposite.
3) ODR violations in user code, which can some times be mostly harmless, and 
some times not.

I read this paragraph as talking about 3 in particular, and giving the idea 
that the ODR, as specified, ends up barfing at too many harmless violations, 
and we are finding that it's more trouble than it's worth as we enforce it in 
more situations.

Regarding MSVC, I don't think we normally would relax standards conformance so 
broadly in order to be consistent with another compiler. For example, see how 
we handle compatibility with MSVC regarding delayed template parsing.



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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

@@ -457,6 +457,28 @@ Note that **currently** the compiler doesn't consider 
inconsistent macro definit
 Currently Clang would accept the above example. But it may produce surprising 
results if the
 debugging code depends on consistent use of ``NDEBUG`` also in other 
translation units.
 
+Definitions consistency
+^^^
+
+The C++ language defines that same declarations in different translation units 
should have
+the same definition, as known as ODR (One Definition Rule). Prior to modules, 
the translation
+units don't dependent on each other and the compiler itself don't and can't 
perform a strong
+ODR violation check. Sometimes it is the linker does some jobs related to ODR, 
where the
+higher level semantics are missing. With the introduction of modules, now the 
compiler have
+the chance to perform ODR violations with language semantics across 
translation units.
+
+However, in the practice we found the existing ODR checking mechanism may be 
too aggressive.
+In the many issue reports about ODR violation diagnostics, most of them are 
false positive
+ODR violations and the true positive ODR violations are rarely reported. Also 
MSVC don't
+perform ODR check for declarations in the global module fragment.
+
+So in order to get better user experience, save the time checking ODR and keep 
consistent
+behavior with MSVC, we disabled the ODR check for the declarations in the 
global module
+fragment by default. Users who want more strict check can still use the
+``-Xclang -fno-skip-odr-check-in-gmf`` flag to get the ODR check enabled. It 
is also
+encouraged to report issues if users find false positive ODR violations or 
false negative ODR
+violations with the flag enabled.

mizvekov wrote:

Again I find this paragraph concerning 3). This jusitifies in terms of MSVC 
conformance, but doesn't limit the behavior to that environment. Also I don't 
think we offer compiler switches that decrease standards conformance in order 
to save build time.

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

[clang] [C++20] [Modules] Introduce -fskip-odr-check-in-gmf (PR #79959)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

@dwblaikie The flag is for our own use within LLVM test suite. With the 
previous change, we were unconditionally weakening standards conformance and 
runtime checking, because there are a bunch of bugs which are believed to be 
mostly harmless. However, if we do that without some sort of switch, we can't 
keep testing and fixing things on our end, so that we can some day revert to 
the correct behavior.

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

[clang] [clang][Sema] Improve error recovery for id-expressions referencing invalid decls (PR #81662)

[Matheus Izvekov via cfe-commits]

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

Minor nit, otherwise LGTM.

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

[clang] [clang][Sema] Improve error recovery for id-expressions referencing invalid decls (PR #81662)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang][Sema] Improve error recovery for id-expressions referencing invalid decls (PR #81662)

[Matheus Izvekov via cfe-commits]

@@ -3453,6 +3453,10 @@ ExprResult Sema::BuildDeclarationNameExpr(const 
CXXScopeSpec ,
NeedsADL, R.isOverloadedResult(),
R.begin(), R.end());
 
+  if (ULE && R.isSingleResult() && R.getFoundDecl()->isInvalidDecl()) {

mizvekov wrote:

```suggestion
  if (R.isSingleResult() && R.getFoundDecl()->isInvalidDecl()) {
```
AST Node `Create*` functions normally can't fail.

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

[clang] [clang][Sema] Improve error recovery for id-expressions referencing invalid decls (PR #81662)

[Matheus Izvekov via cfe-commits]

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov commented:

I think it would be a good idea to double check this for performance 
regressions, since this case will recurse into the function every time with 
this patch.
Though I don't know if there is a better way to test it than to merge it and 
wait for it to be picked up by google.

Another idea to avoid any potential regressions, is to mark the return type as 
deduced anyway, with the distinction that 'auto' was not written.

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

@@ -3649,19 +3649,15 @@ class IsTypeDeclaredInsideVisitor
 
 /// This function checks if the function has 'auto' return type that contains
 /// a reference (in any way) to a declaration inside the same function.

mizvekov wrote:

```suggestion
/// This function checks if the given function has a return type that contains
/// a reference (in any way) to a declaration inside the same function.
```

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

[clang] Improve stack usage to increase recursive initialization depth (PR #88546)

[Matheus Izvekov via cfe-commits]

@@ -1070,57 +1077,24 @@ class Sema;
 };
 
   private:
-SmallVector Candidates;
-llvm::SmallPtrSet Functions;
-
-// Allocator for ConversionSequenceLists. We store the first few of these
-// inline to avoid allocation for small sets.
-llvm::BumpPtrAllocator SlabAllocator;
+ASTContext 
+SmallVector Candidates;
+llvm::SmallPtrSet Functions;

mizvekov wrote:

I see, it's fine either way  

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

@@ -3647,15 +3647,28 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
-/// This function checks if the function has 'auto' return type that contains
+/// This function checks if the given function has a return type that contains
 /// a reference (in any way) to a declaration inside the same function.
-bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
+bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
   assert(FromFPT && "Must be called on FunctionProtoType");
 
+  auto IsCXX11LambdaWithouTrailingReturn = [&]() {
+if (!Importer.FromContext.getLangOpts().CPlusPlus11)

mizvekov wrote:

I think CPlusPlus11 is set for C++11 *and later*, but we want to return early 
here for C++14 onwards, as in that case lambda with deduced return type will 
have AutoType.
```suggestion
if (Importer.FromContext.getLangOpts().CPlusPlus14)
```

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

[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

@term-est It would be helpful if you could post the surrounding code context 
where this crashed, This is pointed at the top of the stacktrace:
```
0.  
/home/est/Working-Directory/DS/wetend-emulator-new/WetendEmulator.hpp:85:128: 
current parser token ')'
```

You said, and I confirm from the stack trace, it looks like this happened in a 
clangd invocation.
Does it also happen during compilation with clang?

Are you able to share prints and dumps from values surrounding the crash site?
Some examples:
```
NumNamedPacks
Info.getNumExplicitArgs()
Info.getDeducedDepth()
S.CurrentInstantiationScope != nullptr
PartialPackDepthIndex
Packs[...].Index
```



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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

@@ -13456,6 +13458,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
 return;
   }
 
+  if (VDecl->isInvalidDecl()) {
+CorrectDelayedTyposInExpr(Init, VDecl);
+ExprResult Recovery =
+CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init});
+if (Expr *E = Recovery.get())

mizvekov wrote:

Oops. I think this is still UB, please double check:
```suggestion
if (ExprResult Recovery =
 CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init}))
 VDecl->setInit(Recovery.get());
```

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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


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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

@@ -13435,16 +13435,18 @@ void Sema::checkNonTrivialCUnion(QualType QT, 
SourceLocation Loc,
 void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   // If there is no declaration, there was an error parsing it.  Just ignore
   // the initializer.
-  if (!RealDecl || RealDecl->isInvalidDecl()) {
+  if (!RealDecl) {
 CorrectDelayedTyposInExpr(Init, dyn_cast_or_null(RealDecl));
 return;
   }
 
   if (CXXMethodDecl *Method = dyn_cast(RealDecl)) {

mizvekov wrote:

Minor nit: We can use separate init-statement since we adopted C++17; You call.
```suggestion
  if (auto *Method = dyn_cast(RealDecl); 
!Method->isInvalidDecl()) {
```

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

LGTM, Thanks!

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

@@ -13456,6 +13458,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
 return;
   }
 
+  if (VDecl->isInvalidDecl()) {
+CorrectDelayedTyposInExpr(Init, VDecl);
+ExprResult Recovery =
+CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init});
+if (Expr *E = Recovery.get())

mizvekov wrote:

This is ok, I see that action result's get method will return a nullptr in 
unset/failed case anyway, and we don't need to make the distinction here.

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -8343,58 +8343,52 @@ bool 
Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   // C++1z [temp.arg.template]p3: (DR 150)
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
-  // FIXME: We should enable RelaxedTemplateTemplateArgs by default as it is a
-  //  defect report resolution from C++17 and shouldn't be introduced by
-  //  concepts.
-  if (getLangOpts().RelaxedTemplateTemplateArgs) {

mizvekov wrote:

Only the flag is deprecated, we want the new 'on' behavior to stay.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -8343,58 +8343,52 @@ bool 
Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   // C++1z [temp.arg.template]p3: (DR 150)
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
-  // FIXME: We should enable RelaxedTemplateTemplateArgs by default as it is a
-  //  defect report resolution from C++17 and shouldn't be introduced by
-  //  concepts.
-  if (getLangOpts().RelaxedTemplateTemplateArgs) {

mizvekov wrote:

The code following this removal contains the exact same code, but outside the 
if-block and de-indented.
It's a shame the GitHub diff viewer is not very good at showing this, phab was 
much better.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -8343,58 +8343,52 @@ bool 
Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   // C++1z [temp.arg.template]p3: (DR 150)
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
-  // FIXME: We should enable RelaxedTemplateTemplateArgs by default as it is a
-  //  defect report resolution from C++17 and shouldn't be introduced by
-  //  concepts.
-  if (getLangOpts().RelaxedTemplateTemplateArgs) {

mizvekov wrote:

It's a removal of the flag's behavior, with the addition of provisional wording 
changes, in order to moot the original reason the flag existed.

I see where the confusion comes from. The original patch did not remove the 
flag completely, just changed the default, but still deprecated it. After some 
offline discussion with @zygoloid, we decided to remove the flag as well, but I 
forgot to update the title.

@zygoloid thoughts on that?

How about for starters we just change the default, but still deprecate the flag 
both ways anyway?
If the transition is a concern, I think it would be a problem that we don't 
start to warn the users of the flags as soon as possible. Otherwise, I think 
this will be specially problematic if there are current users of the negative 
spelling `-fno-relaxed-template-template-args`.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/89807

This patch will finally allow us to mark C++17 support in clang as complete.

This is a continuation of the review process from an [old PR in 
phab](https://reviews.llvm.org/D109496).

Recap: The original patch from phab initially contained no workarounds / 
provisional resolutions for core defects,
Though testing by @yxsamliu [here](https://reviews.llvm.org/D109496#3101839) 
showed problems with some important libraries used in GPU applications, and we 
ended up reverting it.

In order to implement this as a DR and avoid breaking reasonable code that 
worked before P0522, this patch implements a provisional resolution for 
CWG2398: When deducing template template parameters against each other, and the 
argument side names a template specialization, instead of just deducing A, we 
instead deduce a synthesized template template parameter based on A, but with 
it's parameters using the template specialization's arguments as defaults.

This does not fix all possible regressions introduced by P0522, but it does 
seem to match in effect an undocumented workaround implemented by GCC. Though 
it's unconfirmed how closely we match, as I have not received official word 
regarding this yet.
Fixing other regressions will require more extensive changes, some of them 
would require possibly controversial wording changes, and so is left for future 
work.

The driver flag is deprecated with a warning, and it will not have any effect.

@yxsamliu Can you confirm this version avoids any breakages in your setup?

CC: @yuanfang-chen @MaskRay @chandlerc @jicama @lichray @AaronBallman 

>From 873043f6ea08b8828a5e061cf67c70e39b66b8b8 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] deprecate frelaxed-template-template-args, make it on
 by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning, and it will not have any effect.
---
 .../clang/Basic/DiagnosticDriverKinds.td  |  2 +-
 clang/include/clang/Basic/LangOptions.def |  1 -
 clang/include/clang/Driver/Options.td |  7 +-
 clang/lib/Driver/SanitizerArgs.cpp|  9 +-
 clang/lib/Driver/ToolChains/Clang.cpp | 11 +--
 clang/lib/Frontend/InitPreprocessor.cpp   |  4 +-
 clang/lib/Sema/SemaTemplate.cpp   | 88 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 99 +--
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |  2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |  4 +-
 .../frelaxed-template-template-args.cpp   |  5 +
 clang/test/Lexer/cxx-features.cpp |  5 +-
 clang/test/SemaTemplate/default-arguments.cpp |  7 +-
 .../instantiate-template-template-parm.cpp| 17 ++--
 clang/test/SemaTemplate/nested-template.cpp   |  8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |  6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |  2 +-
 clang/www/cxx_status.html | 17 ++--
 18 files changed, 183 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support '%0'">;
 def warn_drv_deprecated_arg : Warning<
-  "argument '%0' is deprecated, use '%1' instead">, InGroup;
+  "argument '%0' is deprecated%select{|, use '%2' instead}1">, 
InGroup;
 def warn_drv_deprecated_custom : Warning<
   "argument '%0' is deprecated, %1">, InGroup;
 def warn_drv_assuming_mfloat_abi_is : Warning<
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 8ef6700ecdc78e..2dfe70f29d7c85 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -158,7 +158,6 @@ LANGOPT(GNUAsm, 1, 1, "GNU-style inline 
assembly")
 LANGOPT(Coroutines, 1, 0, "C++20 coroutines")
 LANGOPT(CoroAlignedAllocation, 1, 0, "prefer Aligned Allocation according to 
P2014 Option 2")
 LANGOPT(DllExportInlines  , 1, 1, "dllexported classes dllexport inline 
methods")
-LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of 

[clang] Improve stack usage to increase recursive initialization depth (PR #88546)

[Matheus Izvekov via cfe-commits]

@@ -1070,57 +1077,24 @@ class Sema;
 };
 
   private:
-SmallVector Candidates;
-llvm::SmallPtrSet Functions;
-
-// Allocator for ConversionSequenceLists. We store the first few of these
-// inline to avoid allocation for small sets.
-llvm::BumpPtrAllocator SlabAllocator;
+ASTContext 
+SmallVector Candidates;
+llvm::SmallPtrSet Functions;

mizvekov wrote:

pre-existing small nit, for consistency:
```suggestion
SmallVector Candidates;
SmallPtrSet Functions;
```

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

@@ -13435,8 +13435,7 @@ void Sema::checkNonTrivialCUnion(QualType QT, 
SourceLocation Loc,
 void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   // If there is no declaration, there was an error parsing it.  Just ignore
   // the initializer.
-  if (!RealDecl || RealDecl->isInvalidDecl()) {
-CorrectDelayedTyposInExpr(Init, dyn_cast_or_null(RealDecl));
+  if (!RealDecl) {
 return;
   }

mizvekov wrote:

There is an odd change in behavior here, is that intentional?
If there is no declaration, or the declaration is not a VarDecl, then we don't 
try to correct typos in `Init` anymore.

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

@@ -13456,6 +13455,14 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
 return;
   }
 
+  if (VDecl->isInvalidDecl()) {
+CorrectDelayedTyposInExpr(Init, VDecl);
+VDecl->setInit(
+CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init})
+.get());

mizvekov wrote:

`CreateRecoveryExpr` returns an ExprResult, which means it not only returns an 
expression on success, but it can also return an error, and using `get()` in 
that case is UB, you have to check it first.

It can return an error in case we are in an SFINAE context, and also when using 
the frontend flag `-fno-recovery-ast`.

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

[clang] [clang][Sema] Preserve the initializer of invalid VarDecls (PR #88645)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov requested changes to this pull request.

Thanks for the improvement!

Can you also add a test case?

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

[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)

[Matheus Izvekov via cfe-commits]

@@ -1061,6 +1070,16 @@ CodeGenAction::CreateASTConsumer(CompilerInstance , 
StringRef InFile) {
 CI.getPreprocessor().addPPCallbacks(std::move(Callbacks));
   }
 
+  if (CI.getFrontendOpts().GenReducedBMI &&
+  !CI.getFrontendOpts().ModuleOutputPath.empty()) {
+std::vector> Consumers{2};

mizvekov wrote:

Minor nit: https://llvm.org/docs/CodingStandards.html#id28
```suggestion
std::vector> Consumers(2);
```

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

[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)

[Matheus Izvekov via cfe-commits]

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

[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)

[Matheus Izvekov via cfe-commits]

@@ -4045,6 +4045,24 @@ static bool RenderModulesOptions(Compilation , const 
Driver ,
   // module fragment.
   CmdArgs.push_back("-fskip-odr-check-in-gmf");
 
+  if (Args.hasArg(options::OPT_modules_reduced_bmi) &&
+  (Input.getType() == driver::types::TY_CXXModule ||
+   Input.getType() == driver::types::TY_PP_CXXModule)) {
+CmdArgs.push_back("-fexperimental-modules-reduced-bmi");
+
+if (Args.hasArg(options::OPT_fmodule_output_EQ))
+  Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ);
+else
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fmodule-output=" +
+  getCXX20NamedModuleOutputPath(Args, Input.getBaseInput(;

mizvekov wrote:

Would it be reasonable to push two separate arguments, instead of concatenating 
them?

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

[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)

[Matheus Izvekov via cfe-commits]

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

I share the objections that it may be too soon to introduce a driver flag for 
this. Only a frontend flag is ok for now, but it's non blocking on my side 
because it doesn't look like it will be particularly hard to deprecate it later.

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

[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)

[Matheus Izvekov via cfe-commits]

@@ -831,7 +831,7 @@ class PackDeductionScope {
 if (IsPartiallyExpanded)
   PackElements += NumPartialPackArgs;
 else if (IsExpanded)
-  PackElements += *FixedNumExpansions;
+  PackElements += FixedNumExpansions.value_or(1);

mizvekov wrote:

The assert is not necessary, unless it added explanation.
The `*` operator on the optional here should already assert in that case.

I would advise you run this test case on an llvm build with runtime checks 
enabled. The compiler is going off the rails before this point, so maybe there 
is an earlier assert that could narrow it down.

Otherwise, the change lacks explanation, about what is the problem, and what 
the fix does.

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

[clang] [clang][Index] Use canonical function parameter types in USRs (PR #68222)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

@sdkrystian ping, do you still intend to continue this? Just adding your 
example as a test case would be fine.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())

mizvekov wrote:

I see.

The code I posted was an attempt to construct such an example, where a pack 
doesn't need to be defaulted as-written for the proposed changes in the rules 
to have an effect. Except that example hits the wall at something else first, 
and I haven't fully investigated that. I haven't excluded the possibility that 
such an example could be constructed yet.

So when I proposed that provisional wording, I didn't exclude packs from the 
equation. I don't see a reason to, as without excluding them, the rules stay 
simpler and more general and should still work.

If we want to exclude them for implementation efficiency reasons, we could, as 
presumably it could still work as-if.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new

mizvekov wrote:

While it's true this is a DR, I just don't think for this particular case it's 
worth the cost of testing every single mode, as there are no particularly 
interesting interactions there.
Moot point as I will be moving it away anyway.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/89807

>From 43f813d0a1a87b6cad9b859237489778f4f2945f Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Closes #55894
---
 clang/docs/ReleaseNotes.rst   |  18 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 ++-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +++-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 115 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 ++-
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 ++-
 19 files changed, 292 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5e5d3a2e6ea36..f525bdd73010cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,16 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions.
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++20 Feature Support
@@ -152,6 +167,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

mizvekov wrote:

Thanks.

Since the core issue is not resolved by WG21, I don't want to claim to be 
resolving it here with this patch.

And while we don't remove the non-conforming flag, we should still test it, and 
it's just much more economical and error proof that we reuse the same tests.


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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = TemplateTypeParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(),
+T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+T->hasTypeConstraint());
+R->setDefaultArgument(
+S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+if (R->hasTypeConstraint()) {
+  auto *C = R->getTypeConstraint();
+  R->setTypeConstraint(C->getConceptReference(),
+   C->getImmediatelyDeclaredConstraint());

mizvekov wrote:

It's the same tests I linked in the other thread: 
https://github.com/llvm/llvm-project/blob/15f02723d49be9a828fbf072966a225babd60457/clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp#L48

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-verify=expected,new
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++23 
-fno-relaxed-template-template-args -verify=expected,old

mizvekov wrote:

Okay, I think I misunderstood then what `CXX/drs` was about, I should probably 
move it elsewhere.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/89807

>From 3f6e50edc7b4d4bf4781c71bd29f48224b62822d Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Closes #55894
---
 clang/docs/ReleaseNotes.rst   |  18 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 ++-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +++-
 clang/test/CXX/drs/cwg2398.cpp| 115 ++
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 ++-
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 ++-
 19 files changed, 292 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/CXX/drs/cwg2398.cpp
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5e5d3a2e6ea36..f525bdd73010cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,16 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions.
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++20 Feature Support
@@ -152,6 +167,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support '%0'">;
 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -519,13 +571,45 @@ DeduceTemplateArguments(Sema , TemplateParameterList 
*TemplateParams,
 return TemplateDeductionResult::Success;
   }
 
-  if (TemplateTemplateParmDecl *TempParam
-= dyn_cast(ParamDecl)) {
+  if (auto *TempParam = dyn_cast(ParamDecl)) {
 // If we're not deducing at this depth, there's nothing to deduce.
 if (TempParam->getDepth() != Info.getDeducedDepth())
   return TemplateDeductionResult::Success;
 
-DeducedTemplateArgument 
NewDeduced(S.Context.getCanonicalTemplateName(Arg));
+auto NewDeduced = DeducedTemplateArgument(Arg);
+// Provisional resolution for CWG2398: If Arg is also a template template
+// param, and it names a template specialization, then we deduce a
+// synthesized template template parameter based on A, but using the TS's
+// arguments as defaults.
+if (auto *TempArg = dyn_cast_or_null(
+Arg.getAsTemplateDecl())) {
+  assert(Arg.getKind() == TemplateName::Template);
+  assert(!TempArg->isExpandedParameterPack());
+
+  TemplateParameterList *As = TempArg->getTemplateParameters();
+  if (DefaultArguments.size() != 0) {
+assert(DefaultArguments.size() <= As->size());
+SmallVector Params(As->size());
+for (unsigned I = 0; I < DefaultArguments.size(); ++I)
+  Params[I] =
+  DeduceTemplateArguments(S, As->getParam(I), DefaultArguments[I]);
+for (unsigned I = DefaultArguments.size(); I < As->size(); ++I)
+  Params[I] = As->getParam(I);
+// FIXME: We could unique these, and also the parameters, but we don't
+// expect programs to contain a large enough amount of these deductions
+// for that to be worthwhile.

mizvekov wrote:

Yeah, doesn't affect identity. The results from deduction are assumed to be 
sugared, this was one of the changes I made to the deduction machinery back in 
2021 or so.

Where identity is concerned, the types should be canonicalized, and we have a 
special step for canonicalizing template template parameters.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = TemplateTypeParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(),
+T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+T->hasTypeConstraint());
+R->setDefaultArgument(
+S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+if (R->hasTypeConstraint()) {
+  auto *C = R->getTypeConstraint();
+  R->setTypeConstraint(C->getConceptReference(),
+   C->getImmediatelyDeclaredConstraint());
+}
+return R;
+  }
+  case Decl::NonTypeTemplateParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = NonTypeTemplateParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(), T->getType(),
+/*ParameterPack=*/false, T->getTypeSourceInfo());
+R->setDefaultArgument(Default.getAsExpr());
+if (auto *PTC = T->getPlaceholderTypeConstraint())
+  R->setPlaceholderTypeConstraint(PTC);
+return R;
+  }
+  case Decl::TemplateTemplateParm: {

mizvekov wrote:

A template template parameter can have a template template parameter as a 
parameter. These parameters can have default arguments too.

I thought I saw this test case covered in the clang test suite, but I was 
mistaken.

Here is one example test that will hit this case:
```C++
template  struct X {};

template class T2 = X> struct A {};

template struct B {};

template class TT1, class T4> struct B>; // #1
template class> class TT2, class T5, 
template  class T6> struct B> {}; // #2

template struct B>;
```

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -8343,58 +8343,52 @@ bool 
Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   // C++1z [temp.arg.template]p3: (DR 150)
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
-  // FIXME: We should enable RelaxedTemplateTemplateArgs by default as it is a
-  //  defect report resolution from C++17 and shouldn't be introduced by
-  //  concepts.
-  if (getLangOpts().RelaxedTemplateTemplateArgs) {

mizvekov wrote:

Yes, I think if there are problems in existing C++03 code, we should know about 
it, I can't see how that same problem wouldn't necessarily apply to newer 
standards.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())
+  return A;
+auto *R = TemplateTypeParmDecl::Create(
+S.Context, A->getDeclContext(), SourceLocation(), SourceLocation(),
+T->getDepth(), T->getIndex(), T->getIdentifier(),
+T->wasDeclaredWithTypename(), /*ParameterPack=*/false,
+T->hasTypeConstraint());
+R->setDefaultArgument(
+S.Context.getTrivialTypeSourceInfo(Default.getAsType()));
+if (R->hasTypeConstraint()) {
+  auto *C = R->getTypeConstraint();
+  R->setTypeConstraint(C->getConceptReference(),
+   C->getImmediatelyDeclaredConstraint());

mizvekov wrote:

Note we don't do anything interesting about concepts in this patch, we are just 
doing a member-wise copy of the declarations, except for the DefaultArgument. 
Examples where concepts are being copied along here already exist in the clang 
test suite.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())

mizvekov wrote:

It's possible to write code that hits this condition, in fact such code is 
already in the clang test suite.

Currently the Decls for Type and Expr parameters don't store a whole 
TemplateArgument for the default argument, in order to save a few bytes, they 
store only a TSI and Expr* respectively, which doesn't cover the Pack case.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -1133,8 +1133,8 @@ C++17 implementation status
 
 
   Matching template template parameters to compatible arguments
-  https://wg21.link/p0522r0;>P0522R0
-  Partial (10)
+  https://wg21.link/p0522r0;>P0522R0 (DR)
+  Clang 4 (10)

mizvekov wrote:

This entry was one of the review requests from @zygoloid in the original phab 
PR: https://reviews.llvm.org/D109496?id=384873#3031693

I think saying we implemented in Clang 4 is more accurate, we even explain in 
the note that it was behind a switch. But still users have known about it and 
have been using it for a long time.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

@@ -507,10 +507,62 @@ static TemplateDeductionResult 
DeduceNonTypeTemplateArgument(
   S, TemplateParams, NTTP, DeducedTemplateArgument(New), T, Info, Deduced);
 }
 
+static NamedDecl *DeduceTemplateArguments(Sema , NamedDecl *A,
+  TemplateArgument Default) {
+  switch (A->getKind()) {
+  case Decl::TemplateTypeParm: {
+auto *T = cast(A);
+// FIXME: DefaultArgument can't represent a pack.
+if (T->isParameterPack())

mizvekov wrote:

The test `foo` in 
`clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp` exercises 
this, but it doesn't involve any default arguments, so any effects are 
incidental / harmless.

I have actually just come up with a relevant test case that shows something 
interesting that I wasn't aware before, this could possibly be new info for the 
core issue.

https://godbolt.org/z/rnbzhnqG7

This shows a change of behavior introduced by P05522, only instead of 
introducing ambiguity, it causes different candidates to be picked. Also 
affects GCC.

---

Regarding the FIXME:

So the (Type | NonType | Template) template parameter declarations store the 
default argument for themselves. They have a `setDefaultArgument` method for 
setting that. While for template template parameter declarations the 
`setDefaultArgument` takes a `TemplateArgument`, and that means it can use any 
kind of TemplateArgument as a default argument, including packs.

The same is not true for (Type | NonType) template parameter declarations, 
their `setDefaultArgument` only takes a (Type / Expression), which means a pack 
can't be passed.

This is done presumably to save space in those declarations, while a Type and 
an Expression are a single pointer, a `TemplateArgument` is considerably 
larger, and you would need at least a pointer and an integer to represent a 
pack.

To 'fix' this FIXME, we would have to change that, and allow those declarations 
to be larger in at least some circumstances.

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

[clang] [clang] deprecate frelaxed-template-template-args, make it on by default (PR #89807)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

So Jason pointed out that GCC's provisional wording for CWG2398 picks a dubious 
candidate for this example:
```C++
template struct match2;

template class t1,typename T>
struct match2, typename t1::type > { typedef int type; }; // #5
 

template class t2,typename T0,typename T1>
struct match2, typename t2::type > { typedef int type; }; // 
#6 

template  struct Q { typedef int type; };
match2, int> m;
```

They pick #6, where with this PR we stay with ambiguous.
According to [this GCC 
bug](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114841), he suggests changing 
GCC to adopt the wording proposed here.

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

@@ -3647,15 +3647,19 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
-/// This function checks if the function has 'auto' return type that contains
+/// This function checks if the given function has a return type that contains
 /// a reference (in any way) to a declaration inside the same function.
-bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
+bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
   assert(FromFPT && "Must be called on FunctionProtoType");
 
+  bool IsLambdaDefinition = false;
+  if (const auto *MD = dyn_cast(D))
+IsLambdaDefinition = cast(MD->getDeclContext())->isLambda();

mizvekov wrote:

Also, this problem of the return type not being wrapped on an AutoType only 
happens on lambdas in *C++11*.

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> I tested this idea, might be like this:

I meant something else. This patch makes it so the delayed function type 
importation happens unconditionally.
I was aware that change would cause breakages.

What I meant is that, we have a special sugar type node that marks deduction, 
this AutoType, and we apply it everywhere else, except this lambda case you 
exposed. I was going to suggest we do the same for these lambdas. That would 
make things more consistent.



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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

@@ -3647,15 +3647,19 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
-/// This function checks if the function has 'auto' return type that contains
+/// This function checks if the given function has a return type that contains
 /// a reference (in any way) to a declaration inside the same function.
-bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
+bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
   assert(FromFPT && "Must be called on FunctionProtoType");
 
+  bool IsLambdaDefinition = false;
+  if (const auto *MD = dyn_cast(D))
+IsLambdaDefinition = cast(MD->getDeclContext())->isLambda();

mizvekov wrote:

Thanks, this works, but if we are going this way, why not also check the lambda 
was defined without a return type?

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

@@ -3647,15 +3647,19 @@ class IsTypeDeclaredInsideVisitor
 };
 } // namespace
 
-/// This function checks if the function has 'auto' return type that contains
+/// This function checks if the given function has a return type that contains
 /// a reference (in any way) to a declaration inside the same function.
-bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
+bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
   QualType FromTy = D->getType();
   const auto *FromFPT = FromTy->getAs();
   assert(FromFPT && "Must be called on FunctionProtoType");
 
+  bool IsLambdaDefinition = false;
+  if (const auto *MD = dyn_cast(D))
+IsLambdaDefinition = cast(MD->getDeclContext())->isLambda();

mizvekov wrote:

No, you don't need to hunt for return statements.

You can just check on the CXXMethodDecl that it's FunctionProtoType has a 
trailing return (`hasTrailingReturn` method).
I think that should cover it for all cases.

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

[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)

[Matheus Izvekov via cfe-commits]

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

LGTM, Thanks!

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> @mizvekov We have a bunch of related issues, could you look at them 
> https://github.com/llvm/llvm-project/issues?q=is%3Aissue+is%3Aopen+%22-frelaxed-template-template-args%22
>  ?

Removed a bunch of duplicates. 4 issues remaining:

1) #36505 Is the issue we are fixing in this patch.
2) #65843 Is unrelated.
3) #63281 Is not a bug
4) #62529 The bug is accepting it, although perhaps we could seek clarification 
if we should exclude template template parameters from 
[CWG1430](https://cplusplus.github.io/CWG/issues/1430.html)


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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

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

[clang] Fix a crash introduced by 3d5e9ab by adding a nullptr check. (PR #90301)

[Matheus Izvekov via cfe-commits]

@@ -54,7 +54,7 @@ class UncountedCallArgsChecker
   bool shouldVisitImplicitCode() const { return false; }
 
   bool TraverseDecl(Decl *D) {
-if (isa(D) && isRefType(safeGetName(D)))
+if (D && isa(D) && isRefType(safeGetName(D)))
   return true;
 return RecursiveASTVisitor::TraverseDecl(D);
   }

mizvekov wrote:

Shouldn't this be overriding `TraverseTemplateInstantiations(ClassTemplateDecl 
*D)` instead?

 `RecursiveASTVisitor::TraverseDecl`  will ensure you won't be 
visiting null nodes.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/89807

>From 4ee58efa0f154b531dcc674b6f4fe084182aa803 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Fixes https://github.com/llvm/llvm-project/issues/36505
---
 clang/docs/ReleaseNotes.rst   |  19 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 ++-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +++-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 115 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 ++-
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 ++-
 19 files changed, 293 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1390d6536b28c..bd7f96246fd407 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,17 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions
+  (#GH36505).
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++17 Feature Support
@@ -164,6 +180,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported 

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/89807

>From 1756044e71d756f7102f962d0298627ede27871c Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Tue, 9 Apr 2024 01:14:28 -0300
Subject: [PATCH] [clang] Enable C++17 relaxed template template argument
 matching by default

In order to implement this as a DR and avoid breaking reasonable code
that worked before P0522, this patch implements a provisional resolution
for CWG2398: When deducing template template parameters against each other,
and the argument side names a template specialization, instead of just
deducing A, we instead deduce a synthesized template template parameter based
on A, but with it's parameters using the template specialization's arguments
as defaults.

The driver flag is deprecated with a warning.

With this patch, we finally mark C++17 support in clang as complete.

Fixes https://github.com/llvm/llvm-project/issues/36505
---
 clang/docs/ReleaseNotes.rst   |  19 +++
 .../clang/Basic/DiagnosticDriverKinds.td  |   2 +-
 clang/include/clang/Basic/LangOptions.def |   2 +-
 clang/include/clang/Driver/Options.td |   8 +-
 clang/lib/Driver/SanitizerArgs.cpp|   9 +-
 clang/lib/Driver/ToolChains/Clang.cpp |  16 +-
 clang/lib/Sema/SemaTemplate.cpp   |   3 -
 clang/lib/Sema/SemaTemplateDeduction.cpp  | 107 +-
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp |   2 +-
 clang/test/CodeGenCXX/mangle-concept.cpp  |   4 +-
 .../frelaxed-template-template-args.cpp   |   5 +
 clang/test/Lexer/cxx-features.cpp |   6 +-
 clang/test/SemaTemplate/cwg2398.cpp   | 139 ++
 clang/test/SemaTemplate/default-arguments.cpp |   7 +-
 .../instantiate-template-template-parm.cpp|  17 +--
 clang/test/SemaTemplate/nested-template.cpp   |   8 +-
 clang/test/SemaTemplate/temp_arg_template.cpp |   6 +-
 .../SemaTemplate/temp_arg_template_cxx1z.cpp  |   2 +-
 clang/www/cxx_status.html |  18 +--
 19 files changed, 317 insertions(+), 63 deletions(-)
 create mode 100644 clang/test/Driver/frelaxed-template-template-args.cpp
 create mode 100644 clang/test/SemaTemplate/cwg2398.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1390d6536b28c..bd7f96246fd407 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C++ Specific Potentially Breaking Changes
 - Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
   This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
+- The behavior controlled by the `-frelaxed-template-template-args` flag is now
+  on by default, and the flag is deprecated. Until the flag is finally removed,
+  it's negative spelling can be used to obtain compatibility with previous
+  versions of clang.
+
 ABI Changes in This Version
 ---
 - Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -88,6 +93,17 @@ sections with improvements to Clang's support for those 
languages.
 
 C++ Language Changes
 
+- C++17 support is now completed, with the enablement of the
+  relaxed temlate template argument matching rules introduced in P0522,
+  which was retroactively applied as a defect report.
+  While the implementation already existed since Clang 4, it was turned off by
+  default, and was controlled with the `-frelaxed-template-template-args` flag.
+  In this release, we implement provisional wording for a core defect on
+  P0522 (CWG2398), which avoids the most serious compatibility issues caused
+  by it, allowing us to enable it by default in this release.
+  The flag is now deprecated, and will be removed in the next release, but can
+  still be used to turn it off and regain compatibility with previous versions
+  (#GH36505).
 - Implemented ``_BitInt`` literal suffixes ``__wb`` or ``__WB`` as a Clang 
extension with ``unsigned`` modifiers also allowed. (#GH85223).
 
 C++17 Feature Support
@@ -164,6 +180,9 @@ Resolutions to C++ Defect Reports
 - Clang now diagnoses declarative nested-name-specifiers with 
pack-index-specifiers.
   (`CWG2858: Declarative nested-name-specifiers and pack-index-specifiers 
`_).
 
+- P0522 implementation is enabled by default in all language versions, and
+  provisional wording for CWG2398 is implemented.
+
 C Language Changes
 --
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ed3fd9b1c4a55b..9781fcaa4ff5e9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -435,7 +435,7 @@ def warn_drv_diagnostics_misexpect_requires_pgo : Warning<
 def warn_drv_clang_unsupported : 

[clang] Fix a crash introduced by 3d5e9ab by adding a nullptr check. (PR #90301)

[Matheus Izvekov via cfe-commits]

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


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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> Here's a preprocessed file: 
> [repro.zip](https://github.com/llvm/llvm-project/files/15250584/repro.zip)
> 
> I tried to reduce, and got rid of most of the test code and some of the 
> stdexec code, but there's still a lot left. I hit the end of my timebox on 
> that. Maybe creduce can do better.

That crashes with released clang 18.1.4 as well, same stack trace it seems.
```
Homebrew clang version 18.1.4
Target: arm64-apple-darwin23.4.0
```

Is it possible you may have reduced into a different bug?
It's always helpful in that case to keep testing with known good compiler while 
reducing.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> @mizvekov I have a [reduced test 
> case](https://github.com/llvm/llvm-project/files/15261978/repro.zip) for the 
> crash @sam-mccall reported.
> 
> Clang does not crash before but crashes at this revision.

Thanks. I confirm it crashes, but it crashes on clang 18.1.4 as well, if one 
just passes `-frelaxed-template-template-args`.
So most likely this is the same problem, we exposed it by default, but wasn't 
caused by changes in this patch per se.

The program became invalid with the change. The crash is most likely secondary, 
caused by bad error recovery.

The former is the most interesting thing here, we can try to see if it's one of 
the cases where we can solve with additional wording, but the reduction process 
might have mangled this.

Is it possible the original code already had workaround for 
template-template-args with GCC, but the workaround was otherwise GCC specific, 
not using the feature testing macro?


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

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

I agree with @sdkrystian, even though the test crashes for maybe yet another 
reason, it demonstrates you can friend a function from a different template 
context, so comparing the depths from different branches is not helpful.

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

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

I had some discussion about that with @Endilll on the previous MR regarding 
this core issue: https://github.com/llvm/llvm-project/pull/89807

Since there is no posting at all in core about any possible solutions, I wanted 
to get feedback from them before claiming to be implementing a solution in the 
public documentation.

Also, this needs to test a non-confirming mode until we finally remove the 
corresponding flag, and that is against the rules for the DR suite.

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

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/90820

This solves some ambuguity introduced in P0522 regarding how template template 
parameters are partially ordered, and should reduce the negative impact of 
enabling `-frelaxed-template-template-args` by default.

A template template parameter containing no packs should be more specialized 
than one that does.

Given the following example:
```C++
template struct A;
template class TT1, class T4> struct A>; #1
template class TT2, class T6> struct A>; #2

template struct B;
template struct A>;
```

Prior to P0522, #2 would be the only viable candidate. After P0522, #1 is also 
viable, and neither is considered more specialized, so this becomes ambiguous.
With this change, #2 is considered more specialized, so that is what is picked 
and we remain compatible with pre-P0522 implementations.

The problem we solve here is that the specialization rules in 
`[temp.arg.template]/4` are defined in terms of a rewrite to function 
templates, but in the case of partial ordering, the rules on how P and A lists 
are matched to each other is swapped regarding how specialization makes sense 
conceptually.

---

Since this changes provisional implementation of CWG2398 which has not been 
released yet, and already contains a changelog entry, we don't provide a 
changelog entry here.

>From b7df9a342724479e129af252f2ff37229f30d41b Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Wed, 1 May 2024 22:29:45 -0300
Subject: [PATCH] [clang] Implement provisional wording for CWG2398 regarding
 packs

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling `-frelaxed-template-template-args`
by default.

A template template parameter containing no packs should be more
specialized than one that does.

Given the following example:
```C++
template struct A;
template class TT1, class T4> struct A>; #1
template class TT2, class T6> struct A>; #2

template struct B;
template struct A>;
```

Prior to P0522, #2 would be the only viable candidate.
After P0522, #1 is also viable, and neither is considered more specialized,
so this becomes ambiguous.
With this change, #2 is considered more specialized, so that is what is picked
and we remain compatible with pre-P0522 implementations.

The problem we solve here is that the specialization rules in
`[temp.arg.template]/4` are defined in terms of a rewrite to
function templates, but in the case of partial ordering, the
rules on how P and A lists are matched to each other
is swapped regarding how specialization makes sense conceptually.

---

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.
---
 clang/include/clang/Sema/Sema.h  |  5 +-
 clang/lib/Sema/SemaTemplate.cpp  | 10 +--
 clang/lib/Sema/SemaTemplateDeduction.cpp | 79 ++--
 clang/test/SemaTemplate/cwg2398.cpp  |  6 --
 4 files changed, 68 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a80ac6dbc76137..2bea8732f61fcd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9238,7 +9238,7 @@ class Sema final : public SemaBase {
CheckTemplateArgumentKind CTAK);
   bool CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
  TemplateParameterList *Params,
- TemplateArgumentLoc );
+ TemplateArgumentLoc , bool IsDeduced);
 
   void NoteTemplateLocation(const NamedDecl ,
 std::optional ParamRange = {});
@@ -9708,7 +9708,8 @@ class Sema final : public SemaBase {
 sema::TemplateDeductionInfo );
 
   bool isTemplateTemplateParameterAtLeastAsSpecializedAs(
-  TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc);
+  TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc,
+  bool IsDeduced);
 
   void MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
   unsigned Depth, llvm::SmallBitVector );
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 989f3995ca5991..2c921d4365db0a 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -6384,7 +6384,8 @@ bool Sema::CheckTemplateArgument(
 
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion:
-if (CheckTemplateTemplateArgument(TempParm, Params, Arg))
+if (CheckTemplateTemplateArgument(TempParm, Params, Arg,
+  /*IsDeduced=*/CTAK != CTAK_Specified))
   return true;
 
 SugaredConverted.push_back(Arg.getArgument());
@@ -8296,7 

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

Note we are not implementing the solution Jason posted on the core mailing 
list, neither on the previous patch, as we have a better solution than current 
GCC on this, nor on this MR, as GCC implements no such workaround and still 
fails this test.

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

[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] CTAD: fix the aggregate deduction guide for alias templates. (PR #90894)

[Matheus Izvekov via cfe-commits]

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

[clang] [clang] CTAD: fix the aggregate deduction guide for alias templates. (PR #90894)

[Matheus Izvekov via cfe-commits]

@@ -261,6 +261,13 @@ AG ag = {1};
 // CHECK:   | `-BuiltinType {{.*}} 'int'
 // CHECK:   `-ParmVarDecl {{.*}} 'int'
 
+template 
+using BG = G;
+BG bg(1.0);
+// CHECK-LABEL: Dumping 
+// CHECK:  FunctionTemplateDecl {{.*}} implicit 
+// CHECK: |-CXXDeductionGuideDecl {{.*}} 'auto (int) -> G' aggregate

mizvekov wrote:

Nit: Since this isn't matching the structure of the AST anyway, it would 
probably be a good idea to avoid the possibility of unrelated test churn.

```suggestion
// CHECK: CXXDeductionGuideDecl {{.*}} 'auto (int) -> G' aggregate
```

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

[clang] [clang] CTAD: fix the aggregate deduction guide for alias templates. (PR #90894)

[Matheus Izvekov via cfe-commits]

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

This looks like a straightforward fix to me as well

LGTM

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

[clang] [clang] Implement provisional wording for CWG2398 regarding packs (PR #90820)

[Matheus Izvekov via cfe-commits]

mizvekov wrote:

> Sounds like should perhaps note that we are implementing our own resolution, 
> until there's an update to the cwg issue that can be referred to?

That could be. Is there another similar issue we could use as a reference on 
the format for this?

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