[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
@@ -42,7 +41,11 @@
#include
#include
-namespace clang::clangd {
+namespace clang {
+
+class Module;
kadircet wrote:
yes that's mostly something we try in clangd, but fair point, feel free to
ignore this one.
i was mostly pushing because this is just a leaf hence having that dependency
won't affect compile times drastically, and we'll probably keep pulling it
through other headers anyway. (e.g. even after this change, we still have
`clangd/SourceCode.h -> clang/Lex/HeaderSearch.h -> clang/Lex/DirectoryLookup.h
-> clang/Lex/ModuleMap.h -> clang/Basic/Module.h, hence this isn't really
changing much in practice, while hiding the dependency)
https://github.com/llvm/llvm-project/pull/93417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
@@ -42,7 +41,11 @@
#include
#include
-namespace clang::clangd {
+namespace clang {
+
+class Module;
kadircet wrote:
this is not an "unnecessary" include, clangd requires a declaration of
`clang::Module` in this file (even if incomplete) and we try not to have
forward declarations.
what's the motivation behind this change exactly?
https://github.com/llvm/llvm-project/pull/93417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/93079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/93079
From 98ae27a0d303252a23891b204df18112a846f661 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access by not printing target parameter range when there's a
pack in the function parameters.
Fixes https://github.com/llvm/llvm-project/issues/93076.
Fixes https://github.com/llvm/llvm-project/issues/76354.
Fixes https://github.com/llvm/llvm-project/issues/70191.
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/lib/Sema/SemaOverload.cpp | 13 +++--
clang/test/SemaCXX/overload-template.cpp | 10 ++
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 825e91876ffce..81b8d42aaa84e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -734,7 +734,6 @@ Bug Fixes to C++ Support
from being explicitly specialized for a given implicit instantiation of the
class template.
- Fixed a crash when ``this`` is used in a dependent class scope function
template specialization
that instantiates to a static member function.
-
- Fix crash when inheriting from a cv-qualified type. Fixes #GH35603
- Fix a crash when the using enum declaration uses an anonymous enumeration.
Fixes (#GH86790).
- Handled an edge case in ``getFullyPackExpandedSize`` so that we now avoid a
false-positive diagnostic. (#GH84220)
@@ -796,6 +795,8 @@ Bug Fixes to C++ Support
Fixes (#GH91308).
- Fix a crash caused by a regression in the handling of ``source_location``
in dependent contexts. Fixes (#GH92680).
+- Fixed a crash when diagnosing failed conversions involving template parameter
+ packs. (#GH93076)
Bug Fixes to AST Handling
^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..61d3c1633a2b7 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DependenceFlags.h"
@@ -11301,8 +11302,16 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+
+ // FIXME: In presence of parameter packs we can't determine parameter range
+ // reliably, as we don't have access to instantiation.
+ bool HasParamPack =
+ llvm::any_of(Fn->parameters().take_front(I), [](const ParmVarDecl *Parm)
{
+return Parm->isParameterPack();
+ });
+ if (!isObjectArgument && !HasParamPack)
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..3277a17e5e450 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,13 @@ namespace overloadCheck{
}
}
#endif
+
+namespace GH93076 {
+template int b(a..., int); // expected-note-re 3 {{candidate
function template not viable: no known conversion from 'int ()' to 'int' for
{{.*}} argument}}
+int d() {
+ (void)b(0, 0, d); // expected-error {{no matching function for
call to 'b'}}
+ (void)b(0, d, 0); // expected-error {{no matching function for
call to 'b'}}
+ (void)b(d, 0, 0); // expected-error {{no matching function for
call to 'b'}}
+ return 0;
+ }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/93079
From d133b1bf63ab8c5408497ef4c2d2d629ebcff8eb Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access by not printing target parameter range when there's a
pack in the function parameters.
Fixes https://github.com/llvm/llvm-project/issues/93076.
Fixes https://github.com/llvm/llvm-project/issues/76354.
Fixes https://github.com/llvm/llvm-project/issues/70191.
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/lib/Sema/SemaOverload.cpp | 11 +--
clang/test/SemaCXX/overload-template.cpp | 10 ++
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 825e91876ffce..81b8d42aaa84e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -734,7 +734,6 @@ Bug Fixes to C++ Support
from being explicitly specialized for a given implicit instantiation of the
class template.
- Fixed a crash when ``this`` is used in a dependent class scope function
template specialization
that instantiates to a static member function.
-
- Fix crash when inheriting from a cv-qualified type. Fixes #GH35603
- Fix a crash when the using enum declaration uses an anonymous enumeration.
Fixes (#GH86790).
- Handled an edge case in ``getFullyPackExpandedSize`` so that we now avoid a
false-positive diagnostic. (#GH84220)
@@ -796,6 +795,8 @@ Bug Fixes to C++ Support
Fixes (#GH91308).
- Fix a crash caused by a regression in the handling of ``source_location``
in dependent contexts. Fixes (#GH92680).
+- Fixed a crash when diagnosing failed conversions involving template parameter
+ packs. (#GH93076)
Bug Fixes to AST Handling
^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..86e869c7c72ff 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DependenceFlags.h"
@@ -11301,8 +11302,14 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+
+ // FIXME: In presence of parameter packs we can't determine parameter range
+ // reliably, as we don't have access to instantiation.
+ bool HasParamPack = llvm::any_of(Fn->parameters().take_front(I),
+ [](const ParmVarDecl *Parm) { return Parm->isParameterPack();
});
+ if (!isObjectArgument && !HasParamPack)
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..3277a17e5e450 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,13 @@ namespace overloadCheck{
}
}
#endif
+
+namespace GH93076 {
+template int b(a..., int); // expected-note-re 3 {{candidate
function template not viable: no known conversion from 'int ()' to 'int' for
{{.*}} argument}}
+int d() {
+ (void)b(0, 0, d); // expected-error {{no matching function for
call to 'b'}}
+ (void)b(0, d, 0); // expected-error {{no matching function for
call to 'b'}}
+ (void)b(d, 0, 0); // expected-error {{no matching function for
call to 'b'}}
+ return 0;
+ }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/93079
From 9c691ab41ba500c1962bf9d63de86b65f184f047 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access by not printing target parameter range when there's a
pack in the function parameters.
Fixes https://github.com/llvm/llvm-project/issues/93076.
Fixes https://github.com/llvm/llvm-project/issues/76354.
Fixes https://github.com/llvm/llvm-project/issues/70191.
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/lib/Sema/SemaOverload.cpp | 11 +--
clang/test/SemaCXX/overload-template.cpp | 3 +++
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 45676a02b760b..023af70572306 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -730,7 +730,6 @@ Bug Fixes to C++ Support
from being explicitly specialized for a given implicit instantiation of the
class template.
- Fixed a crash when ``this`` is used in a dependent class scope function
template specialization
that instantiates to a static member function.
-
- Fix crash when inheriting from a cv-qualified type. Fixes #GH35603
- Fix a crash when the using enum declaration uses an anonymous enumeration.
Fixes (#GH86790).
- Handled an edge case in ``getFullyPackExpandedSize`` so that we now avoid a
false-positive diagnostic. (#GH84220)
@@ -790,6 +789,8 @@ Bug Fixes to C++ Support
Fixes (#GH87210), (GH89541).
- Clang no longer tries to check if an expression is immediate-escalating in
an unevaluated context.
Fixes (#GH91308).
+- Fixed a crash when diagnosing failed conversions involving template parameter
+ packs. (#GH93076)
Bug Fixes to AST Handling
^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..86e869c7c72ff 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DependenceFlags.h"
@@ -11301,8 +11302,14 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+
+ // FIXME: In presence of parameter packs we can't determine parameter range
+ // reliably, as we don't have access to instantiation.
+ bool HasParamPack = llvm::any_of(Fn->parameters().take_front(I),
+ [](const ParmVarDecl *Parm) { return Parm->isParameterPack();
});
+ if (!isObjectArgument && !HasParamPack)
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..01cfe87a05831 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,6 @@ namespace overloadCheck{
}
}
#endif
+
+template int b(a...); // expected-note {{candidate function
template not viable: no known conversion from 'int ()' to 'int' for 2nd
argument}}
+int d() { return b(0, d); } // expected-error {{no matching function
for call to 'b'}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
kadircet wrote: @shafik thx for the pointers, i didn't know this came up before and even there were attempted fixes. I can verify that both of those are also fixed with this patch. @knightXun do you plan to proceed with https://github.com/llvm/llvm-project/pull/70280 ? If so I am happy to discard this PR in favor of yours. https://github.com/llvm/llvm-project/pull/93079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/93079
From 6a057ff4b539045ce81e55b63702892496d18a97 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access.
Fixes https://github.com/llvm/llvm-project/issues/93076.
Fixes https://github.com/llvm/llvm-project/issues/76354.
Fixes https://github.com/llvm/llvm-project/issues/70191.
---
clang/lib/Sema/SemaOverload.cpp | 11 +--
clang/test/SemaCXX/overload-template.cpp | 3 +++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..86e869c7c72ff 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DependenceFlags.h"
@@ -11301,8 +11302,14 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+
+ // FIXME: In presence of parameter packs we can't determine parameter range
+ // reliably, as we don't have access to instantiation.
+ bool HasParamPack = llvm::any_of(Fn->parameters().take_front(I),
+ [](const ParmVarDecl *Parm) { return Parm->isParameterPack();
});
+ if (!isObjectArgument && !HasParamPack)
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..01cfe87a05831 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,6 @@ namespace overloadCheck{
}
}
#endif
+
+template int b(a...); // expected-note {{candidate function
template not viable: no known conversion from 'int ()' to 'int' for 2nd
argument}}
+int d() { return b(0, d); } // expected-error {{no matching function
for call to 'b'}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/93079
From f7bdd39714e21ff31b3c5aa6a3a18967cb6fef2c Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH 1/2] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access.
Fixes https://github.com/llvm/llvm-project/issues/93076
---
clang/lib/Sema/SemaOverload.cpp | 5 +++--
clang/test/SemaCXX/overload-template.cpp | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..7465d6d96c20f 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -11301,8 +11301,9 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+ if (!isObjectArgument && I < Fn->getNumParams())
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..01cfe87a05831 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,6 @@ namespace overloadCheck{
}
}
#endif
+
+template int b(a...); // expected-note {{candidate function
template not viable: no known conversion from 'int ()' to 'int' for 2nd
argument}}
+int d() { return b(0, d); } // expected-error {{no matching function
for call to 'b'}}
From 7c5716a726fe0c4a2a3e0ddfe8f992491bd0299d Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Fri, 24 May 2024 10:25:25 +0200
Subject: [PATCH 2/2] add fixme and improve range handling
---
clang/lib/Sema/SemaOverload.cpp | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7465d6d96c20f..86e869c7c72ff 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DependenceFlags.h"
@@ -11302,7 +11303,12 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
SourceRange ToParamRange;
- if (!isObjectArgument && I < Fn->getNumParams())
+
+ // FIXME: In presence of parameter packs we can't determine parameter range
+ // reliably, as we don't have access to instantiation.
+ bool HasParamPack = llvm::any_of(Fn->parameters().take_front(I),
+ [](const ParmVarDecl *Parm) { return Parm->isParameterPack();
});
+ if (!isObjectArgument && !HasParamPack)
ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
@@ -11298,8 +11298,9 @@ static void DiagnoseBadConversion(Sema , OverloadCandidate *Cand, Expr *FromExpr = Conv.Bad.FromExpr; QualType FromTy = Conv.Bad.getFromType(); QualType ToTy = Conv.Bad.getToType(); - SourceRange ToParamRange = - !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() : SourceRange(); + SourceRange ToParamRange; + if (!isObjectArgument && I < Fn->getNumParams()) +ToParamRange = Fn->getParamDecl(I)->getSourceRange(); kadircet wrote: > Though I don't think solving this is complicated sorry I guess I didn't convey full context in my previous message when I just said "it requires propagating extra information in the bad conversion about the parameter location". `Fn` in this context is the template decl itself, hence the packs in the parameters are dependent. hence we can't simply ask how many expansions are there. https://github.com/llvm/llvm-project/pull/93079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
@@ -11298,8 +11298,9 @@ static void DiagnoseBadConversion(Sema , OverloadCandidate *Cand, Expr *FromExpr = Conv.Bad.FromExpr; QualType FromTy = Conv.Bad.getFromType(); QualType ToTy = Conv.Bad.getToType(); - SourceRange ToParamRange = - !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() : SourceRange(); + SourceRange ToParamRange; + if (!isObjectArgument && I < Fn->getNumParams()) +ToParamRange = Fn->getParamDecl(I)->getSourceRange(); kadircet wrote: that's a fair point, I was mostly operating with the assumption of "parameter pack is always the last parameter", but that isn't necessarily true. as you pointed out this patch prevents the OOB access, and won't necessarily do the "right" thing in terms of pointing at the failed conversion. but this patch is still an improvement to today's case of just crashing even before printing the candidate. I think adjusting the parameter range to be accurate in presence of parameter packs is more of a feature request at this point. since (AFAICT) it requires propagating extra information in the bad conversion about the parameter location. so I'd rather move forward with this patch, while filing a feature request for the "ideal" state. WDYT? https://github.com/llvm/llvm-project/pull/93079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/93079
Prevent OOB access.
Fixes https://github.com/llvm/llvm-project/issues/93076
From 7840ea2b16863ee7057f3b1239c59b6be06cbd42 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 22 May 2024 19:37:18 +0200
Subject: [PATCH] [clang][Sema] Fix crash when diagnosing candidates with
parameter packs
Prevent OOB access.
Fixes https://github.com/llvm/llvm-project/issues/93076
---
clang/lib/Sema/SemaOverload.cpp | 5 +++--
clang/test/SemaCXX/overload-template.cpp | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 2eb25237a0de6..c4f85df1ef697 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -11298,8 +11298,9 @@ static void DiagnoseBadConversion(Sema ,
OverloadCandidate *Cand,
Expr *FromExpr = Conv.Bad.FromExpr;
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
- SourceRange ToParamRange =
- !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() :
SourceRange();
+ SourceRange ToParamRange;
+ if (!isObjectArgument && I < Fn->getNumParams())
+ToParamRange = Fn->getParamDecl(I)->getSourceRange();
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
diff --git a/clang/test/SemaCXX/overload-template.cpp
b/clang/test/SemaCXX/overload-template.cpp
index 0fe13c479cce2..01cfe87a05831 100644
--- a/clang/test/SemaCXX/overload-template.cpp
+++ b/clang/test/SemaCXX/overload-template.cpp
@@ -58,3 +58,6 @@ namespace overloadCheck{
}
}
#endif
+
+template int b(a...); // expected-note {{candidate function
template not viable: no known conversion from 'int ()' to 'int' for 2nd
argument}}
+int d() { return b(0, d); } // expected-error {{no matching function
for call to 'b'}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -48,6 +48,9 @@ Major New Features Improvements to clangd -- +- Introduced exmperimental support for C++20 Modules. The experimental support can + be enabled by `-experimental-modules-support` option. kadircet wrote: i think we should also document current limitations here (we can update them as we implement more). https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
kadircet wrote:
i think we should generate a random prefix for the directory, rather than
generating a random pcm name for each individual module under a preamble.
in the end we want to achieve uniqueness per-preamble (even if it's for the
same file)
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,81 @@
+//=== ModuleDependencyScanner.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModuleDependencyScanner.h"
+#include "support/Logger.h"
+
+namespace clang {
+namespace clangd {
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+ std::optional Cmd = CDB.getCompileCommand(FilePath);
+
+ if (!Cmd)
+return std::nullopt;
+
+ using namespace clang::tooling::dependencies;
+
+ llvm::SmallString<128> FilePathDir(FilePath);
+ llvm::sys::path::remove_filename(FilePathDir);
+ DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir));
+
+ llvm::Expected ScanningResult =
+ ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory);
+
+ if (auto E = ScanningResult.takeError()) {
+log("Scanning modules dependencies for {0} failed: {1}", FilePath,
kadircet wrote:
`elog("...", FilePath, std::move(E))`
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
kadircet wrote:
this is already deleted.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,81 @@
+//=== ModuleDependencyScanner.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModuleDependencyScanner.h"
+#include "support/Logger.h"
+
+namespace clang {
+namespace clangd {
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+ std::optional Cmd = CDB.getCompileCommand(FilePath);
+
+ if (!Cmd)
+return std::nullopt;
+
+ using namespace clang::tooling::dependencies;
+
+ llvm::SmallString<128> FilePathDir(FilePath);
+ llvm::sys::path::remove_filename(FilePathDir);
+ DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir));
+
+ llvm::Expected ScanningResult =
+ ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory);
+
+ if (auto E = ScanningResult.takeError()) {
+log("Scanning modules dependencies for {0} failed: {1}", FilePath,
+llvm::toString(std::move(E)));
+return std::nullopt;
+ }
+
+ ModuleDependencyInfo Result;
+
+ if (ScanningResult->Provides) {
+ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath;
kadircet wrote:
nit: maybe move caching to `globalScan` and mark this function as `const` ?
that way `getRequiredModules` can also become a `const` method.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
kadircet wrote:
s/a specific module name/the primary module interface for a specific module
name/
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
+ /// every version of every file!
+ /// TODO: We should find an efficient method to get the
+ /// to map. We can make it either by providing
+ /// a global module dependency scanner to monitor every file. Or we
+ /// can simply require the build systems (or even if the end users)
kadircet wrote:
s/even if/even
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
kadircet wrote:
not sure what this `NOTE` serves. could you explain "why" this should be
avoided (instead of documenting current state of the implementation)?
if you really intent this to be internal, i think you can just make it private,
and use just the public interfaces in the tests (currently there's a single
test that calls `scan` directly, it might as well call `globalScan` with a
single file)
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
kadircet wrote:
i think we can drop this sentence. following todo clearly says we need a more
efficient way
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
kadircet wrote:
`globalScan(vector)`
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
kadircet wrote:
we avoid forward declarations in clangd, especially when they're for working
around dependency cycles.
can you just include "PrerequisiteModules.h" here (see other header for ideas
about breaking the cycle).
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
+ /// every version of every file!
+ /// TODO: We should find an efficient method to get the
+ /// to map. We can make it either by providing
+ /// a global module dependency scanner to monitor every file. Or we
+ /// can simply require the build systems (or even if the end users)
+ /// to provide the map.
+ void globalScan(const std::vector );
kadircet wrote:
nit: `llvm::ArrayRef`
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
kadircet wrote:
not sure if this comment is helpful, possibly even distracting as it makes the
author search for `ScanningAllProjectModules`.
I believe `ModuleDependencyScanner` as it stands here is pretty self-contained
i don't see why we should be referring to others.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
kadircet wrote:
s/interere/interfere/
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
+
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
+
+ ~ModulesBuilder() = default;
kadircet wrote:
again i don't think there's a difference between the implicit vs explicitly
default'd destructor here
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
kadircet wrote:
as things stand today these are not really "cache" files, but rather
"temporary" files, similar to clangd's preambles.
as a result, if clangd process dies unexpectedly, we'll just leak all of these
pcms indefinitely, which is quite expensive for the user.
can you move this to `/tmp/` instead, until we start sharing pcms and have a
reason to use a "persistent" storage?
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
https://github.com/kadircet requested changes to this pull request. thanks a lot and sorry for taking so long. i think this looks great at a high level, mostly asked for some changes in the implementation. the only layering-wise change is who should own `ModulesBuilder` (you can find details in the specific comment thread). apart from that one, i think unittests can benefit from some "virtualizaiton", i believe things become a lot more simpler, if we ignore all the other module related flags that needs to be provided. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
kadircet wrote:
3 slashes here and below
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add modernize-use-std-format check (PR #90397)
kadircet wrote: hi! this seems to be crashing for certain code patterns, https://github.com/llvm/llvm-project/issues/92896 has a minimal reproducer https://github.com/llvm/llvm-project/pull/90397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][CodeComplete] Omit ExplicitObject when completing code (PR #92743)
kadircet wrote: instead of testing this in clangd, can you add tests to `clang/test/CodeCompletion/`? (even better if you can extend `clang/unittests/Sema/CodeCompleteTest.cpp`). as this is a clang feature, and it'd be nice to make sure it doesn't regress at clang-level. https://github.com/llvm/llvm-project/pull/92743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
https://github.com/kadircet approved this pull request. thanks a lot, LGTM! it'd be great if you can also follow up with a change to https://github.com/llvm/clangd-www/blob/main/config.md https://github.com/llvm/llvm-project/pull/87208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -68,24 +68,32 @@ bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter
IgnoreHeaders) {
}
bool mayConsiderUnused(const Inclusion , ParsedAST ,
- const include_cleaner::PragmaIncludes *PI) {
+ const include_cleaner::PragmaIncludes *PI,
+ bool AnalyzeAngledIncludes) {
assert(Inc.HeaderID);
auto HID = static_cast(*Inc.HeaderID);
auto FE = AST.getSourceManager().getFileManager().getFileRef(
AST.getIncludeStructure().getRealPath(HID));
assert(FE);
if (FE->getDir() == AST.getPreprocessor()
- .getHeaderSearchInfo()
- .getModuleMap()
- .getBuiltinDir())
+ .getHeaderSearchInfo()
+ .getModuleMap()
+ .getBuiltinDir())
return false;
if (PI && PI->shouldKeep(*FE))
return false;
// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
// System headers are likely to be standard library headers.
- // Until we have good support for umbrella headers, don't warn about them.
- if (Inc.Written.front() == '<')
-return tooling::stdlib::Header::named(Inc.Written).has_value();
+ // Until we have good support for umbrella headers, don't warn about them
+ // (unless analysis is explicitly enabled).
+ if (Inc.Written.front() == '<') {
+if (tooling::stdlib::Header::named(Inc.Written)) {
+ return true;
+}
+if (!AnalyzeAngledIncludes) {
+ return false;
+}
kadircet wrote:
LLVM style prefers no braces for single line && statement blocks.
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -135,6 +135,21 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\"";
}
+TEST(IncludeCleaner, SystemUnusedHeaders) {
+ auto TU = TestTU::withCode(R"cpp(
+#include
+#include
+SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass
{};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.ExtraArgs = {"-isystem", testPath("system")};
kadircet wrote:
thanks, sgtm
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -135,6 +135,21 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\"";
}
+TEST(IncludeCleaner, SystemUnusedHeaders) {
+ auto TU = TestTU::withCode(R"cpp(
+#include
+#include
+SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass
{};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.ExtraArgs = {"-isystem", testPath("system")};
+ auto AST = TU.build();
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST, true);
kadircet wrote:
> Isn;t this already covered by the GetUnusedHeaders test?
Yes but that one is a more bulky test that checks for multiple pieces playing
nicely together. Whereas this one is more "specific" to the code-path you're
adding.
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -572,32 +572,43 @@ struct FragmentCompiler {
#else
static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
#endif
-auto Filters = std::make_shared>();
-for (auto : F.IgnoreHeader) {
- // Anchor on the right.
- std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
- llvm::Regex CompiledRegex(AnchoredPattern, Flags);
- std::string RegexError;
- if (!CompiledRegex.isValid(RegexError)) {
-diag(Warning,
- llvm::formatv("Invalid regular expression '{0}': {1}",
- *HeaderPattern, RegexError)
- .str(),
- HeaderPattern.Range);
-continue;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
+ Filters = std::make_shared>();
+ for (auto : F.IgnoreHeader) {
+// Anchor on the right.
+std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+std::string RegexError;
+if (!CompiledRegex.isValid(RegexError)) {
+ diag(Warning,
+ llvm::formatv("Invalid regular expression '{0}': {1}",
+ *HeaderPattern, RegexError)
+ .str(),
+ HeaderPattern.Range);
+ continue;
+}
+Filters->push_back(std::move(CompiledRegex));
}
- Filters->push_back(std::move(CompiledRegex));
}
-if (Filters->empty())
+std::optional AnalyzeSystemHeaders;
+if (F.AnalyzeSystemHeaders.has_value())
+ AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders;
kadircet wrote:
> F.AnalyzeSystemHeaders is a std::optional>
Hence RHS is just a `bool`, so I don't understand why we prefer a bool.
But now that I read the logic again it actually makes sense, not for C++23
idioms, but because we want to override the config field only if it was
explicitly set, and inherit from parent config fragment otherwise. Can you add
a comment about that?
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -572,32 +572,43 @@ struct FragmentCompiler {
#else
static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
#endif
-auto Filters = std::make_shared>();
-for (auto : F.IgnoreHeader) {
- // Anchor on the right.
- std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
- llvm::Regex CompiledRegex(AnchoredPattern, Flags);
- std::string RegexError;
- if (!CompiledRegex.isValid(RegexError)) {
-diag(Warning,
- llvm::formatv("Invalid regular expression '{0}': {1}",
- *HeaderPattern, RegexError)
- .str(),
- HeaderPattern.Range);
-continue;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
kadircet wrote:
config compilation isn't really a hot code path, hence i'd lean towards
simplicity. up to you though.
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
kadircet wrote: thanks for the patch! this definitely LGTM at high level. as you folks also pointed out we don't want to surface analysis for angled includes in general, as include-cleaner doesn't have support for system headers yet (we wanted to have something similar to Stdlib includes, which maps certain include suffixes for known system libraries like posix, glibc to their umbrella headers, but never got to it). As a result, turning this on unconditionally would yield a ton of false negatives, which renders analysis useless. But doing that behind a flag, especially in conjunction with `IgnoreHeaders` is a solution that works nicely in practice. https://github.com/llvm/llvm-project/pull/87208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -135,6 +135,21 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\"";
}
+TEST(IncludeCleaner, SystemUnusedHeaders) {
+ auto TU = TestTU::withCode(R"cpp(
+#include
+#include
+SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass
{};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.ExtraArgs = {"-isystem", testPath("system")};
kadircet wrote:
using `-I` instead of `-isystem` might be better here. as in the implementation
we don't really disambiguate between type of the inclusions, just how they're
spelled.
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -572,32 +572,43 @@ struct FragmentCompiler {
#else
static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
#endif
-auto Filters = std::make_shared>();
-for (auto : F.IgnoreHeader) {
- // Anchor on the right.
- std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
- llvm::Regex CompiledRegex(AnchoredPattern, Flags);
- std::string RegexError;
- if (!CompiledRegex.isValid(RegexError)) {
-diag(Warning,
- llvm::formatv("Invalid regular expression '{0}': {1}",
- *HeaderPattern, RegexError)
- .str(),
- HeaderPattern.Range);
-continue;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
+ Filters = std::make_shared>();
+ for (auto : F.IgnoreHeader) {
+// Anchor on the right.
+std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+std::string RegexError;
+if (!CompiledRegex.isValid(RegexError)) {
+ diag(Warning,
+ llvm::formatv("Invalid regular expression '{0}': {1}",
+ *HeaderPattern, RegexError)
+ .str(),
+ HeaderPattern.Range);
+ continue;
+}
+Filters->push_back(std::move(CompiledRegex));
}
- Filters->push_back(std::move(CompiledRegex));
}
-if (Filters->empty())
+std::optional AnalyzeSystemHeaders;
+if (F.AnalyzeSystemHeaders.has_value())
+ AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders;
kadircet wrote:
RHS here is no longer an optional
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -62,15 +64,6 @@ issueIncludeCleanerDiagnostics(ParsedAST ,
llvm::StringRef Code,
const ThreadsafeFS ,
HeaderFilter IgnoreHeader = {});
-/// Affects whether standard library includes should be considered for
-/// removal. This is off by default for now due to implementation limitations:
-/// - macros are not tracked
-/// - symbol names without a unique associated header are not tracked
-/// - references to std-namespaced C types are not properly tracked:
-/// instead of std::size_t -> we see ::size_t ->
-/// FIXME: remove this hack once the implementation is good enough.
-void setIncludeCleanerAnalyzesStdlib(bool B);
kadircet wrote:
completely agreed, thanks for cleaning this up!
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -114,6 +114,7 @@ struct Config {
/// these regexes.
struct {
std::vector> IgnoreHeader;
+ bool AnalyzeSystemHeaders = false;
kadircet wrote:
can you move the comment about regexes near `IgnoreHeader`.
Also I think `AnalyzeAngledIncludes` would be a more fitting name, WDYT?
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -572,32 +572,43 @@ struct FragmentCompiler {
#else
static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
#endif
-auto Filters = std::make_shared>();
-for (auto : F.IgnoreHeader) {
- // Anchor on the right.
- std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
- llvm::Regex CompiledRegex(AnchoredPattern, Flags);
- std::string RegexError;
- if (!CompiledRegex.isValid(RegexError)) {
-diag(Warning,
- llvm::formatv("Invalid regular expression '{0}': {1}",
- *HeaderPattern, RegexError)
- .str(),
- HeaderPattern.Range);
-continue;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
kadircet wrote:
nit: you can keep this part the same, and just replace `if (Filters->emtpy())
return;` with `Filters.reset();`
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -254,6 +254,10 @@ struct Fragment {
/// unused or missing. These can match any suffix of the header file in
/// question.
std::vector> IgnoreHeader;
+
+ /// If false (default), unused system headers will be ignored.
+ /// Standard library headers are analyzed regardless of this option.
+ std::optional> AnalyzeSystemHeaders;
kadircet wrote:
Same renaming suggestion here for `s/SystemHeaders/AngledIncludes`
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
@@ -135,6 +135,21 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\"";
}
+TEST(IncludeCleaner, SystemUnusedHeaders) {
+ auto TU = TestTU::withCode(R"cpp(
+#include
+#include
+SystemClass x;
+ )cpp");
+ TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass
{};");
+ TU.AdditionalFiles["system/system_unused.h"] = guard("");
+ TU.ExtraArgs = {"-isystem", testPath("system")};
+ auto AST = TU.build();
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST, true);
kadircet wrote:
can you also add a negative test? (i.e. when `AnalyzeSystemHeaders = false`,
`UnusedIncludes` is empty)
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/87208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add config option to allow detection of unused system headers (PR #87208)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/87208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
kadircet wrote:
So my 2cents;
> 1. I know a [previous comment on the
> bug](https://github.com/clangd/clangd/issues/959#issuecomment-998927030)
> stated "We don't show bodies of classes/enums/functions etc by policy", but
> can we consider changing this policy in the more limited case of public
> struct fields and enum members? Like I mentioned in [this
> comment](https://github.com/clangd/clangd/issues/959#issuecomment-2068307365),
> the usefulness/verbosity tradeoff might be different in this subset of
> cases. Also cc @colin-grant-work in case you'd like to weigh in on this
> further.
I feel like a more focused approach could still be preferred. E.g. what about
showing this kind of "summary" only when hovering over the declaration of the
symbol, rather than references? (similar to the way we show size/alignment
info). As I believe showing `struct Foo {}` in hover response, when the cursor
is already at a struct's definition, isn't really useful.
For enabling it more generally, I still have some hesitations, as I am not sure
how useful that interaction is in general. As not all editors let you interact
with hover cards, and just seeing a bunch of field names, without any extra
info, sounds suboptimal to me. I'd still prefer going over those fields via
`Foo{}.^` code completion, which shows all the "public" info with extra context
like "documentation".
> 2. If we're willing to make this change, do we want it unconditionally, or
> behind a new config option in the `Hover` section? My personal opinion is
> that even in the case of a struct/enum with many members this is not
> particularly bothersome and would lean towards making it unconditional, but I
> am of course happy to put it behind a config option if that helps build a
> consensus for this.
As you might've sensed from the previous response, i'd definitely prefer a
config option. As most terminal-based editors don't really have "rich"
hovercard support, and even if we set the default to "verbose", i'd like to
have a way for such people to keep using hover cards without definition eating
up the whole screen estate.
> 3. Regarding the implementation approach, is it fine to add a flag to
> `PrintingPolicy` (which is a clang utility class used in a variety of places)
> for a clangd-specific use case like this? I did it this way because the
> alternative seemed to involve duplicating a bunch of code related to
> decl-printing, but I'm happy to explore alternatives if this is an issue.
I feel like "print only public fields" is too specific for clangd use case, and
probably won't generalize to other callers at all. But I definitely agree with
all the concerns around duplicating code. Looks like we have some
`PrintingCallbacks`, maybe we can have something like `SummarizeTagDecl`, which
enables customizing what to put into the body, when printing a TagDecl in terse
mode?
https://github.com/llvm/llvm-project/pull/89557
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
kadircet wrote: Hi @zibi2, sorry for missing the breakage in the buildbot. ce2f6423f016 should fix it. https://github.com/llvm/llvm-project/pull/88381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Propagate context into stdlib indexing thread (PR #87611)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/87611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Propagate context into stdlib indexing thread (PR #87611)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/87611
From 2b8899a6fd9477dd411f2a89409703a6e16aef2b Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 4 Apr 2024 10:57:44 +0200
Subject: [PATCH] [clangd] Propagate context into stdlib indexing thread
Some FS implementations rely on snapshots available in the context.
---
clang-tools-extra/clangd/ClangdServer.cpp | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5790273d625ef1..1c4c2a79b5c051 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -30,6 +30,7 @@
#include "refactor/Rename.h"
#include "refactor/Tweak.h"
#include "support/Cancellation.h"
+#include "support/Context.h"
#include "support/Logger.h"
#include "support/MemoryTree.h"
#include "support/ThreadsafeFS.h"
@@ -112,7 +113,12 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
// Index outlives TUScheduler (declared first)
FIndex(FIndex),
// shared_ptr extends lifetime
- Stdlib(Stdlib)]() mutable {
+ Stdlib(Stdlib),
+ // We have some FS implementations that rely on information in
+ // the context.
+ Ctx(Context::current().clone())]() mutable {
+ // Make sure we install the context into current thread.
+ WithContext C(std::move(Ctx));
clang::noteBottomOfStack();
IndexFileIn IF;
IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/88381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/88381
From a99cb340ec6ede680a2b4d278b48d6062c42bef5 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 11 Apr 2024 14:02:43 +0200
Subject: [PATCH] [clangd] Use TargetOpts from preamble when building ASTs
Building ASTs with compile flags that are incompatible to the ones used
for the Preamble are not really supported by clang and can trigger
crashes.
In an ideal world, we should be re-using not only TargetOpts, but the
full ParseInputs from the Preamble to prevent such failures.
Unfortunately current contracts of ThreadSafeFS makes this a non-safe
change for certain implementations. As there are no guarantees that the
same ThreadSafeFS is going to be valid in the Context::current() we're
building the AST in.
---
clang-tools-extra/clangd/Preamble.cpp | 7
clang-tools-extra/clangd/Preamble.h | 5 +++
.../clangd/unittests/CodeCompleteTests.cpp| 25
.../clangd/unittests/ParsedASTTests.cpp | 39 +++
4 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index f181c7befec156..d5818e0ca309b0 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -700,6 +700,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
Result->Marks = CapturedInfo.takeMarks();
Result->StatCache = StatCache;
Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+Result->TargetOpts = CI.TargetOpts;
if (PreambleCallback) {
trace::Span Tracer("Running PreambleCallback");
auto Ctx = CapturedInfo.takeLife();
@@ -913,6 +914,12 @@ PreamblePatch
PreamblePatch::createMacroPatch(llvm::StringRef FileName,
}
void PreamblePatch::apply(CompilerInvocation ) const {
+ // Make sure the compilation uses same target opts as the preamble. Clang has
+ // no guarantees around using arbitrary options when reusing PCHs, and
+ // different target opts can result in crashes, see
+ // ParsedASTTest.PreambleWithDifferentTarget.
+ CI.TargetOpts = Baseline->TargetOpts;
+
// No need to map an empty file.
if (PatchContents.empty())
return;
diff --git a/clang-tools-extra/clangd/Preamble.h
b/clang-tools-extra/clangd/Preamble.h
index 37da3833748a9c..160b884beb56bb 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
#include "clang-include-cleaner/Record.h"
#include "support/Path.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetOptions.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Lexer.h"
@@ -97,6 +98,10 @@ struct PreambleData {
// Version of the ParseInputs this preamble was built from.
std::string Version;
tooling::CompileCommand CompileCommand;
+ // Target options used when building the preamble. Changes in target can
cause
+ // crashes when deserializing preamble, this enables consumers to use the
+ // same target (without reparsing CompileCommand).
+ std::shared_ptr TargetOpts = nullptr;
PrecompiledPreamble Preamble;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 8fbac73cb653bc..96d1ee1f0add73 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4160,7 +4160,32 @@ TEST(CompletionTest, DoNotCrash) {
auto Completions = completions(Case);
}
}
+TEST(CompletionTest, PreambleFromDifferentTarget) {
+ constexpr std::string_view PreambleTarget = "x86_64";
+ constexpr std::string_view Contents =
+ "int foo(int); int num; int num2 = foo(n^";
+ Annotations Test(Contents);
+ auto TU = TestTU::withCode(Test.code());
+ TU.ExtraArgs.emplace_back("-target");
+ TU.ExtraArgs.emplace_back(PreambleTarget);
+ auto Preamble = TU.preamble();
+ ASSERT_TRUE(Preamble);
+ // Switch target to wasm.
+ TU.ExtraArgs.pop_back();
+ TU.ExtraArgs.emplace_back("wasm32");
+
+ MockFS FS;
+ auto Inputs = TU.inputs(FS);
+ auto Result = codeComplete(testPath(TU.Filename), Test.point(),
+ Preamble.get(), Inputs, {});
+ auto Signatures =
+ signatureHelp(testPath(TU.Filename), Test.point(), *Preamble, Inputs,
{});
+
+ // Make sure we don't crash.
+ EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
+ EXPECT_THAT(Signatures.signatures, Not(testing::IsEmpty()));
+}
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 500b72b9b327a0..4bb76cd6ab8304 100644
---
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
@@ -112,7 +112,10 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
// Index outlives TUScheduler (declared first)
FIndex(FIndex),
// shared_ptr extends lifetime
- Stdlib(Stdlib)]() mutable {
+ Stdlib(Stdlib),
+ // We have some FS implementations that rely on infomration in
+ // the context.
+ Ctx(Context::current().clone())]() mutable {
kadircet wrote:
oops that shouldn't be here, commit from a different branch messed things up.
(FWIW, it's part of https://github.com/llvm/llvm-project/pull/87611)
https://github.com/llvm/llvm-project/pull/88381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/88381
From d747d817b6df04c8b93b7f89379448e467f0a0cd Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 11 Apr 2024 14:02:43 +0200
Subject: [PATCH] [clangd] Use TargetOpts from preamble when building ASTs
Building ASTs with compile flags that are incompatible to the ones used
for the Preamble are not really supported by clang and can trigger
crashes.
In an ideal world, we should be re-using not only TargetOpts, but the
full ParseInputs from the Preamble to prevent such failures.
Unfortunately current contracts of ThreadSafeFS makes this a non-safe
change for certain implementations. As there are no guarantees that the
same ThreadSafeFS is going to be valid in the Context::current() we're
building the AST in.
---
clang-tools-extra/clangd/Preamble.cpp | 7
clang-tools-extra/clangd/Preamble.h | 5 +++
.../clangd/unittests/CodeCompleteTests.cpp| 25
.../clangd/unittests/ParsedASTTests.cpp | 39 +++
4 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index f181c7befec156..d5818e0ca309b0 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -700,6 +700,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
Result->Marks = CapturedInfo.takeMarks();
Result->StatCache = StatCache;
Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+Result->TargetOpts = CI.TargetOpts;
if (PreambleCallback) {
trace::Span Tracer("Running PreambleCallback");
auto Ctx = CapturedInfo.takeLife();
@@ -913,6 +914,12 @@ PreamblePatch
PreamblePatch::createMacroPatch(llvm::StringRef FileName,
}
void PreamblePatch::apply(CompilerInvocation ) const {
+ // Make sure the compilation uses same target opts as the preamble. Clang has
+ // no guarantees around using arbitrary options when reusing PCHs, and
+ // different target opts can result in crashes, see
+ // ParsedASTTest.PreambleWithDifferentTarget.
+ CI.TargetOpts = Baseline->TargetOpts;
+
// No need to map an empty file.
if (PatchContents.empty())
return;
diff --git a/clang-tools-extra/clangd/Preamble.h
b/clang-tools-extra/clangd/Preamble.h
index 37da3833748a9c..160b884beb56bb 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
#include "clang-include-cleaner/Record.h"
#include "support/Path.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetOptions.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Lexer.h"
@@ -97,6 +98,10 @@ struct PreambleData {
// Version of the ParseInputs this preamble was built from.
std::string Version;
tooling::CompileCommand CompileCommand;
+ // Target options used when building the preamble. Changes in target can
cause
+ // crashes when deserializing preamble, this enables consumers to use the
+ // same target (without reparsing CompileCommand).
+ std::shared_ptr TargetOpts = nullptr;
PrecompiledPreamble Preamble;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 8fbac73cb653bc..96d1ee1f0add73 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4160,7 +4160,32 @@ TEST(CompletionTest, DoNotCrash) {
auto Completions = completions(Case);
}
}
+TEST(CompletionTest, PreambleFromDifferentTarget) {
+ constexpr std::string_view PreambleTarget = "x86_64";
+ constexpr std::string_view Contents =
+ "int foo(int); int num; int num2 = foo(n^";
+ Annotations Test(Contents);
+ auto TU = TestTU::withCode(Test.code());
+ TU.ExtraArgs.emplace_back("-target");
+ TU.ExtraArgs.emplace_back(PreambleTarget);
+ auto Preamble = TU.preamble();
+ ASSERT_TRUE(Preamble);
+ // Switch target to wasm.
+ TU.ExtraArgs.pop_back();
+ TU.ExtraArgs.emplace_back("wasm32");
+
+ MockFS FS;
+ auto Inputs = TU.inputs(FS);
+ auto Result = codeComplete(testPath(TU.Filename), Test.point(),
+ Preamble.get(), Inputs, {});
+ auto Signatures =
+ signatureHelp(testPath(TU.Filename), Test.point(), *Preamble, Inputs,
{});
+
+ // Make sure we don't crash.
+ EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
+ EXPECT_THAT(Signatures.signatures, Not(testing::IsEmpty()));
+}
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 500b72b9b327a0..4bb76cd6ab8304 100644
---
[clang-tools-extra] [clangd] Propagate context into stdlib indexing thread (PR #87611)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/87611
From a5cc9640c05ad06bff6ec28c958669df4b7e8214 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 4 Apr 2024 10:57:44 +0200
Subject: [PATCH] [clangd] Propagate context into stdlib indexing thread
Some FS implementations rely on snapshots available in the context.
---
clang-tools-extra/clangd/ClangdServer.cpp | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5790273d625ef1..1c4c2a79b5c051 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -30,6 +30,7 @@
#include "refactor/Rename.h"
#include "refactor/Tweak.h"
#include "support/Cancellation.h"
+#include "support/Context.h"
#include "support/Logger.h"
#include "support/MemoryTree.h"
#include "support/ThreadsafeFS.h"
@@ -112,7 +113,12 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
// Index outlives TUScheduler (declared first)
FIndex(FIndex),
// shared_ptr extends lifetime
- Stdlib(Stdlib)]() mutable {
+ Stdlib(Stdlib),
+ // We have some FS implementations that rely on information in
+ // the context.
+ Ctx(Context::current().clone())]() mutable {
+ // Make sure we install the context into current thread.
+ WithContext C(std::move(Ctx));
clang::noteBottomOfStack();
IndexFileIn IF;
IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Propagate context into stdlib indexing thread (PR #87611)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/87611
From cd8993935f5c5ff8a46d2741ef8c76348ab8e868 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 4 Apr 2024 10:57:44 +0200
Subject: [PATCH] [clangd] Propagate context into stdlib indexing thread
Some FS implementations rely on snapshots available in the context.
---
clang-tools-extra/clangd/ClangdServer.cpp | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5790273d625ef1..23fe6db57fa790 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -30,6 +30,7 @@
#include "refactor/Rename.h"
#include "refactor/Tweak.h"
#include "support/Cancellation.h"
+#include "support/Context.h"
#include "support/Logger.h"
#include "support/MemoryTree.h"
#include "support/ThreadsafeFS.h"
@@ -112,7 +113,12 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
// Index outlives TUScheduler (declared first)
FIndex(FIndex),
// shared_ptr extends lifetime
- Stdlib(Stdlib)]() mutable {
+ Stdlib(Stdlib),
+ // We have some FS implementations that rely on infomration in
+ // the context.
+ Ctx(Context::current().clone())]() mutable {
+ // Make sure we install the context into current thread.
+ WithContext C(std::move(Ctx));
clang::noteBottomOfStack();
IndexFileIn IF;
IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/88381
From 233f413b5921ff23c171fa5ada5623ad1a8e3d82 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 4 Apr 2024 10:57:44 +0200
Subject: [PATCH 1/2] [clangd] Propagate context into stdlib indexing thread
Some FS implementations rely on snapshots available in the context.
---
clang-tools-extra/clangd/ClangdServer.cpp | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5790273d625ef14..4eeed83b3e37516 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -112,7 +112,10 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
// Index outlives TUScheduler (declared first)
FIndex(FIndex),
// shared_ptr extends lifetime
- Stdlib(Stdlib)]() mutable {
+ Stdlib(Stdlib),
+ // We have some FS implementations that rely on infomration in
+ // the context.
+ Ctx(Context::current().clone())]() mutable {
clang::noteBottomOfStack();
IndexFileIn IF;
IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
From d396ab92ba577bdda05e43acb069de4aad60b083 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 11 Apr 2024 14:02:43 +0200
Subject: [PATCH 2/2] [clangd] Use TargetOpts from preamble when building ASTs
Building ASTs with compile flags that are incompatible to the ones used
for the Preamble are not really supported by clang and can trigger
crashes.
In an ideal world, we should be re-using not only TargetOpts, but the
full ParseInputs from the Preamble to prevent such failures.
Unfortunately current contracts of ThreadSafeFS makes this a non-safe
change for certain implementations. As there are no guarantees that the
same ThreadSafeFS is going to be valid in the Context::current() we're
building the AST in.
---
clang-tools-extra/clangd/Preamble.cpp | 7
clang-tools-extra/clangd/Preamble.h | 5 +++
.../clangd/unittests/CodeCompleteTests.cpp| 25
.../clangd/unittests/ParsedASTTests.cpp | 39 +++
4 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index f181c7befec156a..d5818e0ca309b03 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -700,6 +700,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
Result->Marks = CapturedInfo.takeMarks();
Result->StatCache = StatCache;
Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+Result->TargetOpts = CI.TargetOpts;
if (PreambleCallback) {
trace::Span Tracer("Running PreambleCallback");
auto Ctx = CapturedInfo.takeLife();
@@ -913,6 +914,12 @@ PreamblePatch
PreamblePatch::createMacroPatch(llvm::StringRef FileName,
}
void PreamblePatch::apply(CompilerInvocation ) const {
+ // Make sure the compilation uses same target opts as the preamble. Clang has
+ // no guarantees around using arbitrary options when reusing PCHs, and
+ // different target opts can result in crashes, see
+ // ParsedASTTest.PreambleWithDifferentTarget.
+ CI.TargetOpts = Baseline->TargetOpts;
+
// No need to map an empty file.
if (PatchContents.empty())
return;
diff --git a/clang-tools-extra/clangd/Preamble.h
b/clang-tools-extra/clangd/Preamble.h
index 37da3833748a9c6..160b884beb56bb7 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
#include "clang-include-cleaner/Record.h"
#include "support/Path.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetOptions.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Lexer.h"
@@ -97,6 +98,10 @@ struct PreambleData {
// Version of the ParseInputs this preamble was built from.
std::string Version;
tooling::CompileCommand CompileCommand;
+ // Target options used when building the preamble. Changes in target can
cause
+ // crashes when deserializing preamble, this enables consumers to use the
+ // same target (without reparsing CompileCommand).
+ std::shared_ptr TargetOpts = nullptr;
PrecompiledPreamble Preamble;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 8fbac73cb653bcc..96d1ee1f0add735 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4160,7 +4160,32 @@ TEST(CompletionTest, DoNotCrash) {
auto Completions = completions(Case);
}
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
kadircet wrote: > Just to confirm I understand the broader context: we only need it when > building the AST because the rest of the code (completions, signature help) > actually already uses a compile command and FS from preamble inputs, right? That was my remembrance, but apparently it was wrong :/ Changed the logic to cover those cases as well && added some tests. https://github.com/llvm/llvm-project/pull/88381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
@@ -768,6 +764,35 @@ TEST(ParsedASTTest, GracefulFailureOnAssemblyFile) {
<< "Should not try to build AST for assembly source file";
}
+TEST(ParsedASTTest, PreambleWithDifferentTarget) {
+ constexpr std::string_view kPreambleTarget = "x86_64";
kadircet wrote:
yeah happy to wait and see if some build bots fail
https://github.com/llvm/llvm-project/pull/88381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
@@ -451,6 +451,7 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs ,
DiagnosticConsumer *DiagConsumer =
IgnoreDiagnostics DropDiags;
if (Preamble) {
+CI->TargetOpts = Preamble->TargetOpts;
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
kadircet wrote:
well, idea also crossed my mind while putting together the patch but wasn't so
sure if that would be the best place. since preamble-patch'ing was mostly for
adjusting the compilation to "patch" changes in the preamble contents.
but considering the fact that this actually needs to be done on all the places
that re-uses a preamble (CodeCompletion, SignatureHelp), I guess PreamblePatch
is a nice common ground (I was leaning towards `prepareCompilerInstance` but
unfortunately that one can't depend on Preamble.h :/)
https://github.com/llvm/llvm-project/pull/88381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Dont apply name-match for non-owning headers (PR #82625)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/82625 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Dont apply name-match for non-owning headers (PR #82625)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/82625
From d5e35aac5bc4103b668766aa10d42bc4dc2c9087 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 22 Feb 2024 15:46:20 +0100
Subject: [PATCH] [include-cleaner] Dont apply name-match for non-owning
headers
---
.../include-cleaner/lib/FindHeaders.cpp | 6 ++
.../unittests/FindHeadersTest.cpp | 19 +++
2 files changed, 25 insertions(+)
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index fd2de6a17ad4a5..7b28d1c252d715 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -275,6 +275,12 @@ llvm::SmallVector headersForSymbol(const Symbol ,
// are already ranked in the stdlib mapping.
if (H.kind() == Header::Standard)
continue;
+// Don't apply name match hints to exporting headers. As they usually have
+// names similar to the original header, e.g. foo_wrapper/foo.h vs
+// foo/foo.h, but shouldn't be preferred (unless marked as the public
+// interface).
+if ((H.Hint & Hints::OriginHeader) == Hints::None)
+ continue;
if (nameMatch(SymbolName, H))
H.Hint |= Hints::PreferredHeader;
}
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 5a2a41b2d99bdd..07302142a13e36 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -628,5 +628,24 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) {
tooling::stdlib::Header::named("")));
}
+TEST_F(HeadersForSymbolTest, ExporterNoNameMatch) {
+ Inputs.Code = R"cpp(
+#include "exporter/foo.h"
+#include "foo_public.h"
+ )cpp";
+ Inputs.ExtraArgs.emplace_back("-I.");
+ // Deliberately named as foo_public to make sure it doesn't get name-match
+ // boost and also gets lexicographically bigger order than "exporter/foo.h".
+ Inputs.ExtraFiles["foo_public.h"] = guard(R"cpp(
+struct foo {};
+ )cpp");
+ Inputs.ExtraFiles["exporter/foo.h"] = guard(R"cpp(
+#include "foo_public.h" // IWYU pragma: export
+ )cpp");
+ buildAST();
+ EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("foo_public.h"),
+ physicalHeader("exporter/foo.h")));
+}
+
} // namespace
} // namespace clang::include_cleaner
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Use TargetOpts from preamble when building ASTs (PR #88381)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/88381
Building ASTs with compile flags that are incompatible to the ones used
for the Preamble are not really supported by clang and can trigger
crashes.
In an ideal world, we should be re-using not only TargetOpts, but the
full ParseInputs from the Preamble to prevent such failures.
Unfortunately current contracts of ThreadSafeFS makes this a non-safe
change for certain implementations. As there are no guarantees that the
same ThreadSafeFS is going to be valid in the Context::current() we're
building the AST in.
From d23a0e1da78abe76f2f178ed4f97927147682ec4 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 11 Apr 2024 14:02:43 +0200
Subject: [PATCH] [clangd] Use TargetOpts from preamble when building ASTs
Building ASTs with compile flags that are incompatible to the ones used
for the Preamble are not really supported by clang and can trigger
crashes.
In an ideal world, we should be re-using not only TargetOpts, but the
full ParseInputs from the Preamble to prevent such failures.
Unfortunately current contracts of ThreadSafeFS makes this a non-safe
change for certain implementations. As there are no guarantees that the
same ThreadSafeFS is going to be valid in the Context::current() we're
building the AST in.
---
clang-tools-extra/clangd/ParsedAST.cpp| 1 +
clang-tools-extra/clangd/Preamble.cpp | 1 +
clang-tools-extra/clangd/Preamble.h | 5 +++
.../clangd/unittests/ParsedASTTests.cpp | 39 +++
4 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 3ff759415f7c8b..b6e784db4719fe 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -451,6 +451,7 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs ,
DiagnosticConsumer *DiagConsumer =
IgnoreDiagnostics DropDiags;
if (Preamble) {
+CI->TargetOpts = Preamble->TargetOpts;
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
}
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index f181c7befec156..d579e90adc49df 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -700,6 +700,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
Result->Marks = CapturedInfo.takeMarks();
Result->StatCache = StatCache;
Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+Result->TargetOpts = CI.TargetOpts;
if (PreambleCallback) {
trace::Span Tracer("Running PreambleCallback");
auto Ctx = CapturedInfo.takeLife();
diff --git a/clang-tools-extra/clangd/Preamble.h
b/clang-tools-extra/clangd/Preamble.h
index 37da3833748a9c..160b884beb56bb 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -30,6 +30,7 @@
#include "clang-include-cleaner/Record.h"
#include "support/Path.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetOptions.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Lexer.h"
@@ -97,6 +98,10 @@ struct PreambleData {
// Version of the ParseInputs this preamble was built from.
std::string Version;
tooling::CompileCommand CompileCommand;
+ // Target options used when building the preamble. Changes in target can
cause
+ // crashes when deserializing preamble, this enables consumers to use the
+ // same target (without reparsing CompileCommand).
+ std::shared_ptr TargetOpts = nullptr;
PrecompiledPreamble Preamble;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 500b72b9b327a0..4bb76cd6ab8304 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -12,10 +12,7 @@
//===--===//
#include "../../clang-tidy/ClangTidyCheck.h"
-#include "../../clang-tidy/ClangTidyModule.h"
-#include "../../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
-#include "CompileCommands.h"
#include "Compiler.h"
#include "Config.h"
#include "Diagnostics.h"
@@ -32,7 +29,6 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
-#include "clang/Lex/PPCallbacks.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Testing/Annotations/Annotations.h"
@@ -41,6 +37,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include
+#include
#include
#include
@@ -347,9 +344,8 @@ TEST(ParsedASTTest,
[clang-tools-extra] [clangd][trace] Fix comment to mention that trace spans are measured … (PR #86938)
https://github.com/kadircet approved this pull request. https://github.com/llvm/llvm-project/pull/86938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Tooling] Add special symbol mappings for C, starting with size_t (PR #85784)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/85784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Tooling] Add special symbol mappings for C, starting with size_t (PR #85784)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/85784
From 8b12e22a01058bcfdcf211b1f64d192d7c5f8463 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Tue, 19 Mar 2024 13:33:35 +0100
Subject: [PATCH 1/3] [clang][Tooling] Add special symbol mappings for C,
starting with size_t
---
.../Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc | 8
.../Tooling/Inclusions/Stdlib/StandardLibrary.cpp| 3 ++-
clang/unittests/Tooling/StandardLibraryTest.cpp | 12
3 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
new file mode 100644
index 00..1d9c294d207970
--- /dev/null
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
@@ -0,0 +1,8 @@
+//===-- StdSpecialSymbolMap.inc ---*- C
-*-===//
+//
+// This is a hand-curated list for C symbols that cannot be parsed/extracted
+// via the include-mapping tool (gen_std.py).
+//
+//===--===//
+
+SYMBOL(size_t, None, )
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
index adf1b230ff0318..386094fc2992e1 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -55,11 +55,12 @@ static const SymbolHeaderMapping *getMappingPerLang(Lang L)
{
}
static int countSymbols(Lang Language) {
- ArrayRef Symbols;
+ ArrayRef Symbols;
#define SYMBOL(Name, NS, Header) #NS #Name,
switch (Language) {
case Lang::C: {
static constexpr const char *CSymbols[] = {
+#include "CSpecialSymbolMap.inc"
#include "CSymbolMap.inc"
};
Symbols = CSymbols;
diff --git a/clang/unittests/Tooling/StandardLibraryTest.cpp
b/clang/unittests/Tooling/StandardLibraryTest.cpp
index edca31649accfa..d93b7d1af82d37 100644
--- a/clang/unittests/Tooling/StandardLibraryTest.cpp
+++ b/clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -185,6 +185,18 @@ TEST(StdlibTest, RecognizerForC99) {
stdlib::Symbol::named("", "uint8_t", stdlib::Lang::C));
}
+TEST(StdlibTest, SpecialCMappings) {
+ TestInputs Input("typedef char size_t;");
+ Input.Language = TestLanguage::Lang_C99;
+ TestAST AST(Input);
+
+ auto = lookup(AST, "size_t");
+ stdlib::Recognizer Recognizer;
+ auto ActualSym = Recognizer();
+ EXPECT_EQ(ActualSym, stdlib::Symbol::named("", "size_t", stdlib::Lang::C));
+ EXPECT_EQ(ActualSym->header()->name(), "");
+}
+
} // namespace
} // namespace tooling
} // namespace clang
From 01f76a373ae64cfad3025bee57fada2368d20aa8 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 20 Mar 2024 07:43:21 +0100
Subject: [PATCH 2/3] add alternative headers
---
clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc | 6 ++
1 file changed, 6 insertions(+)
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
index 1d9c294d207970..a515f69ea6a8cf 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
@@ -6,3 +6,9 @@
//===--===//
SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
+SYMBOL(size_t, None, )
From bb12119a0b4e0c7ea298e4c8c2cdbdafd9e60641 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Wed, 20 Mar 2024 07:48:31 +0100
Subject: [PATCH 3/3] add assertion into tests and fix it
---
clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp | 1 +
clang/unittests/Tooling/StandardLibraryTest.cpp | 1 +
2 files changed, 2 insertions(+)
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
index 386094fc2992e1..0832bcf66145fa 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -148,6 +148,7 @@ static int initialize(Lang Language) {
switch (Language) {
case Lang::C: {
static constexpr Symbol CSymbols[] = {
+#include "CSpecialSymbolMap.inc"
#include "CSymbolMap.inc"
};
for (const Symbol : CSymbols)
diff --git a/clang/unittests/Tooling/StandardLibraryTest.cpp
b/clang/unittests/Tooling/StandardLibraryTest.cpp
index d93b7d1af82d37..e4c109f3d580d1 100644
--- a/clang/unittests/Tooling/StandardLibraryTest.cpp
+++ b/clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -193,6 +193,7 @@ TEST(StdlibTest, SpecialCMappings) {
auto = lookup(AST, "size_t");
stdlib::Recognizer Recognizer;
auto ActualSym = Recognizer();
+
[clang] [clang][Tooling] Add special symbol mappings for C, starting with size_t (PR #85784)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/85784
None
From 8b12e22a01058bcfdcf211b1f64d192d7c5f8463 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Tue, 19 Mar 2024 13:33:35 +0100
Subject: [PATCH] [clang][Tooling] Add special symbol mappings for C, starting
with size_t
---
.../Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc | 8
.../Tooling/Inclusions/Stdlib/StandardLibrary.cpp| 3 ++-
clang/unittests/Tooling/StandardLibraryTest.cpp | 12
3 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
new file mode 100644
index 00..1d9c294d207970
--- /dev/null
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CSpecialSymbolMap.inc
@@ -0,0 +1,8 @@
+//===-- StdSpecialSymbolMap.inc ---*- C
-*-===//
+//
+// This is a hand-curated list for C symbols that cannot be parsed/extracted
+// via the include-mapping tool (gen_std.py).
+//
+//===--===//
+
+SYMBOL(size_t, None, )
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
index adf1b230ff0318..386094fc2992e1 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -55,11 +55,12 @@ static const SymbolHeaderMapping *getMappingPerLang(Lang L)
{
}
static int countSymbols(Lang Language) {
- ArrayRef Symbols;
+ ArrayRef Symbols;
#define SYMBOL(Name, NS, Header) #NS #Name,
switch (Language) {
case Lang::C: {
static constexpr const char *CSymbols[] = {
+#include "CSpecialSymbolMap.inc"
#include "CSymbolMap.inc"
};
Symbols = CSymbols;
diff --git a/clang/unittests/Tooling/StandardLibraryTest.cpp
b/clang/unittests/Tooling/StandardLibraryTest.cpp
index edca31649accfa..d93b7d1af82d37 100644
--- a/clang/unittests/Tooling/StandardLibraryTest.cpp
+++ b/clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -185,6 +185,18 @@ TEST(StdlibTest, RecognizerForC99) {
stdlib::Symbol::named("", "uint8_t", stdlib::Lang::C));
}
+TEST(StdlibTest, SpecialCMappings) {
+ TestInputs Input("typedef char size_t;");
+ Input.Language = TestLanguage::Lang_C99;
+ TestAST AST(Input);
+
+ auto = lookup(AST, "size_t");
+ stdlib::Recognizer Recognizer;
+ auto ActualSym = Recognizer();
+ EXPECT_EQ(ActualSym, stdlib::Symbol::named("", "size_t", stdlib::Lang::C));
+ EXPECT_EQ(ActualSym->header()->name(), "");
+}
+
} // namespace
} // namespace tooling
} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add metric for rename decl kind (PR #83867)
@@ -1072,6 +1072,10 @@ llvm::Expected rename(const RenameInputs
) {
if (Reject)
return makeError(*Reject);
+ static constexpr trace::Metric RenameTriggerCounter(
kadircet wrote:
can you do this at line 1065 instead? that way we can better track all the
requests, and not just the succesfully served ones. (to guide us for improving
the cases that are popular but don't work)
https://github.com/llvm/llvm-project/pull/83867
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add metric for rename decl kind (PR #83867)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/83867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Generate references from explicit functiontemplate specializations (PR #83392)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/83392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Generate references from explicit functiontemplate specializations (PR #83392)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/83392
None
From f62a553d8d4af339bdcb69024d5bf42527e3a01d Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 29 Feb 2024 09:25:18 +0100
Subject: [PATCH] [include-cleaner] Generate references from explicit
functiontemplate specializations
---
clang-tools-extra/include-cleaner/lib/WalkAST.cpp | 5 +
.../include-cleaner/unittests/WalkASTTest.cpp | 10 --
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index 277e6ec5b08900..878067aca0173f 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -228,6 +228,11 @@ class ASTWalker : public RecursiveASTVisitor {
// Mark declaration from definition as it needs type-checking.
if (FD->isThisDeclarationADefinition())
report(FD->getLocation(), FD);
+// Explicit specializaiton/instantiations of a function template requires
+// primary template.
+if (clang::isTemplateExplicitInstantiationOrSpecialization(
+FD->getTemplateSpecializationKind()))
+ report(FD->getLocation(), FD->getPrimaryTemplate());
return true;
}
bool VisitVarDecl(VarDecl *VD) {
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index e238dc3d902bbe..5dc88157e13af0 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,13 +229,9 @@ TEST(WalkAST, FunctionTemplates) {
EXPECT_THAT(testWalk("template void foo(T) {}",
"template void ^foo(int);"),
ElementsAre());
- // FIXME: Report specialized template as used from explicit specializations.
- EXPECT_THAT(testWalk("template void foo(T);",
+ EXPECT_THAT(testWalk("template void $explicit^foo(T);",
"template<> void ^foo(int);"),
- ElementsAre());
- EXPECT_THAT(testWalk("template void foo(T) {}",
- "template void ^foo(T*) {}"),
- ElementsAre());
+ ElementsAre(Decl::FunctionTemplate));
// Implicit instantiations references most relevant template.
EXPECT_THAT(testWalk(R"cpp(
@@ -510,6 +506,8 @@ TEST(WalkAST, Functions) {
// Definition uses declaration, not the other way around.
testWalk("void $explicit^foo();", "void ^foo() {}");
testWalk("void foo() {}", "void ^foo();");
+ testWalk("template void $explicit^foo();",
+ "template void ^foo() {}");
// Unresolved calls marks all the overloads.
testWalk("void $ambiguous^foo(int); void $ambiguous^foo(char);",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Use FoundDecl only for using-shadow-decls (PR #82615)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/82615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Use FoundDecl only for using-shadow-decls (PR #82615)
https://github.com/kadircet updated
https://github.com/llvm/llvm-project/pull/82615
From 32c8199fb9ffb9180b666e0837aa73aa6151cebc Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 22 Feb 2024 13:52:42 +0100
Subject: [PATCH] [include-cleaner] Use FoundDecl only for using-shadow-decls
---
.../include-cleaner/lib/WalkAST.cpp | 5 +++
.../include-cleaner/unittests/WalkASTTest.cpp | 34 +++
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index 6c4d9b7862d915..277e6ec5b08900 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -128,6 +128,11 @@ class ASTWalker : public RecursiveASTVisitor {
bool VisitDeclRefExpr(DeclRefExpr *DRE) {
auto *FD = DRE->getFoundDecl();
+// Prefer the underlying decl if FoundDecl isn't a shadow decl, e.g:
+// - For templates, found-decl is always primary template, but we want the
+// specializaiton itself.
+if (!llvm::isa(FD))
+ FD = DRE->getDecl();
// For refs to non-meber-like decls, use the found decl.
// For member-like decls, we should have a reference from the qualifier to
// the container decl instead, which is preferred as it'll handle
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 0be5db36b1fc51..e238dc3d902bbe 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -200,24 +200,26 @@ TEST(WalkAST, VarTemplates) {
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;)cpp",
"int z = ^Foo;"),
- ElementsAre(Decl::VarTemplate));
+ ElementsAre(Decl::VarTemplateSpecialization));
EXPECT_THAT(testWalk(R"cpp(
-template T $explicit^Foo = 0;
-template<> int Foo = 1;)cpp",
+template T Foo = 0;
+template<> int $explicit^Foo = 1;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplate));
+ ElementsAre(Decl::VarTemplateSpecialization));
// FIXME: This points at implicit specialization, instead we should point to
// explicit partial specializaiton pattern.
EXPECT_THAT(testWalk(R"cpp(
-template T $explicit^Foo = 0;
-template T* Foo = nullptr;)cpp",
+template T Foo = 0;
+template T* $explicit^Foo = nullptr;)cpp",
"int *x = ^Foo;"),
- ElementsAre(Decl::VarTemplate));
+ ElementsAre(Decl::VarTemplateSpecialization));
+ // Implicit specializations through explicit instantiations has source
+ // locations pointing at the primary template.
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;
template int Foo;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplate));
+ ElementsAre(Decl::VarTemplateSpecialization));
}
TEST(WalkAST, FunctionTemplates) {
// Explicit instantiation and (partial) specialization references primary
@@ -239,18 +241,19 @@ TEST(WalkAST, FunctionTemplates) {
EXPECT_THAT(testWalk(R"cpp(
template void $explicit^foo() {})cpp",
"auto x = []{ ^foo(); };"),
- ElementsAre(Decl::FunctionTemplate));
- // FIXME: DeclRefExpr points at primary template, not the specialization.
+ ElementsAre(Decl::Function));
EXPECT_THAT(testWalk(R"cpp(
-template void $explicit^foo() {}
-template<> void foo(){})cpp",
+template void foo() {}
+template<> void $explicit^foo(){})cpp",
"auto x = []{ ^foo(); };"),
- ElementsAre(Decl::FunctionTemplate));
+ ElementsAre(Decl::Function));
+ // The decl is actually the specialization, but explicit instantations point
+ // at the primary template.
EXPECT_THAT(testWalk(R"cpp(
template void $explicit^foo() {};
template void foo();)cpp",
"auto x = [] { ^foo(); };"),
- ElementsAre(Decl::FunctionTemplate));
+ ElementsAre(Decl::Function));
}
TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
// Class templates
@@ -548,7 +551,8 @@ TEST(WalkAST, Concepts) {
testWalk(Concept, "template void func() requires ^Foo {}");
testWalk(Concept, "void func(^Foo auto x) {}");
// FIXME: Foo should be explicitly referenced.
- testWalk("template concept Foo = true;", "void func() { ^Foo
auto x = 1; }");
+ testWalk("template concept Foo = true;",
+ "void func() { ^Foo auto x = 1; }");
}
TEST(WalkAST, FriendDecl) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Reland "[clang] Preserve found-decl when constructing VarTemplateIds" (PR #82612)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/82612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix renaming single argument ObjC methods (PR #82396)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/82396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix renaming single argument ObjC methods (PR #82396)
@@ -811,8 +811,17 @@ renameWithinFile(ParsedAST , const NamedDecl
,
continue;
Locs.push_back(RenameLoc);
}
- if (const auto *MD = dyn_cast())
-return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs));
+ if (const auto *MD = dyn_cast()) {
+// The custom ObjC selector logic doesn't handle the zero arg selector
+// case. We could use it for the one arg selector case but it's simpler to
+// use the standard one-token rename logic.
kadircet wrote:
```suggestion
// The custom ObjC selector logic doesn't handle the zero arg selector
// case, as it relies on parsing selectors via the trailing `:`.
// We also chose to use regular rename logic for the single-arg selectors
// as AST/Index has the right locations in that case.
```
https://github.com/llvm/llvm-project/pull/82396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix renaming single argument ObjC methods (PR #82396)
https://github.com/kadircet approved this pull request. https://github.com/llvm/llvm-project/pull/82396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [include-cleaner] Dont apply name-match for non-owning headers (PR #82625)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/82625
None
From 11dee86b09f99da7ce3d6dd0d3f8fcb7a3b53b43 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 22 Feb 2024 15:46:20 +0100
Subject: [PATCH] [include-cleaner] Dont apply name-match for non-owning
headers
---
.../include-cleaner/lib/FindHeaders.cpp | 6 ++
.../unittests/FindHeadersTest.cpp | 19 +++
2 files changed, 25 insertions(+)
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index fd2de6a17ad4a5..7b28d1c252d715 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -275,6 +275,12 @@ llvm::SmallVector headersForSymbol(const Symbol ,
// are already ranked in the stdlib mapping.
if (H.kind() == Header::Standard)
continue;
+// Don't apply name match hints to exporting headers. As they usually have
+// names similar to the original header, e.g. foo_wrapper/foo.h vs
+// foo/foo.h, but shouldn't be preferred (unless marked as the public
+// interface).
+if ((H.Hint & Hints::OriginHeader) == Hints::None)
+ continue;
if (nameMatch(SymbolName, H))
H.Hint |= Hints::PreferredHeader;
}
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 5a2a41b2d99bdd..07302142a13e36 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -628,5 +628,24 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) {
tooling::stdlib::Header::named("")));
}
+TEST_F(HeadersForSymbolTest, ExporterNoNameMatch) {
+ Inputs.Code = R"cpp(
+#include "exporter/foo.h"
+#include "foo_public.h"
+ )cpp";
+ Inputs.ExtraArgs.emplace_back("-I.");
+ // Deliberately named as foo_public to make sure it doesn't get name-match
+ // boost and also gets lexicographically bigger order than "exporter/foo.h".
+ Inputs.ExtraFiles["foo_public.h"] = guard(R"cpp(
+struct foo {};
+ )cpp");
+ Inputs.ExtraFiles["exporter/foo.h"] = guard(R"cpp(
+#include "foo_public.h" // IWYU pragma: export
+ )cpp");
+ buildAST();
+ EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("foo_public.h"),
+ physicalHeader("exporter/foo.h")));
+}
+
} // namespace
} // namespace clang::include_cleaner
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] SelectionTree marks nodes as complete only if children are complete (PR #82237)
https://github.com/kadircet requested changes to this pull request. mentioned this offline already but putting in here as well for my sanity. the patch is missing the actual changes to the logic, it only has test changes. https://github.com/llvm/llvm-project/pull/82237 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix renaming single argument ObjC methods (PR #82396)
@@ -811,8 +811,14 @@ renameWithinFile(ParsedAST , const NamedDecl
,
continue;
Locs.push_back(RenameLoc);
}
- if (const auto *MD = dyn_cast())
-return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs));
+ if (const auto *MD = dyn_cast()) {
+if (MD->getSelector().getNumArgs() > 1)
kadircet wrote:
can you give some context into why?
commit message just says what the code does, not why either, so it's not
obvious to me what's broken and how it's fixed with this change.
i can see how `zero` arg selectors don't work through the logic you have,
because we expect `foo:` and there's never a `:`, but I don't see what's broken
with single-arg selectors (and I guess we're having the following consume_back,
solely to force single-arg selector case work through C++ rename?)
https://github.com/llvm/llvm-project/pull/82396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [include-cleaner] Use FoundDecl only for using-shadow-decls (PR #82615)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/82615
None
From 1ae249119856ae2d8c1d2d4a3c3e311fa3d66dd0 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 22 Feb 2024 13:22:11 +0100
Subject: [PATCH 1/2] Reland "[clang] Preserve found-decl when constructing
VarTemplateIds"
Update include-cleaner tests. Now that we have proper found-decls set up
for VarTemplates, in case of instationtations we point to primary
templates and not specializations. To be changed in a follow-up patch.
---
.../include-cleaner/unittests/WalkASTTest.cpp | 16
clang/include/clang/Sema/Sema.h| 2 +-
clang/lib/Sema/SemaTemplate.cpp| 18 --
clang/test/AST/ast-dump-using.cpp | 7 +++
4 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index bdfc24b8edee38..0be5db36b1fc51 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -200,24 +200,24 @@ TEST(WalkAST, VarTemplates) {
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;)cpp",
"int z = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
EXPECT_THAT(testWalk(R"cpp(
-template T Foo = 0;
-template<> int $explicit^Foo = 1;)cpp",
+template T $explicit^Foo = 0;
+template<> int Foo = 1;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
// FIXME: This points at implicit specialization, instead we should point to
// explicit partial specializaiton pattern.
EXPECT_THAT(testWalk(R"cpp(
-template T Foo = 0;
-template T* $explicit^Foo = nullptr;)cpp",
+template T $explicit^Foo = 0;
+template T* Foo = nullptr;)cpp",
"int *x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;
template int Foo;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
}
TEST(WalkAST, FunctionTemplates) {
// Explicit instantiation and (partial) specialization references primary
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fcccac10f4733a..e457694e4625db 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8540,7 +8540,7 @@ class Sema final {
/// if the arguments are dependent.
ExprResult CheckVarTemplateId(const CXXScopeSpec ,
const DeclarationNameInfo ,
-VarTemplateDecl *Template,
+VarTemplateDecl *Template, NamedDecl *FoundD,
SourceLocation TemplateLoc,
const TemplateArgumentListInfo *TemplateArgs);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1a975a8d0a0df5..7d3d665194add1 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4958,11 +4958,10 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template,
SourceLocation TemplateLoc,
return Decl;
}
-ExprResult
-Sema::CheckVarTemplateId(const CXXScopeSpec ,
- const DeclarationNameInfo ,
- VarTemplateDecl *Template, SourceLocation TemplateLoc,
- const TemplateArgumentListInfo *TemplateArgs) {
+ExprResult Sema::CheckVarTemplateId(
+const CXXScopeSpec , const DeclarationNameInfo ,
+VarTemplateDecl *Template, NamedDecl *FoundD, SourceLocation TemplateLoc,
+const TemplateArgumentListInfo *TemplateArgs) {
DeclResult Decl = CheckVarTemplateId(Template, TemplateLoc,
NameInfo.getLoc(),
*TemplateArgs);
@@ -4978,8 +4977,7 @@ Sema::CheckVarTemplateId(const CXXScopeSpec ,
NameInfo.getLoc());
// Build an ordinary singleton decl ref.
- return BuildDeclarationNameExpr(SS, NameInfo, Var,
- /*FoundD=*/nullptr, TemplateArgs);
+ return BuildDeclarationNameExpr(SS, NameInfo, Var, FoundD, TemplateArgs);
}
void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
@@ -5066,9 +5064,9 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec
,
bool KnownDependent = false;
// In C++1y, check variable template ids.
if (R.getAsSingle()) {
-ExprResult Res = CheckVarTemplateId(SS, R.getLookupNameInfo(),
-R.getAsSingle(),
-
[clang] [clang-tools-extra] Reland "[clang] Preserve found-decl when constructing VarTemplateIds" (PR #82612)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/82612
Update include-cleaner tests. Now that we have proper found-decls set up
for VarTemplates, in case of instationtations we point to primary
templates and not specializations. To be changed in a follow-up patch.
From 1ae249119856ae2d8c1d2d4a3c3e311fa3d66dd0 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Thu, 22 Feb 2024 13:22:11 +0100
Subject: [PATCH] Reland "[clang] Preserve found-decl when constructing
VarTemplateIds"
Update include-cleaner tests. Now that we have proper found-decls set up
for VarTemplates, in case of instationtations we point to primary
templates and not specializations. To be changed in a follow-up patch.
---
.../include-cleaner/unittests/WalkASTTest.cpp | 16
clang/include/clang/Sema/Sema.h| 2 +-
clang/lib/Sema/SemaTemplate.cpp| 18 --
clang/test/AST/ast-dump-using.cpp | 7 +++
4 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index bdfc24b8edee38..0be5db36b1fc51 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -200,24 +200,24 @@ TEST(WalkAST, VarTemplates) {
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;)cpp",
"int z = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
EXPECT_THAT(testWalk(R"cpp(
-template T Foo = 0;
-template<> int $explicit^Foo = 1;)cpp",
+template T $explicit^Foo = 0;
+template<> int Foo = 1;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
// FIXME: This points at implicit specialization, instead we should point to
// explicit partial specializaiton pattern.
EXPECT_THAT(testWalk(R"cpp(
-template T Foo = 0;
-template T* $explicit^Foo = nullptr;)cpp",
+template T $explicit^Foo = 0;
+template T* Foo = nullptr;)cpp",
"int *x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
EXPECT_THAT(testWalk(R"cpp(
template T $explicit^Foo = 0;
template int Foo;)cpp",
"int x = ^Foo;"),
- ElementsAre(Decl::VarTemplateSpecialization));
+ ElementsAre(Decl::VarTemplate));
}
TEST(WalkAST, FunctionTemplates) {
// Explicit instantiation and (partial) specialization references primary
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fcccac10f4733a..e457694e4625db 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8540,7 +8540,7 @@ class Sema final {
/// if the arguments are dependent.
ExprResult CheckVarTemplateId(const CXXScopeSpec ,
const DeclarationNameInfo ,
-VarTemplateDecl *Template,
+VarTemplateDecl *Template, NamedDecl *FoundD,
SourceLocation TemplateLoc,
const TemplateArgumentListInfo *TemplateArgs);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1a975a8d0a0df5..7d3d665194add1 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4958,11 +4958,10 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template,
SourceLocation TemplateLoc,
return Decl;
}
-ExprResult
-Sema::CheckVarTemplateId(const CXXScopeSpec ,
- const DeclarationNameInfo ,
- VarTemplateDecl *Template, SourceLocation TemplateLoc,
- const TemplateArgumentListInfo *TemplateArgs) {
+ExprResult Sema::CheckVarTemplateId(
+const CXXScopeSpec , const DeclarationNameInfo ,
+VarTemplateDecl *Template, NamedDecl *FoundD, SourceLocation TemplateLoc,
+const TemplateArgumentListInfo *TemplateArgs) {
DeclResult Decl = CheckVarTemplateId(Template, TemplateLoc,
NameInfo.getLoc(),
*TemplateArgs);
@@ -4978,8 +4977,7 @@ Sema::CheckVarTemplateId(const CXXScopeSpec ,
NameInfo.getLoc());
// Build an ordinary singleton decl ref.
- return BuildDeclarationNameExpr(SS, NameInfo, Var,
- /*FoundD=*/nullptr, TemplateArgs);
+ return BuildDeclarationNameExpr(SS, NameInfo, Var, FoundD, TemplateArgs);
}
void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
@@ -5066,9 +5064,9 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec
,
bool KnownDependent = false;
// In C++1y, check
[clang] [clang] Preserve found-decl when constructing VarTemplateIds (PR #82265)
https://github.com/kadircet closed https://github.com/llvm/llvm-project/pull/82265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Prevent printing huge initializer lists in hover definitions (PR #79746)
@@ -143,8 +143,16 @@ std::string printDefinition(const Decl *D, PrintingPolicy
PP,
// Initializers might be huge and result in lots of memory allocations in
// some catostrophic cases. Such long lists are not useful in hover cards
// anyway.
- if (200 < TB.expandedTokens(IE->getSourceRange()).size())
+ const auto = VD->getASTContext().getSourceManager();
+ if (!SM.isInMainFile(VD->getLocation())) {
kadircet wrote:
we actually need `IE->getSourceRange()` to be inside the mainfile, this can
still be `int x[] = EXPANDS_TO_CRAZY_LONG_INIT_DEFINED_OUTSIDE_THE_MAINFILE`.
also you're just checking the first level of children now, we can have
something like: `int **x = { {CRAZY_EXPANSION} };` which only has a single
children at top level, but still has a ton nested underneath.
can we just have:
```
auto TotalChildrenInStmt = [&](const Stmt *S) {
size_t res = 1;
for(auto : S->children()) res += TotalChildrenInStmt(C);
return res;
};
PP.SuppressInitializers = 200 < TB.expandedTokens(IE->getSourceRange()).size()
|| 100 < TotalChildrenInExpr(IE);
```
https://github.com/llvm/llvm-project/pull/79746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Preserve found-decl when constructing VarTemplateIds (PR #82265)
kadircet wrote: cc @hokein https://github.com/llvm/llvm-project/pull/82265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
https://github.com/kadircet approved this pull request. thanks, lgtm! https://github.com/llvm/llvm-project/pull/69477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
kadircet wrote:
> "end":{"character":18,"
But that's [[2 + ]]3, no? I get an end character position of 19 with both Qt
Creator and VSCode for the [[2 + 3]] selection.
oh wow you're right, looks like the version of YCM I have is having off-by-one
for that case.
---
sorry for the noise, i didn't know about `getBinaryOperatorRange` which does
this kind of pruning. it all makes sense now.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Preserve found-decl when constructing VarTemplateIds (PR #82265)
https://github.com/kadircet created
https://github.com/llvm/llvm-project/pull/82265
None
From 05e1d91361b4ce4226d21b00a312eb7642057c8f Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya
Date: Mon, 19 Feb 2024 17:42:34 +0100
Subject: [PATCH] [clang] Preserve found-decl when constructing VarTemplateIds
---
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Sema/SemaTemplate.cpp | 18 --
clang/test/AST/ast-dump-using.cpp | 7 +++
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1f1cbd11ff7358..6c0c8945d7de25 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8416,7 +8416,7 @@ class Sema final {
/// if the arguments are dependent.
ExprResult CheckVarTemplateId(const CXXScopeSpec ,
const DeclarationNameInfo ,
-VarTemplateDecl *Template,
+VarTemplateDecl *Template, NamedDecl *FoundD,
SourceLocation TemplateLoc,
const TemplateArgumentListInfo *TemplateArgs);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9bfa71dc8bcf1d..ee00b1e119e042 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4960,11 +4960,10 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template,
SourceLocation TemplateLoc,
return Decl;
}
-ExprResult
-Sema::CheckVarTemplateId(const CXXScopeSpec ,
- const DeclarationNameInfo ,
- VarTemplateDecl *Template, SourceLocation TemplateLoc,
- const TemplateArgumentListInfo *TemplateArgs) {
+ExprResult Sema::CheckVarTemplateId(
+const CXXScopeSpec , const DeclarationNameInfo ,
+VarTemplateDecl *Template, NamedDecl *FoundD, SourceLocation TemplateLoc,
+const TemplateArgumentListInfo *TemplateArgs) {
DeclResult Decl = CheckVarTemplateId(Template, TemplateLoc,
NameInfo.getLoc(),
*TemplateArgs);
@@ -4980,8 +4979,7 @@ Sema::CheckVarTemplateId(const CXXScopeSpec ,
NameInfo.getLoc());
// Build an ordinary singleton decl ref.
- return BuildDeclarationNameExpr(SS, NameInfo, Var,
- /*FoundD=*/nullptr, TemplateArgs);
+ return BuildDeclarationNameExpr(SS, NameInfo, Var, FoundD, TemplateArgs);
}
void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
@@ -5068,9 +5066,9 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec
,
bool KnownDependent = false;
// In C++1y, check variable template ids.
if (R.getAsSingle()) {
-ExprResult Res = CheckVarTemplateId(SS, R.getLookupNameInfo(),
-R.getAsSingle(),
-TemplateKWLoc, TemplateArgs);
+ExprResult Res = CheckVarTemplateId(
+SS, R.getLookupNameInfo(), R.getAsSingle(),
+R.getRepresentativeDecl(), TemplateKWLoc, TemplateArgs);
if (Res.isInvalid() || Res.isUsable())
return Res;
// Result is dependent. Carry on to build an UnresolvedLookupEpxr.
diff --git a/clang/test/AST/ast-dump-using.cpp
b/clang/test/AST/ast-dump-using.cpp
index c007ecd8bda583..32826bb95676f3 100644
--- a/clang/test/AST/ast-dump-using.cpp
+++ b/clang/test/AST/ast-dump-using.cpp
@@ -2,6 +2,7 @@
namespace a {
struct S;
+template T x = {};
}
namespace b {
using a::S;
@@ -15,4 +16,10 @@ typedef S f; // to dump the introduced type
// CHECK-NEXT: `-UsingType {{.*}} 'a::S' sugar
// CHECK-NEXT: |-UsingShadow {{.*}} 'S'
// CHECK-NEXT: `-RecordType {{.*}} 'a::S'
+using a::x;
+
+void foo() {
+ x = 3;
+ // CHECK: DeclRefExpr {{.*}} 'x' {{.*}} (UsingShadow {{.*}} 'x')
+}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/69477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
kadircet wrote:
sorry I don't think you can get that kind of behavior from a regular clangd.
can you add a test case demonstrating how that comes to be?
here's a sample clangd test proving my point:
```
Content-Length: 1290
{"id":1,"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"documentationFormat":["plaintext","markdown"]},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"documentSymbol":{"hierarchicalDocumentSymbolSupport":false,"labelSupport":false,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"hover":{"contentFormat":["plaintext","markdown"]},"signatureHelp":{"signatureInformation":{"documentationFormat":["plaintext","markdown"],"parameterInformation":{"labelOffsetSupport":true}}},"synchronization":{"didSave":true}},"workspace":{"applyEdit":true,"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"workspaceEdit":{"documentChanges":true}}},"initializationOptions":{"clangdFileStatus":true},"processId":4179957,"rootPath":"/usr/local/google/home/kadircet/repos/tmp","rootUri":"file:///usr/local/google/home/kadircet/repos/tmp"}}Content-Length:
52
{"jsonrpc":"2.0","method":"initialized","params":{}}Content-Length: 109
{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"clangdFileStatus":true}}}Content-Length:
193
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void
foo() {\n int x = 1 + 2 + 3;\n
return;\n}\n","uri":"file:///tmp/a.cc","version":1}}}Content-Length: 217
{"id":2,"jsonrpc":"2.0","method":"textDocument/codeAction","params":{"context":{"diagnostics":[]},"range":{"end":{"character":18,"line":1},"start":{"character":14,"line":1}},"textDocument":{"uri":"file:///tmp/a.cc"}}}Content-Length:
251
{"id":3,"jsonrpc":"2.0","method":"workspace/executeCommand","params":{"arguments":[{"file":"file:///tmp/a.cc","selection":{"end":{"character":18,"line":1},"start":{"character":14,"line":1}},"tweakID":"ExtractVariable"}],"command":"clangd.applyTweak"}}Content-Length:
50
{"id":0,"jsonrpc":"2.0","result":{"applied":true}}Content-Length: 232
{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"contentChanges":[{"text":"void
foo() {\n auto placeholder = 1 + 2 + 3;\n int x = placeholder;\n
return;\n}\n"}],"textDocument":{"uri":"file:///tmp/a.cc","version":2}}}Content-Length:
115
{"jsonrpc":"2.0","method":"textDocument/didClose","params":{"textDocument":{"uri":"file:///tmp/a.cc","version":2}}}Content-Length:
58
{"id":4,"jsonrpc":"2.0","method":"shutdown","params":null}Content-Length: 47
{"jsonrpc":"2.0","method":"exit","params":null}
```
some notable points here are:
- original file contents: `"text":"void foo() {\n int x = 1 + 2 + 3;\n
return;\n}\n"`
- code action request range:
`,"range":{"end":{"character":18,"line":1},"start":{"character":14,"line":1}}`,
namely `[[2 + 3]]`.
- code action response: `"contentChanges":[{"text":"void foo() {\n auto
placeholder = 1 + 2 + 3;\n int x = placeholder;\n return;\n}\n"}]`
that's my clangd version string: `clangd version 19.0.0git
(g...@github.com:llvm/llvm-project.git
7e3fb372b0e8899958ec7e9241797e7e136a7a23)`
you can feed in the json request above into clangd with ` /path/to/clangd
-log=verbose -sync < /tmp/clangd_input.mirror`.
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Do not offer extraction to variable for decl init expression (PR #69477)
@@ -490,6 +491,13 @@ bool eligibleForExtraction(const SelectionTree::Node *N) {
BO->getRHS() == OuterImplicit.ASTNode.get())
return false;
}
+ if (const auto *Decl = Parent->ASTNode.get()) {
+if (!Decl->isInitCapture() &&
kadircet wrote:
thanks for the example!
I guess one conceptual thing to note here is, clangd doesn't run it's
refactorings on the exact selection user provided. instead it figures out an
AST node that's "relevant" to this selection, and everything else is performed
on that AST node, mostly independent of the selection.
So this approach is surely not always what the user means/wants, but let's us
reason about the refactoring going on. For this case in particular, even if you
said the refactoring is available for selection of `x = 1 + [[2 + 3]]` through
these heuristics, clangd will still end up extracting the full RHS, because
that's the AST node we associated with that selection.
Hence I'd actually lean towards not special casing it here, just to do the
thing we're trying to prevent later on. Any particular reason why we should say
extraction is available for such cases (and then still perform the extraction
we're trying to avoid)?
https://github.com/llvm/llvm-project/pull/69477
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token , const syntax::Token ) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token , const syntax::Token
,
+const SourceManager ,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager , Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
kadircet wrote:
nit: `Last` is not used outside the loop, and it's still confusing me to see
loop condition below as `Index < Last`. what about
`for (unsigned Index = 0, Last = Tokens.size(); Index < Last - 1; ++Index)`
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
@@ -538,11 +564,222 @@ std::optional checkName(const NamedDecl
,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token , const syntax::Token ) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token , const syntax::Token
,
+const SourceManager ,
+llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel at the
+// top level (not nested in any () or {} or []). The search will terminate upon
+// seeing Terminator or a ; at the top level.
+std::optional
+findAllSelectorPieces(llvm::ArrayRef Tokens,
+ const SourceManager , Selector Sel,
+ tok::TokenKind Terminator) {
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector Closes;
+ std::vector SelectorPieces;
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+const auto = Tokens[Index];
+
+if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+// If 'foo:' instead of ':' (empty selector), we need to skip the ':'
kadircet wrote:
as discussed we aren't actually handling this case probably (we won't have a
token for an empty selector name).
can you just leave a comment explaining what & why it doesn't work (i don't
necessarily see it as a TODO, as utility/complexity ratio is pretty low)?
https://github.com/llvm/llvm-project/pull/76466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
