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

https://github.com/mizvekov closed 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)

https://github.com/ChuanqiXu9 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)

https://github.com/ChuanqiXu9 approved this pull request. LG. It looks not wise to block this good PR. After all, it is a change 6 lines. Let's go ahead. https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-commits@lists.ll

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

mizvekov wrote: I have revived this PR after a long time. The original ODR violation bad diagnostic from the GH issue is gone from main, so I have removed the test for it. My best guess is that the ODR checking must have gotten weakened somehow. I really don't have time to dig into it though.

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

https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/80245 >From 9763cc9e6f081bc28b74164c77a2b80ac42aec1c Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Thu, 1 Feb 2024 02:26:10 -0300 Subject: [PATCH] [clang] fix merging of UsingShadowDecl Previously, when decidi

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

@@ -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 -

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

@@ -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 -

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

ChuanqiXu9 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

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

@@ -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_cc

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

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] [clang] fix merging of UsingShadowDecl (PR #80245)

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 di

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

@@ -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 -

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

@@ -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_cc

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

@@ -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_cc

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

https://github.com/ChuanqiXu9 commented: I still think the testing for dumped text is not acceptable. Let's make it prettier in unittest. https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

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

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

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

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] [clang] fix merging of UsingShadowDecl (PR #80245)

ChuanqiXu9 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. But they are much easier to maintain than dumpped AST text. I feel thi

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

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 compa

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

ChuanqiXu9 wrote: But the test for dumpped text still looks not good. Can we get rid of that? 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-co

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

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 m

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

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] [clang] fix merging of UsingShadowDecl (PR #80245)

ChuanqiXu9 wrote: > The issue is up: #80252 Thanks. Although I feel it is too implementor's view, (I mean the end users can hardly understand it), it is much better. > Yeah, looking at the similarities, there is a decent chance this is the same > issue, it's worth trying it out. Yeah, let's

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

@@ -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 -

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

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

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

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] [clang] fix merging of UsingShadowDecl (PR #80245)

@@ -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 -

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

@@ -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

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

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] [clang] fix merging of UsingShadowDecl (PR #80245)

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)

https://github.com/ChuanqiXu9 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)

@@ -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

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

@@ -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

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

@@ -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

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

@@ -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

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

https://github.com/ChuanqiXu9 commented: IIRC, in https://github.com/llvm/llvm-project/pull/79959, you're saying the patch have the potential to fix https://github.com/llvm/llvm-project/issues/78850? Is it true? https://github.com/llvm/llvm-project/pull/80245 __

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

@@ -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 -

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

https://github.com/ChuanqiXu9 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)

@@ -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

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

@@ -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

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

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

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

ChuanqiXu9 wrote: Before looking into it, I remember the user branch is only allowed for stacked review. If this patch is not the case, let's try to create a new PR. https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-comm

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

https://github.com/ChuanqiXu9 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