[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian created https://github.com/llvm/llvm-project/pull/90152 Reapplies #84050, addressing a bug which cases a crash when an expression with the type of the current instantiation is used as the _postfix-expression_ in a class member access expression (arrow form). >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typenam
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Krystian Stasiowski (sdkrystian) Changes Reapplies #84050, addressing a bug which cases a crash when an expression with the type of the current instantiation is used as the _postfix-expression_ in a class member access expression (arrow form). --- Patch is 68.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90152.diff 30 Files Affected: - (modified) clang-tools-extra/clangd/unittests/FindTargetTests.cpp (+4-4) - (modified) clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp (+1-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp (+2) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp (+12) - (modified) clang/docs/ReleaseNotes.rst (+12) - (modified) clang/include/clang/Sema/Lookup.h (+3-1) - (modified) clang/include/clang/Sema/Sema.h (+8-6) - (modified) clang/lib/AST/Expr.cpp (+1-1) - (modified) clang/lib/Parse/ParseDecl.cpp (+1-1) - (modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+5-2) - (modified) clang/lib/Sema/SemaAttr.cpp (+1-1) - (modified) clang/lib/Sema/SemaDecl.cpp (+4-3) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+3-3) - (modified) clang/lib/Sema/SemaExpr.cpp (+10-10) - (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-1) - (modified) clang/lib/Sema/SemaExprMember.cpp (+69-94) - (modified) clang/lib/Sema/SemaLookup.cpp (+89-25) - (modified) clang/lib/Sema/SemaOpenMP.cpp (+12-5) - (modified) clang/lib/Sema/SemaTemplate.cpp (+12-20) - (modified) clang/lib/Sema/TreeTransform.h (+20) - (modified) clang/test/AST/HLSL/this-reference-template.hlsl (+1-1) - (modified) clang/test/CXX/drs/dr2xx.cpp (+5-5) - (modified) clang/test/CXX/drs/dr3xx.cpp (+8-8) - (added) clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp (+456) - (modified) clang/test/CXX/temp/temp.res/temp.local/p3.cpp (+1-2) - (modified) clang/test/CodeGenCXX/mangle.cpp (-8) - (modified) clang/test/Index/annotate-nested-name-specifier.cpp (+2-2) - (modified) clang/test/SemaCXX/member-expr.cpp (+2-2) - (modified) clang/test/SemaTemplate/instantiate-function-1.cpp (+7-7) - (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+3-2) ``diff diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp index 574efe7bd91478..ae61b17ca14d20 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp @@ -309,6 +309,8 @@ struct HeapArray { // Ok, since destruc HeapArray(HeapA
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: Will add tests tomorrow... https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/4] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 6ea7f9d476910681ad01e2fe4525fb4d2c556c6f Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/4] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 14 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 183 +++ clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 765 insertions(+), 225 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/clang
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 99d48cf8ae4987eb747f3baf128265dadb7d2038 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 20 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 219 - clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 778 insertions(+), 254 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/cla
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
erichkeane wrote: Please also identify the changes you made vs the previous patch. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @erichkeane I placed all the new changes into a [single commit](https://github.com/llvm/llvm-project/pull/90152/commits/b8fda8106405eed0c234802fff8eecde53f2562e) https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/erichkeane commented: Code changes LGTM, please add the tests, and I'll do another run at this. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: Tests have already been added :) @erichkeane https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/Endilll commented: Changes to `Sema.h` and DR tests look good to me. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90152 >From 2d613e80a59d69a263eaa713d080b97c307fe91b Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 14:50:53 -0400 Subject: [PATCH 1/2] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" Consider the following: ```cpp template struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition (i.e. prior to instantiation). --- .../clangd/unittests/FindTargetTests.cpp | 8 +- .../unittests/SemanticHighlightingTests.cpp | 2 +- .../cppcoreguidelines/owning-memory.cpp | 2 + .../modernize/use-equals-default-copy.cpp | 12 + clang/docs/ReleaseNotes.rst | 12 + clang/include/clang/Sema/Lookup.h | 4 +- clang/include/clang/Sema/Sema.h | 20 +- clang/lib/AST/Expr.cpp| 2 +- clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Sema/HLSLExternalSemaSource.cpp | 7 +- clang/lib/Sema/SemaAttr.cpp | 2 +- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/lib/Sema/SemaDeclCXX.cpp| 6 +- clang/lib/Sema/SemaExpr.cpp | 20 +- clang/lib/Sema/SemaExprCXX.cpp| 2 +- clang/lib/Sema/SemaExprMember.cpp | 219 - clang/lib/Sema/SemaLookup.cpp | 114 - clang/lib/Sema/SemaOpenMP.cpp | 17 +- clang/lib/Sema/SemaTemplate.cpp | 32 +- clang/lib/Sema/TreeTransform.h| 20 + .../AST/HLSL/this-reference-template.hlsl | 2 +- clang/test/CXX/drs/dr2xx.cpp | 10 +- clang/test/CXX/drs/dr3xx.cpp | 16 +- .../temp.res/temp.dep/temp.dep.type/p4.cpp| 456 ++ .../test/CXX/temp/temp.res/temp.local/p3.cpp | 3 +- clang/test/CodeGenCXX/mangle.cpp | 8 - .../Index/annotate-nested-name-specifier.cpp | 4 +- clang/test/SemaCXX/member-expr.cpp| 4 +- .../SemaTemplate/instantiate-function-1.cpp | 14 +- .../ASTMatchers/ASTMatchersNarrowingTest.cpp | 5 +- 30 files changed, 778 insertions(+), 254 deletions(-) create mode 100644 clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 799a549ff0816e..94437857cecca6 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but base expression involves a function call. Code = R"cpp( @@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Similar to above but uses a function pointer. Code = R"cpp( @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) { } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()"); + EXPECT_DECLS("MemberExpr", "void foo()"); // Base expression involves a member access into this. Code = R"cpp( @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) { void Foo() { this->[[find]](); } }; )cpp"; - EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()"); + EXPECT_DECLS("MemberExpr", "void find()"); } TEST_F(TargetDeclTest, DependentTypes) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 4156921d83edf8..30b9b1902aa9c7 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]); struct $Class_def[[Foo]] { int $Field_decl[[Waldo]]; void $Method_def[[bar]]() { - $Class[[Foo]]().$Field_dependentName[[Waldo]]; + $Class[[Foo]]().$Field[[Waldo]]; } template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]] void $Method_def[[bar1]]() { diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp b/clang-tools-extra/test/cla
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
https://github.com/sdkrystian closed https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: I built boost with `-DBUILD_TESTING=ON` and didn't see any regressions... hopefully that remains the case https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
hiraditya wrote: I believe this is breaking the buildbot https://lab.llvm.org/buildbot/#/builders/245/builds/23936 please confirm. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @erichkeane pushed a commit which fixes it https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
lukel97 wrote: Hi, this seems to uncover an unfortunate error in xalan-c https://github.com/apache/xalan-c/blob/c326619da4813acfc845c2830d904a4860f9afe1/src/xalanc/XMLSupport/XalanOtherEncodingWriter.hpp#L323 which is present in one of the SPEC CPU 2017 benchmarks (and prevents SPEC CPU 2017 from being built) The error in xalan-c looks to be real since I can't find any definition of `m_isPresentable`, so I don't think this is an issue with this patch per say. Thought I'd flag this anyway in case others are running into the same issue. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
davemgreen wrote: Hi - We've ran into a couple of places where this causes problems, one of them in running Spec as above. Is it possible to turn off this error for older codebases with a flag, turning it into a warning? It doesn't seem like a very useful error if it applies to code that is never used. https://godbolt.org/z/Pf83EW7vE https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @davemgreen you can try using `-fdelayed-template-parsing` to fix the error. A compatibility flag could be added for older code bases -- but I'd like guidance from @erichkeane, @cor3ntin, and/or @AaronBallman on that. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
erichkeane wrote: I think the `-fdelayed-template-parsing` is as much as we're going to do. This is a conformance issue, and I don't think we want to create a backwards compatibility flag for it. Clang has ALWAYS diagnosed un-instantiatable templates as aggressively as we could. ALL of the examples above are functions that can never be used, even in other compilers. SO I think diagnosing them aggressively/early is both acceptable, and of benefit to the programmers. In all 3 cases, I'd suggest fixing it in some way: 1- Change the call to what the author of the code REALLY meant 2- Delete the function containing the issue entirely, since it clearly isn't being instantiated. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
eaeltsin wrote: Hi, This seems to break the following combination with overloaded -> operator - https://godbolt.org/z/jc6chKTdv ``` template class Clone { public: Clone(const Clone&); T* operator->() const; T* ptr_; }; // Assume T* T::clone() template inline Clone::Clone(const Clone& t) : ptr_(t->clone()) {} template inline T* Clone::operator->() const { return ptr_; } ``` https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
ilya-biryukov wrote: 🤯 amazingly, this only happens when the identifier being called is a lowercase version of the class name: https://gcc.godbolt.org/z/8aoaoPKnT: ```cpp template class Clone { public: Clone(const Clone&); T* operator->() const; T* ptr_; }; template struct Foo { Foo(const Foo&); T* operator->() const; T* ptr_; }; // Assume T* T::clone() template inline Clone::Clone(const Clone& t) { t->foo(); // ok t->clone(); // error t->bar(); // ok } // Assume T* T::clone() template inline Foo::Foo(const Foo& t) { t->foo(); // error t->clone(); // ok t->bar(); // ok } ``` I am looking forward to learning what is special about the matching names. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @ilya-biryukov Actually, that is an incredibly useful piece of information :) I think this is an issue with type correction... https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: Repro: ```cpp template struct Typo { Typo(const Typo& t) { t->typo; // error t->Typp; // error t->Tzpo; // error t->ty; // ok } }; ``` https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
ilya-biryukov wrote: Heh, of course! Now that you say it, that's quite obvious. Do you think is fix is just around the corner or will it take a long time? This blocks our internal compiler release and there is no workaround we could easily employ. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @ilya-biryukov I can fix this quickly (less than an hour). https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
ilya-biryukov wrote: Awesome, thanks a lot for looking into it! https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @ilya-biryukov See #91972 https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @eaeltsin @ilya-biryukov Fixed in 596a9c1f9b3179b3c77cbde1e96619292ce2a10a https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
eaeltsin wrote: Thanks for the quick fix! https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sbc100 wrote: This change recently rolled into the emscripten SDK and seems to be breaking the build of regal. I've not invistigated this yet, and its not our code: ``` em++ -c /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal/RegalIff.cpp -o /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports-builds/regal/regal/RegalIff.cpp.o -g -sSTRICT -Werror -O2 -I/usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src -DNDEBUG -DREGAL_LOG=0 -DREGAL_MISSING=0 -std=gnu++14 -fno-rtti -fno-exceptions -O3 -I/usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal -I/usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/lookup3 -I/usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/boost -Wno-deprecated-register -Wno-unused-parameter In file included from /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal/RegalIff.cpp:49: In file included from /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal/RegalIff.h:80: /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal/linear.h:585:26: error: no member named 'negate' in 'Vec4'; did you mean 'Negate'? 585 | Vec4 rv(*this); rv.negate(); return rv; | ^~ | Negate /usr/local/google/home/sbc/dev/wasm/emscripten/cache/ports/regal/regal-version_7/src/regal/linear.h:534:11: note: 'Negate' declared here 534 | void Negate() { | ^ 1 error generated. ``` https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
dyung wrote: We are also seeing a build break internally which I bisected back to this change. In our case, I was able to reduce it to the following code: ```c++ #include template class b { b operator=(b) { return operator=<>; } template ::d>> void operator=(b); }; ``` After this change, this is now giving an error message: ``` repro:3:27: error: missing 'template' keyword prior to dependent template name 'operator=' 3 | b operator=(b) { return operator=<>; } | ^~~ 1 error generated. ``` This code compiles successfully with gcc 13.2, and if I apply the `-fdelayed-template-parsing` it also compiles, but I just wanted to make sure this change was expected, and if so, how should the code be fixed to be correct? Godbolt link: https://godbolt.org/z/57br1s47z https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
yxsamliu wrote: This patch seems to cause a regression for rocThrust: https://github.com/ROCm/rocThrust/blob/f3a28e43355b0f439fb99a2210bd497ca59c8003/thrust/optional.h#L2756 rocThrust/thrust/../thrust/optional.h:2756:11: error: no member named 'construct' in 'optional' A reduced test case is ``` template struct A { T* m_value; void foo() { this->doit(); } T *operator->() { return m_value; } }; ``` https://godbolt.org/z/eo5f3Wdv5 It seems clang does not account for the situation that the arrow operator may be overloaded to return an pointer to a template parameter type whose members are unknown. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @yxsamliu The type of `this` is always a pointer. The class member access operator may only be overloaded for operands of class type. In your reduced example, could you provide a value of `T` that would instantiate to a valid specialization? https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
erichkeane wrote: > @yxsamliu The type of `this` is always a pointer. The class member access > operator may only be overloaded for operands of class type. In your reduced > example, could you provide a value of `T` that would instantiate to a valid > specialization? Agreed, this is another case where your change is actually finding a bug! @yxsamliu : see https://godbolt.org/z/PK38rodax If you intend to call `A::operator->` you have to spell it like this: https://godbolt.org/z/c9s3aqa7P https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @yxsamliu The call to `this->construct` in the `emplace` member of the partial specialization `class optional` _is_ a bug -- unlike the primary class template, the partial specialization for `T&` does _not_ inherit from `optional_operations_base` (which declares the `construct` member function). https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @dyung It's a bit difficult to tell what the code is intended to do. An expression naming a non-static member function must either be the postfix-expression of a call, or be the terminal name of a qualified-id that is the operand of unary `&`. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
yxsamliu wrote: Thanks for all your analysis. I agree this is a rocThrust bug. I will open an issue to rocThrust. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
zmodem wrote: This seems to break the version of libstdc++ we use in our sysroot: (Details on https://crbug.com/338536261) ``` ../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_iterator.h:1718:20: error: use 'template' keyword to treat 'operator =' as a dependent template name 1718 | return this->operator=<_It, _Sent>(__x); |^ 1 error generated. ``` Is that expected, and if so do you have any suggestions for how to work around it? https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
erichkeane wrote: > This seems to break the version of libstdc++ we use in our sysroot: (Details > on https://crbug.com/338536261) > > ``` > ../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_iterator.h:1718:20: > error: use 'template' keyword to treat 'operator =' as a dependent template > name > 1718 | return this->operator=<_It, _Sent>(__x); > |^ > 1 error generated. > ``` > > Is that expected, and if so do you have any suggestions for how to work > around it? Ooof, that one hurts and is likely something we have to do something about. I tracked down thefix to this commit: https://github.com/gcc-mirror/gcc/commit/56c9998602fd5091ba0985e2e5eaa90c6478 It wasn't done intentionally as far as I can tell (just that it was a rewrite/refactor?). So it looks like this was broken back through 2022, so only fixed as of 2 years ago. This IS expected (in that the thing it is diagnosing is broken code AFAICT). The workaround is you can add the `template` keyword I believe. That said, as this is in the STL, that is perhaps not likely. @sdkrystian : I'd suggest seeing if we can put some level of 'hack' into the compiler to recognize this use and exclude it from diagnosis here. We can't have every libstdc++ build before 13 be broken :/ https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @erichkeane I think this is actually a bug in how I applied [[temp.dep.type] p5](http://eel.is/c++draft/temp.dep.type#5): > A qualified name is dependent if > - it is a _conversion-function-id_ whose _conversion-type-id_ is dependent, or > - its lookup context is dependent and is not the current instantiation, or > - its lookup context is the current instantiation and it is `operator=`, or > - its lookup context is the current instantiation and has at least one > dependent base class, and qualified name lookup for the name finds nothing. [[basic.lookup.qual.general] p3](http://eel.is/c++draft/basic.lookup.qual.general#3) states: > [...] Unless otherwise specified, a qualified name undergoes qualified name > lookup in its lookup context from the point where it appears unless the > lookup context either is dependent and is not the current instantiation or is > not a class or class template. [...] In the case of `this->operator=`, even though the _name_ is dependent, it will still be looked up in the template definition context because the lookup context is the current instantiation (as well as in the template instantiation context). If a declaration of a template is found, then `<` will be interpreted as the start of a template argument list. I'll have a fix ready shortly. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
erichkeane wrote: > @erichkeane I think this is actually a bug in how I applied [[temp.dep.type] > p5](http://eel.is/c++draft/temp.dep.type#5): > > > A qualified name is dependent if > > > > * it is a _conversion-function-id_ whose _conversion-type-id_ is dependent, > > or > > * its lookup context is dependent and is not the current instantiation, or > > * its lookup context is the current instantiation and it is `operator=`, or > > * its lookup context is the current instantiation and has at least one > > dependent base class, and qualified name lookup for the name finds nothing. > > [[basic.lookup.qual.general] > p3](http://eel.is/c++draft/basic.lookup.qual.general#3) states: > > > [...] Unless otherwise specified, a qualified name undergoes qualified name > > lookup in its lookup context from the point where it appears unless the > > lookup context either is dependent and is not the current instantiation or > > is not a class or class template. [...] > > In the case of `this->operator=`, even though the _name_ is dependent, it > will still be looked up in the template definition context because the lookup > context is the current instantiation (as well as in the template > instantiation context). If a declaration of a template is found, then `<` > will be interpreted as the start of a template argument list. > > ~I'll have a fix ready shortly.~ I have [a > fix](https://github.com/llvm/llvm-project/pull/90999) ready. Ah! Good catch! I'll review that now. I'm REALLY glad we wont have to do some hackery to make it work. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @zmodem Fixed in 3191e0b52725aa17651e38d26284386f3ea64eb6 https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
dyung wrote: > @zmodem Fixed in > [3191e0b](https://github.com/llvm/llvm-project/commit/3191e0b52725aa17651e38d26284386f3ea64eb6) This change also seems to fix the small example I posted, I'll double check it against our original source code. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @dyung Yup, that is expected https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
zmodem wrote: > @zmodem Fixed in 3191e0b Thanks! In the meantime, we're finding a number of legitimate bugs with this. Pretty cool :) https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
bgra8 wrote: @sdkrystian we've bisected a clang crash to this revision. The code on which it crashes is (`repro.cc`): ``` template struct a { template auto c(b d) -> decltype(operator=(d)); void operator()(); }; a e; void f() { e(); } ``` Compilation command: ``` $ clang -std=gnu++20 -c repro.cc -o /tmp/out repro.cc:6:12: error: cannot compile this l-value expression yet 6 | void f() { e(); } |^ Stack dump: 0. Program arguments: clang -std=gnu++20 -c repro.cc -o /tmp/out 1. parser at end of file 2. repro.cc:6:6: LLVM IR generation of declaration 'f' 3. repro.cc:6:6: Generating code for declaration 'f' #0 0x55ca8e2a92f8 llvm::sys::RunSignalHandlers() (clang+0x80a92f8) #1 0x55ca8e270d16 CrashRecoverySignalHandler(int) (clang+0x8070d16) #2 0x7fa7bc971e80 __restore_rt (/usr/lib64/libpthread.so.0+0x14e80) #3 0x55ca893cb1c7 arrangeFreeFunctionLikeCall(clang::CodeGen::CodeGenTypes&, clang::CodeGen::CodeGenModule&, clang::CodeGen::CallArgList const&, clang::FunctionType const*, unsigned int, bool) (clang+0x31cb1c7) #4 0x55ca89417be8 clang::CodeGen::CodeGenFunction::EmitCall(clang::QualType, clang::CodeGen::CGCallee const&, clang::CallExpr const*, clang::CodeGen::ReturnValueSlot, llvm::Value*) (clang+0x3217be8) #5 0x55ca89416bed clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*, clang::CodeGen::ReturnValueSlot) (clang+0x3216bed) #6 0x55ca8942b1b3 (anonymous namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) (clang+0x322b1b3) #7 0x55ca8941bd50 clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*, bool) (clang+0x321bd50) #8 0x55ca893f83d9 clang::CodeGen::CodeGenFunction::EmitIgnoredExpr(clang::Expr const*) (clang+0x31f83d9) #9 0x55ca894a13a1 clang::CodeGen::CodeGenFunction::EmitStmt(clang::Stmt const*, llvm::ArrayRef) (clang+0x32a13a1) #10 0x55ca894af0d1 clang::CodeGen::CodeGenFunction::EmitCompoundStmtWithoutScope(clang::CompoundStmt const&, bool, clang::CodeGen::AggValueSlot) (clang+0x32af0d1) #11 0x55ca8970c66b clang::CodeGen::CodeGenFunction::GenerateCode(clang::GlobalDecl, llvm::Function*, clang::CodeGen::CGFunctionInfo const&) (clang+0x350c66b) #12 0x55ca8972e3b7 clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl, llvm::GlobalValue*) (clang+0x352e3b7) #13 0x55ca897275d5 clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, llvm::GlobalValue*) (clang+0x35275d5) #14 0x55ca8972b3ba clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) (clang+0x352b3ba) #15 0x55ca89726590 clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) (clang+0x3526590) #16 0x55ca89833edc (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) (clang+0x3633edc) #17 0x55ca893bf6ec clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) (clang+0x31bf6ec) #18 0x55ca8a1f4a19 clang::ParseAST(clang::Sema&, bool, bool) (clang+0x3ff4a19) #19 0x55ca89f6bd3a clang::FrontendAction::Execute() (clang+0x3d6bd3a) #20 0x55ca89ee56e4 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (clang+0x3ce56e4) #21 0x55ca8909b6cf clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (clang+0x2e9b6cf) #22 0x55ca89090bec cc1_main(llvm::ArrayRef, char const*, void*) (clang+0x2e90bec) #23 0x55ca8908dc26 ExecuteCC1Tool(llvm::SmallVectorImpl&, llvm::ToolContext const&) (clang+0x2e8dc26) #24 0x55ca8a06b99e void llvm::function_ref::callback_fn>, std::__u::basic_string, std::__u::allocator>*, bool*) const::$_0>(long) (clang+0x3e6b99e) #25 0x55ca8e270aef llvm::CrashRecoveryContext::RunSafely(llvm::function_ref) (clang+0x8070aef) #26 0x55ca8a06b17a clang::driver::CC1Command::Execute(llvm::ArrayRef>, std::__u::basic_string, std::__u::allocator>*, bool*) const (clang+0x3e6b17a) #27 0x55ca8a02d77d clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (clang+0x3e2d77d) #28 0x55ca8a02da2f clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl>&, bool) const (clang+0x3e2da2f) #29 0x55ca8a04aed0 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl>&) (clang+0x3e4aed0) #30 0x55ca8908d224 clang_main(int, char**, llvm::ToolContext const&) (clang+0x2e8d224) #31 0x55ca8908b6f4 main (clang+0x2e8b6f4) #32 0x7fa7bc8023d4 __libc_start_main (/usr/lib64/libc.so.6+0x613d4) #33 0x55ca8908b62a _start (clang+0x2e8b62a) clang: error: clang frontend command failed with exit code 139 (use -v to see invocation) ``` https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @bgra8 Reduced to ```cpp template struct A { template auto f(U u) -> decltype(operator=(u)); }; template struct A; ``` Seems like `f` is being set as invalid without any diagnostic... I think I can fix this quickly https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: Looks like `ActOnDependentIdExpression` tries to build a `DependentScopeDeclRefExpr`, which fails because `BuildDependentDeclRefExpr` returns `ExprError()` if no qualifier is present. I'll open a PR shortly. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
alexfh wrote: This commit also breaks the following seemingly valid code (https://gcc.godbolt.org/z/1azKEKh6K): ``` template class Base { public: Base& operator=(int) { return *this; } class Inner; }; template class Base::Inner : public Base { public: using Base::operator=; }; void f() { Base::Inner w; w = 7; } ``` (no further commits have fixed this so far, IIUC) https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @bgra8 see #91498 https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
alexfh wrote: > @bgra8 see #91498 Can you check if it also fixes https://github.com/llvm/llvm-project/pull/90152#issuecomment-2100932093 ? Or should it? https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @alexfh It won't fix the example in your comment (but I have a separate fix for it) https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
alexfh wrote: Great! Thanks for the prompt action! https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @alexfh see #91503 https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
dyung wrote: Hi @sdkrystian, we were fixing up some internal codebases which this change exposed problems with but noticed that this slightly tweaked example of one doesn't seem to trigger an error even though it seems like it should with your change: ```c++ enum BK {}; template class COneSet { typedef COneSet self; void Swap(); BK m_bk; }; template void COneSet::Swap() { self pSetOther; SWAP(pSetOther); // Still calls the undeclared function SWAP } ``` https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent
sdkrystian wrote: @dyung Per [[temp.dep.type] p10.11](http://eel.is/c++draft/temp.dep.type#10.11): > A type is dependent if it is > - [...] > - denoted by a _simple-template-id_ in which either the template name is a > template parameter or any of the template arguments is a dependent type or an > expression that is type-dependent or value-dependent or is a pack expansion, > - [...] In this case, `self` is a typedef for `COneSet` (the injected-class-name of the class template). When used as a _type-name_, the injected-class-name of a class template is equivalent to a _simple-template-id_ with the template arguments of the template parameters of that template. Despite that type _being_ the current instantiation, it is still considered to be a dependent type (name lookup has special rules when the lookup context is dependent _and_ is the current instantiation). Since `pSetOther` names a declaration that has a dependent type, `SWAP(pSetOther)` is a dependent call. https://github.com/llvm/llvm-project/pull/90152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits