[PATCH] D53921: Compound literals and enums require constant inits
void created this revision.
void added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
Both compound literals and enums require their initializers to be
constant. Wrap the initializer expressions in a ConstantExpr so that we
can easily check for this later on.
Repository:
rC Clang
https://reviews.llvm.org/D53921
Files:
lib/ARCMigrate/ObjCMT.cpp
lib/AST/ASTImporter.cpp
lib/AST/ExprConstant.cpp
lib/Analysis/CFG.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
lib/StaticAnalyzer/Core/Environment.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Misc/ast-dump-decl.c
test/SemaCXX/compound-literal.cpp
Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -37,28 +37,32 @@
void test() {
(void)(POD){1, 2};
// CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-// CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+// CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
+// CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
// CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
// CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
// CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
(void)(HasDtor){1, 2};
// CHECK: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasDtor'
+// CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasDtor'
// CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasDtor'
// CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::HasDtor'
// CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
// CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
#if __cplusplus >= 201103L
(void)(HasCtor){1, 2};
// CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtor'
-// CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11: ConstantExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
// CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtor'
// CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
// CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
(void)(HasCtorDtor){1, 2};
// CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasCtorDtor'
// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
// CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtorDtor'
// CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -96,6 +96,7 @@
};
// CHECK: EnumConstantDecl{{.*}} TestEnumConstantDecl 'int'
// CHECK: EnumConstantDecl{{.*}} TestEnumConstantDeclInit 'int'
+// CHECK-NEXT: ConstantExpr
// CHECK-NEXT: IntegerLiteral
struct testIndirectFieldDecl {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,9 +1267,6 @@
case Stmt::ObjCPropertyRefExprClass:
llvm_unreachable("These are handled by PseudoObjectExpr");
-case Expr::ConstantExprClass:
- return Visit(cast(S)->getSubExpr(), Pred, DstTop);
-
case Stmt::GNUNullExprClass: {
// GNU __null is a pointer-width integer, not an actual pointer.
ProgramStateRef state = Pred->getState();
@@ -1285,6 +1282,10 @@
Bldr.addNodes(Dst);
break;
+case Expr::ConstantExprClass:
+ // Handled due to it being a wrapper class.
+ break;
+
case Stmt::ExprWithCleanupsClass:
// Handled due to fully linearised CFG.
break;
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -92,6 +92,7 @@
case Stmt::ExprWithCleanupsClass:
case Stmt::GenericSelectionExprClass:
case Stmt::OpaqueValueExprClass:
+ case Stmt::ConstantExprClass:
case Stmt::ParenExprClass:
case Stmt::SubstNonTypeTemplateParmExprClass:
llvm_unreachable("Should have been handled by ignoreTransparentExprs");
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -5503,7 +5503,8 @@
// array from a compound literal that creates an array of the same
// type, so long as the initializer has no s
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
takuto.ikuta accepted this revision. takuto.ikuta added a comment. LGTM, thank you for fix! https://reviews.llvm.org/D53870 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r345699 - [clang-cl] Inherit dllexport to static locals also in template instantiations (PR39496)
Author: hans
Date: Wed Oct 31 01:38:48 2018
New Revision: 345699
URL: http://llvm.org/viewvc/llvm-project?rev=345699&view=rev
Log:
[clang-cl] Inherit dllexport to static locals also in template instantiations
(PR39496)
In the course of D51340, @takuto.ikuta discovered that Clang fails to put
dllexport/import attributes on static locals during template instantiation.
For regular functions, this happens in Sema::FinalizeDeclaration(), however for
template instantiations we need to do something in or around
TemplateDeclInstantiator::VisitVarDecl(). This patch does that, and extracts
the code to a utility function.
Differential Revision: https://reviews.llvm.org/D53870
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=345699&r1=345698&r2=345699&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Oct 31 01:38:48 2018
@@ -2001,6 +2001,7 @@ public:
SourceLocation AttrEnd);
void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
+ void CheckStaticLocalForDllExport(VarDecl *VD);
void FinalizeDeclaration(Decl *D);
DeclGroupPtrTy FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
ArrayRef Group);
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=345699&r1=345698&r2=345699&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Oct 31 01:38:48 2018
@@ -11924,6 +11924,23 @@ static bool hasDependentAlignment(VarDec
return false;
}
+/// Check if VD needs to be dllexport/dllimport due to being in a
+/// dllexport/import function.
+void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
+ assert(VD->isStaticLocal());
+
+ auto *FD = dyn_cast_or_null(VD->getParentFunctionOrMethod());
+ if (!FD)
+return;
+
+ // Static locals inherit dll attributes from their function.
+ if (Attr *A = getDLLAttr(FD)) {
+auto *NewAttr = cast(A->clone(getASTContext()));
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
+ }
+}
+
/// FinalizeDeclaration - called by ParseDeclarationAfterDeclarator to perform
/// any semantic actions necessary after any initializer has been attached.
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
@@ -11977,14 +11994,9 @@ void Sema::FinalizeDeclaration(Decl *Thi
}
if (VD->isStaticLocal()) {
-if (FunctionDecl *FD =
-dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
- // Static locals inherit dll attributes from their function.
- if (Attr *A = getDLLAttr(FD)) {
-auto *NewAttr = cast(A->clone(getASTContext()));
-NewAttr->setInherited(true);
-VD->addAttr(NewAttr);
- }
+CheckStaticLocalForDllExport(VD);
+
+if (dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
// CUDA 8.0 E.3.9.4: Within the body of a __device__ or __global__
// function, only __shared__ variables or variables without any device
// memory qualifiers may be declared with static storage class.
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=345699&r1=345698&r2=345699&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Oct 31 01:38:48 2018
@@ -728,6 +728,9 @@ Decl *TemplateDeclInstantiator::VisitVar
D->getLocation(), D->getIdentifier(), DI->getType(),
DI, D->getStorageClass());
+ if (Var->isStaticLocal())
+SemaRef.CheckStaticLocalForDllExport(Var);
+
// In ARC, infer 'retaining' for variables of retainable type.
if (SemaRef.getLangOpts().ObjCAutoRefCount &&
SemaRef.inferObjCARCLifetime(Var))
Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=345699&r1=345698&r2=345699&view=diff
==
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Wed Oct 31 01:38:48 2018
@@ -1012,6 +1012,18 @@ struct __declspec(dllexport) LayerTreeIm
// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc
%"struct.LayerTreeIm
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
This revision was automatically updated to reflect the committed changes.
Closed by commit rC345699: [clang-cl] Inherit dllexport to static locals also
in template instantiations… (authored by hans, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D53870
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -728,6 +728,9 @@
D->getLocation(), D->getIdentifier(), DI->getType(),
DI, D->getStorageClass());
+ if (Var->isStaticLocal())
+SemaRef.CheckStaticLocalForDllExport(Var);
+
// In ARC, infer 'retaining' for variables of retainable type.
if (SemaRef.getLangOpts().ObjCAutoRefCount &&
SemaRef.inferObjCARCLifetime(Var))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11924,6 +11924,23 @@
return false;
}
+/// Check if VD needs to be dllexport/dllimport due to being in a
+/// dllexport/import function.
+void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
+ assert(VD->isStaticLocal());
+
+ auto *FD = dyn_cast_or_null(VD->getParentFunctionOrMethod());
+ if (!FD)
+return;
+
+ // Static locals inherit dll attributes from their function.
+ if (Attr *A = getDLLAttr(FD)) {
+auto *NewAttr = cast(A->clone(getASTContext()));
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
+ }
+}
+
/// FinalizeDeclaration - called by ParseDeclarationAfterDeclarator to perform
/// any semantic actions necessary after any initializer has been attached.
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
@@ -11977,14 +11994,9 @@
}
if (VD->isStaticLocal()) {
-if (FunctionDecl *FD =
-dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
- // Static locals inherit dll attributes from their function.
- if (Attr *A = getDLLAttr(FD)) {
-auto *NewAttr = cast(A->clone(getASTContext()));
-NewAttr->setInherited(true);
-VD->addAttr(NewAttr);
- }
+CheckStaticLocalForDllExport(VD);
+
+if (dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
// CUDA 8.0 E.3.9.4: Within the body of a __device__ or __global__
// function, only __shared__ variables or variables without any device
// memory qualifiers may be declared with static storage class.
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2001,6 +2001,7 @@
SourceLocation AttrEnd);
void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
+ void CheckStaticLocalForDllExport(VarDecl *VD);
void FinalizeDeclaration(Decl *D);
DeclGroupPtrTy FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
ArrayRef Group);
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -1012,6 +1012,18 @@
// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QAE@XZ"
// M64-DAG: define weak_odr dso_local dllexport %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QEAA@XZ"
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals also in template
+// specializations.
+template struct __declspec(dllexport) S { int foo() { static int x; return x++; } };
+int foo() { S s; return s.foo(); }
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$S@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+template struct T { int foo() { static int x; return x++; } };
+template struct __declspec(dllexport) T;
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+}
+
class __declspec(dllexport) ACE_Shared_Object {
public:
virtual ~ACE_Shared_Object();
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -996,3 +996,16 @@
USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2, func)
// M32-DAG: declare dllimport x86_thiscallcc void @"?func@?$ExplicitInstantiationDeclTemplateBase2@H@@QAEXXZ"
// G32-DAG: define weak_odr dso_local x86_thiscallcc void @_ZN38ExplicitInstantiationDeclTemplateBase2IiE4funcEv
+
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals
[PATCH] D53922: [clangd] fix non linux build
devnexen created this revision.
devnexen added reviewers: kadircet, sammccall.
devnexen created this object with visibility "All Users".
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov, krytarowski.
There is no SCHED_IDLE semantic equivalent in BSD systems.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53922
Files:
clangd/Threading.cpp
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -102,7 +102,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#ifdef HAVE_PTHREAD_H
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -102,7 +102,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#ifdef HAVE_PTHREAD_H
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
hans added inline comments.
Comment at: test/CodeGenCXX/dllimport.cpp:1010
+int bar() { T t; return t.foo(); }
+// MO1-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" =
available_externally dllimport global i32 0, align 4
+}
rnk wrote:
> I notice that we don't emit `foo` as an available_externally definition right
> now. With your change, will we do so? Should we?
Ah, good point. It's actually the static local that was previously preventing
us from emitting it available_externally. We normally would, but
DLLImportFunctionVisitor would discover that the function referenced a
non-dllimport "global" variable, and determine that it was not safe to emit the
definition.
But now that the static local inherits the dll attribute, this works out
automatically.
```
$ bin/clang -cc1 -triple i686-windows-msvc -fms-extensions -emit-llvm
-std=c++1y -O1 -disable-llvm-passes -o -
../tools/clang/test/CodeGenCXX/dllimport.cpp -DMSABI -w | grep
'define.*?foo@?$T@H@pr39496'
define available_externally dllimport x86_thiscallcc i32
@"?foo@?$T@H@pr39496@@QAEHXZ"(%"struct.pr39496::T"* %this) #0 align 2 {
```
(Same for S::foo() one.)
Repository:
rC Clang
https://reviews.llvm.org/D53870
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > takuto.ikuta wrote:
> > > > > > > > hans wrote:
> > > > > > > > > I still don't understand why we need these checks for
> > > > > > > > > template instantiations. Why does it matter whether the
> > > > > > > > > functions get inlined or not?
> > > > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > > > needs to be dllexported from implementation library.
> > > > > > > >
> > > > > > > > c.h
> > > > > > > > ```
> > > > > > > > template
> > > > > > > > class EXPORT C {
> > > > > > > > public:
> > > > > > > > void f() {}
> > > > > > > > };
> > > > > > > > ```
> > > > > > > >
> > > > > > > > cuser.cc
> > > > > > > > ```
> > > > > > > > #include "c.h"
> > > > > > > >
> > > > > > > > void cuser() {
> > > > > > > > C c;
> > > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > > }
> > > > > > > > ```
> > > > > > > >
> > > > > > > > When cuser.cc and c.h are built to different library, cuser.cc
> > > > > > > > needs to be able to see dllexported C::f() when C::f() is not
> > > > > > > > inlined.
> > > > > > > > This is my understanding.
> > > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > > declaration, so doesn't apply here I think.
> > > > > > >
> > > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > > fine. It doesn't matter whether the function gets inlined or not,
> > > > > > > the only thing that matters is that if it's marked dllimport on
> > > > > > > the "consumer side", it must be dllexport when building the dll.
> > > > > > Hmm, I changed the linkage for functions having
> > > > > > DLLExport/ImportStaticLocal Attr.
> > > > > >
> > > > > I changed linkage in ASTContext so that inline function is emitted
> > > > > when it is necessary when we use fno-dllexport-inlines.
> > > > I revived explicit template instantiation check.
> > > > Found that this is necessary.
> > > >
> > > > For explicit template instantiation, inheriting dll attribute from
> > > > function for local static var is run before inheriting dll attribute
> > > > from class for member functions. So I cannot detect local static var of
> > > > inline function after class level dll attribute processing for explicit
> > > > template instantiation.
> > > >
> > > Oh I see, it's a static local problem..
> > > Can you provide a concrete example that does not work without your check?
> > > Maybe this is the right thing to do, but I would like to understand
> > > exactly what the problem is.
> > For the following code
> > ```
> > template
> > class M{
> > public:
> >
> > T Inclass() {
> > static T static_x;
> > ++static_x;
> > return static_x;
> > }
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > int f (){
> > M mi;
> > M ms;
> > return mi.Inclass() + ms.Inclass();
> > }
> >
> > ```
> >
> > llvm code without instantiation check become like below. Both inline
> > functions of M and M is not dllimported/exported.
> > ```
> > $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
> >
> > $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
> >
> > @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local global
> > i32 0, comdat, align 4
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %this)
> > #0 comdat align 2 {
> > entry:
> > %this.addr = alloca %class.M*, align 8
> > store %class.M* %this, %class.M** %this.addr, align 8
> > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > %0 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %inc = add nsw i32 %0, 1
> > store i32 %inc, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %1 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > ret i32 %1
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local i32 @"?f@@YAHXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > %call = call i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %mi)
> > %call1 = call i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0* %ms)
> > %conv = sext i16 %call1 to i32
> > %add = add nsw i32 %call, %conv
> > ret i32 %add
> > }
> >
> > declare dso_local i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0*) #1
> > ```
> >
> >
> > With the check, both functions are dllimported/exported and
[PATCH] D53922: [clangd] fix non linux build
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! We should work out what to do on BSD+Mac (and windows at some point), but no-op is good for now. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Looks sane to me, you might want someone more familiar with ObjC to verify though. Repository: rC Clang https://reviews.llvm.org/D53900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r345700 - [clangd] fix non linux build
Author: devnexen
Date: Wed Oct 31 02:04:15 2018
New Revision: 345700
URL: http://llvm.org/viewvc/llvm-project?rev=345700&view=rev
Log:
[clangd] fix non linux build
There is no SCHED_IDLE semantic equivalent in BSD systems.
Reviewers: kadircet, sammccall
Revieweed By: sammccall
Differential Revision: https://reviews.llvm.org/D53922
Modified:
clang-tools-extra/trunk/clangd/Threading.cpp
Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=345700&r1=345699&r2=345700&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Wed Oct 31 02:04:15 2018
@@ -102,7 +102,7 @@ void wait(std::unique_lock &
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#ifdef HAVE_PTHREAD_H
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345700: [clangd] fix non linux build (authored by devnexen,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53922?vs=171870&id=171873#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53922
Files:
clang-tools-extra/trunk/clangd/Threading.cpp
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -102,7 +102,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#ifdef HAVE_PTHREAD_H
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -102,7 +102,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#ifdef HAVE_PTHREAD_H
+#if defined(HAVE_PTHREAD_H) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
krytarowski added a comment. For NetBSD: - `ThreadPriority::Low` select either `SCHED_RR` or `SCHED_FIFO`, call sched_get_priority_min() and set pthread_setschedparam(). - `ThreadPriority::Normal` use SCHED_OTHER with `PRI_NONE`. Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.
kadircet added inline comments. Comment at: clangd/index/SymbolCollector.cpp:619 + shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache); + if (auto DefLoc = getTokenLocation(Loc, ND.getASTContext().getSourceManager(), Opts, ASTCtx->getLangOpts(), FileURI)) `ND.getASTContext().getSourceManager()` -> `SM` ? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition
ebevhan added a comment. In https://reviews.llvm.org/D53738#1281332, @rjmccall wrote: > Well, maybe the cleanest solution would be to actually fold > `CompoundAssignOperator` back into `BinaryOperator` and just allow > `BinaryOperator` to optionally store information about the intermediate type > of the computation and how the operands need to be promoted. That > information could be elided in the common cases where it's trivial, of course. That sounds like a fairly hefty refactor. Also, doing the fold would put the extra QualType info from CAO into BO, sure, but this cannot work for the full-precision case since we can't represent those with QualTypes. The information for the full precision 'type' would have to be stored separately anyway. Or did you mean to make a subclass of that new BinaryOperator for the full precision case, and store the full precision info there? It might just be easier to store the full-precision info in BO directly. BO might be too common to warrant the size increase, though. FixedPointSemantics can probably be optimized to only take 32 bits. > The infinite-precision rule here is still internal to an individual operator, > right? The standard's not trying to say that we should evaluate `x + y < z` > by doing a comparison as if all the operands were individually > infinite-precision? Correct, the result of the computation is 'implicitly converted' back to the result type after the operation is performed. The type of the expression will always be the result type, never the full precision type. As a side note, comparisons are still a bit up in the air. I don't think we came to a conclusion on whether they should be done in full precision or bitwise. The spec isn't clear. Repository: rC Clang https://reviews.llvm.org/D53738 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
sammccall added a comment. In https://reviews.llvm.org/D53922#1281882, @krytarowski wrote: > For NetBSD: > > - `ThreadPriority::Low` select either `SCHED_RR` or `SCHED_FIFO`, call > sched_get_priority_min() and set pthread_setschedparam(). https://www.netbsd.org/docs/internals/en/chap-processes.htm indicates `SCHED_RR` and `SCHED_FIFO` are real-time priorities on NetBSD, as they are on linux. i.e. higher priority than normal threads, regardless of priority value. Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
krytarowski added a comment. In https://reviews.llvm.org/D53922#1281898, @sammccall wrote: > In https://reviews.llvm.org/D53922#1281882, @krytarowski wrote: > > > For NetBSD: > > > > - `ThreadPriority::Low` select either `SCHED_RR` or `SCHED_FIFO`, call > > sched_get_priority_min() and set pthread_setschedparam(). > > > https://www.netbsd.org/docs/internals/en/chap-processes.htm indicates > `SCHED_RR` and `SCHED_FIFO` are real-time priorities on NetBSD, as they are > on linux. > i.e. higher priority than normal threads, regardless of priority value. Priority level for SCHED_OTHER cannot be changed (it is dynamic), so the applied patch is fine. Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171877.
takuto.ikuta added a comment.
Rebased to take r345699
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,199 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VD44QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for quick fix!
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > Can you give an example for why this is needed?
> Sorry, this change does not need. Removed.
Sorry, this change is necessary still.
Without this, definition of inline function in explicit template instantiation
declaration is not be emitted, due to GVA_AvailableExternally linkage.
But we stop exporting definition of inline function in explicit template
instantiation definition too.
So without this, definition of dllimported inline function of explicit template
instantiation declaration won't be available.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > takuto.ikuta wrote:
> > > > > > > hans wrote:
> > > > > > > > takuto.ikuta wrote:
> > > > > > > > > hans wrote:
> > > > > > > > > > I still don't understand why we need these checks for
> > > > > > > > > > template instantiations. Why does it matter whether the
> > > > > > > > > > functions get inlined or not?
> > > > > > > > > When function of dllimported class is not inlined, such
> > > > > > > > > funtion needs to be dllexported from implementation library.
> > > > > > > > >
> > > > > > > > > c.h
> > > > > > > > > ```
> > > > > > > > > template
> > > > > > > > > class EXPORT C {
> > > > > > > > > public:
> > > > > > > > > void f() {}
> > > > > > > > > };
> > > > > > > > > ```
> > > > > > > > >
> > > > > > > > > cuser.cc
> > > > > > > > > ```
> > > > > > > > > #include "c.h"
> > > > > > > > >
> > > > > > > > > void cuser() {
> > > > > > > > > C c;
> > > > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > >
> > > > > > > > > When cuser.cc and c.h are built to different library,
> > > > > > > > > cuser.cc needs to be able to see dllexported C::f() when
> > > > > > > > > C::f() is not inlined.
> > > > > > > > > This is my understanding.
> > > > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > > > declaration, so doesn't apply here I think.
> > > > > > > >
> > > > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > > > fine. It doesn't matter whether the function gets inlined or
> > > > > > > > not, the only thing that matters is that if it's marked
> > > > > > > > dllimport on the "consumer side", it must be dllexport when
> > > > > > > > building the dll.
> > > > > > > Hmm, I changed the linkage for functions having
> > > > > > > DLLExport/ImportStaticLocal Attr.
> > > > > > >
> > > > > > I changed linkage in ASTContext so that inline function is emitted
> > > > > > when it is necessary when we use fno-dllexport-inlines.
> > > > > I revived explicit template instantiation check.
> > > > > Found that this is necessary.
> > > > >
> > > > > For explicit template instantiation, inheriting dll attribute from
> > > > > function for local static var is run before inheriting dll attribute
> > > > > from class for member functions. So I cannot detect local static var
> > > > > of inline function after class level dll attribute processing for
> > > > > explicit template instantiation.
> > > > >
> > > > Oh I see, it's a static local problem..
> > > > Can you provide a concrete example that does not work without your
> > > > check?
> > > > Maybe this is the right thing to do, but I would like to understand
> > > > exactly what the problem is.
> > > For the following code
> > > ```
> > > template
> > > class M{
> > > public:
> > >
> > > T Inclass() {
> > > static T static_x;
> > > ++static_x;
> > > return static_x;
> > > }
> > > };
> > >
> > > template class __declspec(dllexport) M;
> > >
> > > extern template class __declspec(dllimport) M;
> > >
> > > int f (){
> > > M mi;
> > > M ms;
> > > return mi.Inclass() + ms.Inclass();
> > > }
> > >
> > > ```
> > >
> > > llvm code without instantiation check become like below. Both inline
> > > functions of M and M is not dllimported/exported.
> > > ```
> > > $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
> > >
> > > $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
> > >
> > > @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local
> > > global i32 0, comdat, align 4
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define weak_odr dso_local i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %this)
> > > #0 comdat align 2 {
> > > entry
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; takuto.ikuta wrote: > takuto.ikuta wrote: > > hans wrote: > > > Can you give an example for why this is needed? > > Sorry, this change does not need. Removed. > Sorry, this change is necessary still. > > Without this, definition of inline function in explicit template > instantiation declaration is not be emitted, due to GVA_AvailableExternally > linkage. > But we stop exporting definition of inline function in explicit template > instantiation definition too. > > So without this, definition of dllimported inline function of explicit > template instantiation declaration won't be available. > Can you provide a code example of why this is needed? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value.
hjl.tools added inline comments.
Comment at: lib/CodeGen/TargetInfo.cpp:2862
// place naturally.
- if (!isAggregateTypeForABI(Ty)) {
+ if (!isAggregateTypeForABI(Ty) && !IsIllegalVectorType(Ty)) {
// Treat an enum type as its underlying type.
Doesn't -m32 have the same issue?
Repository:
rC Clang
https://reviews.llvm.org/D53919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
devnexen added a comment. I thought applying this sort of change for BSD but SCHED_IDLE is a GNU extension and does not have equivalence per see. Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
sammccall added a comment. In https://reviews.llvm.org/D53922#1281902, @krytarowski wrote: > In https://reviews.llvm.org/D53922#1281898, @sammccall wrote: > > > In https://reviews.llvm.org/D53922#1281882, @krytarowski wrote: > > > > > For NetBSD: > > > > > > - `ThreadPriority::Low` select either `SCHED_RR` or `SCHED_FIFO`, call > > > sched_get_priority_min() and set pthread_setschedparam(). > > > > > > https://www.netbsd.org/docs/internals/en/chap-processes.htm indicates > > `SCHED_RR` and `SCHED_FIFO` are real-time priorities on NetBSD, as they are > > on linux. > > i.e. higher priority than normal threads, regardless of priority value. > > > Priority level for SCHED_OTHER cannot be changed (it is dynamic), so the > applied patch is fine. Thanks. I was replying to your suggestion for NetBSD though - my reading of the docs says your "low" suggestion would end up being a very high priority. Is this not the case? Can you suggest a better reference for how BSD scheduling works? Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
hans added a comment.
Bah, I should have known something was going to break if I touched this :-)
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/37132
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/1017
Here's a repro:
template struct __declspec(dllimport) S {
void foo() {
static constexpr char src[] = {"hello"};
T arr[sizeof(src)];
}
};
void use() {
S s;
s.foo();
}
$ bin/clang -target i686-pc-win32 -c /tmp/a.cc
/tmp/a.cc:4:17: error: invalid application of 'sizeof' to an incomplete type
'char const[]'
T arr[sizeof(src)];
^
/tmp/a.cc:10:5: note: in instantiation of member function 'S::foo'
requested here
s.foo();
^
1 error generated.
This is annoying. Looking into it now.
Repository:
rC Clang
https://reviews.llvm.org/D53870
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53654: [clang] Improve ctor initializer completions.
ilya-biryukov added a comment.
Mostly NITs
Comment at: lib/Sema/SemaCodeComplete.cpp:5136
+ auto AddDefaultCtorInit = [&](const char *TypedName,
+const char *TypeName,
+const NamedDecl* ND) {
kadircet wrote:
> ilya-biryukov wrote:
> > Maybe use StringRef?
> Don't think this looks any better, since we need to use `.data()` at chunk
> additions than.
>
> Even if we try to push allocations all the way down until we add those
> chunks, we still get bad looking code since types are returned as
> `std::string` and we need to make a copy in the stack to be able to pass it
> as `StringRef`.
`char*` and `StringRef` have equivalent lifetime properties, so the set of
cases you need to do extra copies is equivalent. Am I missing something?
But yeah, since AddChunk methods accept a `char*`, `StringRef` is not suitable
because it's not guaranteed to be null-terminated. LG
Comment at: lib/Sema/SemaCodeComplete.cpp:813
+DeclContext::lookup_result getConstructorResults(ASTContext &Context,
+ const CXXRecordDecl *Record) {
Maybe make the name shorter?
E.g. `getConstructors` or `lookupConstructors`?
Comment at: lib/Sema/SemaCodeComplete.cpp:845
+ auto Ctors = getConstructorResults(SemaRef.Context, Record);
+ for(auto Ctor : Ctors) {
+R.Declaration = Ctor;
Maybe inline `Ctors`?
I.e.
```
for (auto Ctor : getConstructorResults(SemaRef.Context, Record))
```
Comment at: lib/Sema/SemaCodeComplete.cpp:5086
+ auto GenerateCCS = [&](const NamedDecl *ND, const char *Name) {
+Builder.AddTypedTextChunk(Name);
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
`Builder` is not used outside `GenerateCCS` and `AddDefaultCtorInit`, maybe
move its declaration into those lamdbas and remove from the enclosing function?
Would mean less mutable state to care about and the function seems big enough
for that to matter.
Comment at: lib/Sema/SemaCodeComplete.cpp:5088
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+if (const FunctionDecl *Function = dyn_cast(ND))
+ AddFunctionParameterChunks(PP, Policy, Function, Builder);
NIT: use auto
Comment at: lib/Sema/SemaCodeComplete.cpp:5090
+ AddFunctionParameterChunks(PP, Policy, Function, Builder);
+else if (const FunctionTemplateDecl *FunTemplDecl =
+ dyn_cast(ND))
NIT: use auto
Comment at: test/CodeCompletion/ctor-initializer.cpp:44
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:38:39 %s -o
- | FileCheck -check-prefix=CHECK-CC5 %s
+// CHECK-CC5-NOT: COMPLETION: Pattern : Base1()
+// CHECK-CC5-NOT: COMPLETION: Pattern : Base1(<#int#>)
Maybe use a single check `CHECK-CC5-NOT: COMPLETION: Pattern : Base`? (i.e. not
mentioning the arguments)
Comment at: test/CodeCompletion/ctor-initializer.cpp:69
struct Base2 {
- Base2(int);
+ Base2(int, float x = 3);
};
Why do we want to change this test?
Comment at: test/Index/complete-ctor-inits.cpp:33
// RUN: c-index-test -code-completion-at=%s:18:10 %s | FileCheck
-check-prefix=CHECK-CC1 %s
-// CHECK-CC1: MemberRef:{TypedText a}{LeftParen (}{Placeholder
args}{RightParen )} (35)
-// CHECK-CC1: MemberRef:{TypedText b}{LeftParen (}{Placeholder
args}{RightParen )} (35)
Reading through the code, it seems `MemberRef` was the correct CursorKind here
(the docs mention that MemberRef is a reference in non-expression context and
ctor-initializer seems to be one of those).
Maybe keep it a `MemberRef`, since it looks simple enough.
Having CXXConstrutor instead of NotImpleneted is clearly a win, though :-)
Repository:
rC Clang
https://reviews.llvm.org/D53654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r345709 - Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates
Author: hans
Date: Wed Oct 31 03:34:46 2018
New Revision: 345709
URL: http://llvm.org/viewvc/llvm-project?rev=345709&view=rev
Log:
Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates
Calling it too early might cause dllimport to get inherited onto the
VarDecl before the initializer got attached. See the test case for an
example where this broke things.
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=345709&r1=345708&r2=345709&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Oct 31 03:34:46 2018
@@ -728,9 +728,6 @@ Decl *TemplateDeclInstantiator::VisitVar
D->getLocation(), D->getIdentifier(), DI->getType(),
DI, D->getStorageClass());
- if (Var->isStaticLocal())
-SemaRef.CheckStaticLocalForDllExport(Var);
-
// In ARC, infer 'retaining' for variables of retainable type.
if (SemaRef.getLangOpts().ObjCAutoRefCount &&
SemaRef.inferObjCARCLifetime(Var))
@@ -751,6 +748,9 @@ Decl *TemplateDeclInstantiator::VisitVar
Var->setImplicit(D->isImplicit());
+ if (Var->isStaticLocal())
+SemaRef.CheckStaticLocalForDllExport(Var);
+
return Var;
}
Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=345709&r1=345708&r2=345709&view=diff
==
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Wed Oct 31 03:34:46 2018
@@ -1008,4 +1008,14 @@ template struct T { int foo()
extern template struct __declspec(dllimport) T;
int bar() { T t; return t.foo(); }
// MO1-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" =
available_externally dllimport global i32 0, align 4
+
+template struct __declspec(dllimport) U {
+ void foo() {
+// Don't inherit dllimport to src before attaching the initializer.
+static constexpr char src[] = {"hello"};
+T arr[sizeof(src)];
+ }
+};
+void baz() { U u; u.foo(); } // No diagnostic.
+
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
hans added a comment. r345709 should do it. Repository: rC Clang https://reviews.llvm.org/D53870 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Can you give an example for why this is needed?
> > > Sorry, this change does not need. Removed.
> > Sorry, this change is necessary still.
> >
> > Without this, definition of inline function in explicit template
> > instantiation declaration is not be emitted, due to GVA_AvailableExternally
> > linkage.
> > But we stop exporting definition of inline function in explicit template
> > instantiation definition too.
> >
> > So without this, definition of dllimported inline function of explicit
> > template instantiation declaration won't be available.
> >
> Can you provide a code example of why this is needed?
If we don't change function linkage, following code will be compiled like below
with -fno-dllexport-inlines.
```
template
class M{
public:
void foo() {}
};
template class __declspec(dllexport) M;
extern template class __declspec(dllimport) M;
void f (){
M mi;
mi.foo();
M ms;
ms.foo();
}
```
```
$"?foo@?$M@H@@QEAAXXZ" = comdat any
; Function Attrs: noinline nounwind optnone
define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
comdat align 2 {
entry:
%this.addr = alloca %class.M*, align 8
store %class.M* %this, %class.M** %this.addr, align 8
%this1 = load %class.M*, %class.M** %this.addr, align 8
ret void
}
; Function Attrs: noinline nounwind optnone
define dso_local void @"?f@@YAXXZ"() #0 {
entry:
%mi = alloca %class.M, align 1
%ms = alloca %class.M.0, align 1
call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
ret void
}
declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
```
M::foo() is declared, but inline function is not dllexported (See
M::foo() is not dllexported). So this code cannot be linked because
definition of M::foo does not exist. If the function is properly
inlined, we don't need to have this code. But I'm not sure why the function is
not inlined.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53925: [modules] Defer emission of inline key functions.
v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, bruno.
Herald added a subscriber: cfe-commits.
If the ABI supports inline key functions we must emit them eagerly. The generic
reason is that CodeGen might assume they are emitted and generate a reference
to the vtable. In deserialization this rule becomes a little less clear because
if a reference is generated, CodeGen will make a request and we will emit the
vtable on demand.
This patch improves the performance of loading modules by slimming down the
EAGERLY_DESERIALIZED_DECLS sections. It is equivalent to 'pinning' the vtable,
with two benefits: (a) it does not require expertise in modules to find out
that outlining the key function brings performance; (b) it also helps with
third-party codebases.
We ran this change through our codebase and we saw no observable changes in
behavior.
Patch by Yuka Takahashi and me!
Repository:
rC Clang
https://reviews.llvm.org/D53925
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/Serialization/ASTWriterDecl.cpp
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2264,6 +2264,19 @@
return false;
}
+ // If the ABI supports inline key functions we must emit them eagerly. The
+ // generic reason is that CodeGen might assume they are emitted and generate
a
+ // reference to the vtable. In deserialization this rule becomes a little
less
+ // clear because if a reference is generated, CodeGen will make a request and
+ // we will emit the vtable on demand.
+ //
+ // This change in behavior is driven mostly by performance considerations and
+ // is equivalent in outlining the key function (aka pinning the vtable). It
+ // also works in cases where we cannot modify the source code.
+ if (const CXXMethodDecl *MD = dyn_cast(D))
+if (Context.isKeyFunction(MD))
+ return false;
+
return Context.DeclMustBeEmitted(D);
}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9700,6 +9700,20 @@
basicGVALinkageForVariable(*this, VD)));
}
+bool ASTContext::isKeyFunction(const CXXMethodDecl *MD) {
+ assert(MD);
+
+ if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
+const CXXRecordDecl *RD = MD->getParent();
+if (MD->isOutOfLine() && RD->isDynamicClass()) {
+ const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
+ if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
+return true;
+}
+ }
+ return false;
+}
+
bool ASTContext::DeclMustBeEmitted(const Decl *D) {
if (const auto *VD = dyn_cast(D)) {
if (!VD->isFileVarDecl())
@@ -9781,18 +9795,11 @@
if (FD->hasAttr() || FD->hasAttr())
return true;
-// The key function for a class is required. This rule only comes
+// The key function for a class is required. This rule only comes
// into play when inline functions can be key functions, though.
-if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
- if (const auto *MD = dyn_cast(FD)) {
-const CXXRecordDecl *RD = MD->getParent();
-if (MD->isOutOfLine() && RD->isDynamicClass()) {
- const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
- if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
-return true;
-}
- }
-}
+if (const CXXMethodDecl *MD = dyn_cast(FD))
+ if (isKeyFunction(MD))
+return true;
GVALinkage Linkage = GetGVALinkageForFunction(FD);
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2733,6 +2733,12 @@
GVALinkage GetGVALinkageForFunction(const FunctionDecl *FD) const;
GVALinkage GetGVALinkageForVariable(const VarDecl *VD);
+ /// Determines if the decl is the key function for a class.
+ ///
+ /// \ returns true if the function is the key and it was inline requiring
+ /// emission even if it is not used.
+ bool isKeyFunction(const CXXMethodDecl *MD);
+
/// Determines if the decl can be CodeGen'ed or deserialized from PCH
/// lazily, only when used; this is only relevant for function or file scoped
/// var definitions.
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2264,6 +2264,19 @@
return false;
}
+ // If the ABI supports inline key functions we must emit them eagerly. The
+ // generic reason is that CodeGen might assume they are emitted and generate a
+ // reference to the vtable. In deserialization this rule becomes a little less
+ // c
[PATCH] D53866: [Preamble] Fix preamble for circular #includes
ilya-biryukov added a comment. Why does resetting the conditional stack is the right thing to do here? I can see how it can hide the problem, but can't come up with with a consistent model to handle the fact that the file contents were trimmed. Failing to build the preamble in that case seems like the best thing we could do. WDYT? Repository: rC Clang https://reviews.llvm.org/D53866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53925: [modules] Defer emission of inline key functions.
v.g.vassilev added a comment. @rsmith, @bruno could you verify this patch on your codebases? Repository: rC Clang https://reviews.llvm.org/D53925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)
lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D52695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans added a comment. In https://reviews.llvm.org/D53457#1279191, @neerajksingh wrote: > In https://reviews.llvm.org/D53457#1278579, @hans wrote: > > > The `-Xclang` option has the same issue, and I think `/clang:` should work > > similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at > > least it's consistent. Yes, that means processing consecutive `/Xclang:` > > options together, but hopefully that's doable. > > > It looks like the handling for -Xclang is a lot simpler (in > `Clang::ConstructJob`). There the Xclang options are all gathered and > forwarded at a particular spot in the command line for cc1. They aren't > interleaved with other options, and it wouldn't really make sense to do so > anyway since it doesn't really look like cc1 arguments are constructed from > driver arguments in any particular order. > > The llvm/lib/Option/OptTable.cpp code is not really setup to easily > interleave arguments like would be required for your request. Can you see a > way to accomplish what you want without a deep refactoring of > OptTable::ParseArgs and the InputArgList class? Okay, if it's hard to do, I suppose collecting the /clang: options and processing them separately after the others is the second-best option. We'll need to make sure the behaviour is well-documented. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This avoids duplicated scopes in the query e.g. when anonymous namespace
is present.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
Files:
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53705: [OpenCL] Postpone PSV address space diagnostic
Anastasia added a comment. FYI, I have created a bug to the OpenCL C++ spec: https://github.com/KhronosGroup/OpenCL-Docs/issues/13. Gladly, we now moved into discussing the issues publicly. So anyone can create bugs and also participate in discussions. I would like to feed all of the suggestions back and try to fix problematic parts of the language as much as possible. Repository: rC Clang https://reviews.llvm.org/D53705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions
steleman created this revision.
steleman added a reviewer: joel_k_jones.
Herald added subscribers: kristina, kristof.beyls, javed.absar.
This changeset adds the required Builtins for enabling AArch64 vectorization of
libm trigonometry functions via SLEEF: http://sleef.org/.
- A new argument is added to -fveclib=: SLEEF.
- A number of Builtins that were previously unimplemented are now enabled.
This changeset depends on https://reviews.llvm.org/D53927.
List of SLEEF vectorized trigonometry functions:
acos
asin
atan
atanh
cos
cosh
exp
exp2
exp10
lgamma
log10
log2
log
pow
sin
sinh
sqrt
tan
tanh
tgamma
Repository:
rC Clang
https://reviews.llvm.org/D53928
Files:
include/clang/Basic/Builtins.def
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Frontend/CompilerInvocation.cpp
Index: include/clang/Frontend/CodeGenOptions.h
===
--- include/clang/Frontend/CodeGenOptions.h
+++ include/clang/Frontend/CodeGenOptions.h
@@ -54,7 +54,8 @@
enum VectorLibrary {
NoLibrary, // Don't use any vector library.
Accelerate, // Use the Accelerate framework.
-SVML// Intel short vector math library.
+SVML, // Intel short vector math library.
+SLEEF // SLEEF - SIMD Library for Evaluating Elementary Functions.
};
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1388,7 +1388,7 @@
Group, Flags<[CC1Option]>,
HelpText<"Disables an experimental new pass manager in LLVM.">;
def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>,
-HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,none">;
+HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,SLEEF,none">;
def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, Group,
HelpText<"Disallow implicit conversions between vectors with a different number of elements or different element types">, Flags<[CC1Option]>;
def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group,
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -196,6 +196,9 @@
BUILTIN(__builtin_exp2 , "dd" , "Fne")
BUILTIN(__builtin_exp2f, "ff" , "Fne")
BUILTIN(__builtin_exp2l, "LdLd", "Fne")
+BUILTIN(__builtin_exp10 , "dd" , "Fne")
+BUILTIN(__builtin_exp10f, "ff" , "Fne")
+BUILTIN(__builtin_exp10l, "LdLd", "Fne")
BUILTIN(__builtin_expm1 , "dd", "Fne")
BUILTIN(__builtin_expm1f, "ff", "Fne")
BUILTIN(__builtin_expm1l, "LdLd", "Fne")
@@ -1133,6 +1136,10 @@
LIBBUILTIN(exp2f, "ff", "fne", "math.h", ALL_LANGUAGES)
LIBBUILTIN(exp2l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
+
LIBBUILTIN(expm1, "dd", "fne", "math.h", ALL_LANGUAGES)
LIBBUILTIN(expm1f, "ff", "fne", "math.h", ALL_LANGUAGES)
LIBBUILTIN(expm1l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
@@ -1370,10 +1377,6 @@
LIBBUILTIN(__tanpi, "dd", "fne", "math.h", ALL_LANGUAGES)
LIBBUILTIN(__tanpif, "ff", "fne", "math.h", ALL_LANGUAGES)
-// Similarly, __exp10 is OS X only
-LIBBUILTIN(__exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(__exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
-
// Blocks runtime Builtin math library functions
LIBBUILTIN(_Block_object_assign, "vv*vC*iC", "f", "Blocks.h", ALL_LANGUAGES)
LIBBUILTIN(_Block_object_dispose, "vvC*iC", "f", "Blocks.h", ALL_LANGUAGES)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -557,6 +557,8 @@
Opts.setVecLib(CodeGenOptions::Accelerate);
else if (Name == "SVML")
Opts.setVecLib(CodeGenOptions::SVML);
+else if (Name == "SLEEF")
+ Opts.setVecLib(CodeGenOptions::SLEEF);
else if (Name == "none")
Opts.setVecLib(CodeGenOptions::NoLibrary);
else
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -344,6 +344,9 @@
case CodeGenOptions::SVML:
TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::SVML);
break;
+ case CodeGenOptions::SLEEF:
+TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::SLEEF);
+break;
default:
break;
}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1306,14 +1306,70 @@
[PATCH] D53866: [Preamble] Fix preamble for circular #includes
nik added a comment. In https://reviews.llvm.org/D53866#1281978, @ilya-biryukov wrote: > Why does resetting the conditional stack is the right thing to do here? Because this case can be detected and handled without loosing the benefits of the preamble. > Failing to build the preamble in that case seems like the best thing we could > do. WDYT? See above - we would not have the benefits of the preamble anymore. > I can see how it can hide the problem, but can't come up with with a > consistent model to handle the fact that the file contents were trimmed. If we are in preamble-generation-mode and the main file #includes itself, then it should see the full file content of the file, not the truncated preamble version of it. Would this be more consistent? Repository: rC Clang https://reviews.llvm.org/D53866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53922: [clangd] fix non linux build
krytarowski added a comment. In https://reviews.llvm.org/D53922#1281914, @sammccall wrote: > Thanks. I was replying to your suggestion for NetBSD though - my reading of > the docs says your "low" suggestion would end up being a very high priority. > Is this not the case? Can you suggest a better reference for how BSD > scheduling works? This reference is fine and the patch restricting it to Linux is fine. I think that there is no way to implement it for NetBSD, also it does not seem to be a crucial feature. Repository: rL LLVM https://reviews.llvm.org/D53922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace
alexfh added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:30
+ if (const NamedDecl *AN = Node.getAliasedNamespace()) {
+// If this aliases to an actual namespace, check if its std.
+if (const auto *N = dyn_cast(AN))
s/its/it's/, but maybe rephrase this a bit more: ", check that the target
namespace of the alias is the `std` namespace.".
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:176
+
+ Warns when the `std` namespace is used, as its use is against Zircon's libc++
+ policy for the kernel.
JonasToth wrote:
> s/its/it's/
>
> Could `std` be considered code here? Not sure, but maybe using quotes is
> better?
Actually, "its" is correct in this context ("its use" vs "it's used").
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:177
+ Warns when the `std` namespace is used, as its use is against Zircon's libc++
+ policy for the kernel.
+
I guess that you're referring here to this wording from
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md: "Zircon does not
use the C++ standard library". If so, it's better to avoid using the term
`libc++`, which is the name of one particular implementation of the C++
standard library (http://libcxx.llvm.org/docs/).
https://reviews.llvm.org/D53882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53025: [clang-tidy] implement new check for const return types.
ymandel updated this revision to Diff 171897.
ymandel marked 2 inline comments as done.
ymandel added a comment.
Small tweaks in response to comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53025
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ConstValueReturnCheck.cpp
clang-tidy/readability/ConstValueReturnCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-const-value-return.rst
test/clang-tidy/readability-const-value-return.cpp
Index: test/clang-tidy/readability-const-value-return.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-const-value-return.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s readability-const-value-return %t -- -- -isystem
+
+// p# = positive test
+// n# = negative test
+
+#include
+
+const int p1() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
+// CHECK-FIXES: int p1() {
+ return 1;
+}
+
+const int p15();
+// CHECK-FIXES: int p15();
+
+template
+const int p31(T v) { return 2; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// CHECK-FIXES: int p31(T v) { return 2; }
+
+// We detect const-ness even without instantiating T.
+template
+const T p32(T t) { return t; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const T' is 'const'-qual
+// CHECK-FIXES: T p32(T t) { return t; }
+
+// However, if the return type is itself a template instantiation, Clang does
+// not consider it const-qualified without knowing `T`.
+template
+typename std::add_const::type n15(T v) { return v; }
+
+template
+class Klazz {
+public:
+ Klazz(A) {}
+};
+
+class Clazz {
+ public:
+ Clazz *const p2() {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'Clazz *const' is 'co
+// CHECK-FIXES: Clazz *p2() {
+return this;
+ }
+
+ Clazz *const p3();
+ // CHECK-FIXES: Clazz *p3();
+
+ const int p4() const {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const
+// CHECK-FIXES: int p4() const {
+return 4;
+ }
+
+ const Klazz* const p5() const;
+ // CHECK-FIXES: const Klazz* p5() const;
+
+ const Clazz operator++(int x) { // p12
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz' is 'const
+ // CHECK-FIXES: Clazz operator++(int x) {
+ }
+
+ struct Strukt {
+int i;
+ };
+
+ const Strukt p6() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+ // CHECK-FIXES: Strukt p6() {}
+
+ // No warning is emitted here, because this is only the declaration. The
+ // warning will be associated with the definition, below.
+ const Strukt* const p7();
+ // CHECK-FIXES: const Strukt* p7();
+
+ // const-qualifier is the first `const` token, but not the first token.
+ static const int p8() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-
+ // CHECK-FIXES: static int p8() {}
+
+ static const Strukt p9() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+ // CHECK-FIXES: static Strukt p9() {}
+
+ int n0() const { return 0; }
+ const Klazz& n11(const Klazz) const;
+};
+
+Clazz *const Clazz::p3() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+ // CHECK-FIXES: Clazz *Clazz::p3() {
+ return this;
+}
+
+const Klazz* const Clazz::p5() const {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* Clazz::p5() const {}
+
+const Clazz::Strukt* const Clazz::p7() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz::Strukt *con
+// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {}
+
+Clazz *const p10();
+// CHECK-FIXES: Clazz *p10();
+
+Clazz *const p10() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+ // CHECK-FIXES: Clazz *p10() {
+ return new Clazz();
+}
+
+const Clazz bar;
+const Clazz *const p11() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz *const' is
+ // CHECK-FIXES: const Clazz *p11() {
+ return &bar;
+}
+
+const Klazz p12() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz'
+// CHECK-FIXES: Klazz p12() {}
+
+const Klazz* const p13() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* p13() {}
+
+// re-declaration of p15.
+const int p15();
+// CHECK-FIXES: int p15();
+
+const int p15() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: int p15() {
+ return 0;
+}
+
+// Exercise the lexer.
+
+const /* comment */ /* another comment*/ int p16() { return 0; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+//
[PATCH] D53025: [clang-tidy] implement new check for const return types.
ymandel added a comment. In https://reviews.llvm.org/D53025#1281097, @aaron.ballman wrote: > I think this is getting really close! One question: would it make more sense > to name this `readability-const-type-return` or > `readability-const-return-type` instead? It's not that the functions return a > const *value* that's the issue, it's that the declared return type is > top-level const. I think removing "value" and using "type" instead would be > an improvement (and similarly, rename the files and the check). Yes, that sounds good. Will do that in a separate diff so you can see my changes in reply to your comments first. In https://reviews.llvm.org/D53025#1281107, @JonasToth wrote: > Am 30.10.18 um 21:47 schrieb Aaron Ballman via Phabricator: > > > aaron.ballman added a comment. > > > > I think this is getting really close! One question: would it make more > > sense to name this `readability-const-type-return` or > > `readability-const-return-type` instead? It's not that the functions return > > a const *value* that's the issue, it's that the declared return type is > > top-level const. I think removing "value" and using "type" instead would be > > an improvement (and similarly, rename the files and the check). > > There is a `rename-check.py` script in the repository. Just to prevent a > lot of manual work ;) Jonas -- thanks for the tip! Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getReturnType(); JonasToth wrote: > ymandel wrote: > > aaron.ballman wrote: > > > ymandel wrote: > > > > aaron.ballman wrote: > > > > > ymandel wrote: > > > > > > aaron.ballman wrote: > > > > > > > ymandel wrote: > > > > > > > > aaron.ballman wrote: > > > > > > > > > ymandel wrote: > > > > > > > > > > aaron.ballman wrote: > > > > > > > > > > > It seems strange to me that this is in the readability > > > > > > > > > > > module but the diagnostic here is about compiler > > > > > > > > > > > optimizations. Should this be in the performance module > > > > > > > > > > > instead? Or should this diagnostic be reworded about the > > > > > > > > > > > readability concerns? > > > > > > > > > > Good point. The readability angle is that the const > > > > > > > > > > clutters the code to no benefit. That is, even if it was > > > > > > > > > > performance neutral, I'd still want to discourage this > > > > > > > > > > practice. But, it does seem like the primary impact is > > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > > > I'm fine either way -- moving it to performance or > > > > > > > > > > emphasizing the clutter of the unhelpful "const". I'm > > > > > > > > > > inclined to moving it, but don't have good sense of how > > > > > > > > > > strict these hierarchies are. What do you think? > > > > > > > > > I'm not sold that `const`-qualified return types always > > > > > > > > > pessimize optimizations. However, I'm also not sold that it's > > > > > > > > > *always* a bad idea to have a top-level cv-qualifier on a > > > > > > > > > return type either (for instance, this is one way to prevent > > > > > > > > > callers from modifying a temp returned by a function). How > > > > > > > > > about leaving this in readability and changing the diagnostic > > > > > > > > > into: "top-level 'const' qualifier on a return type may > > > > > > > > > reduce code readability with limited benefit" or something > > > > > > > > > equally weasely? > > > > > > > > I talked this over with other google folks and seems like the > > > > > > > > consensus is: > > > > > > > > > > > > > > > > 1. The readability benefits may be controversial. Some folks > > > > > > > > might not view `const` as clutter and there are some corner > > > > > > > > cases where the qualifier may prevent something harmful. > > > > > > > > 2. The performance implication is real, if not guaranteed to > > > > > > > > be a problem. > > > > > > > > > > > > > > > > Based on this, seems best to move it to performance, but water > > > > > > > > down the performance claims. That keeps the focus to an issue > > > > > > > > we can assume nearly everyone agrees on. > > > > > > > I'm not convinced the performance implications are real compared > > > > > > > to how the check is currently implemented. I know there are > > > > > > > performance concerns when you return a const value of class type > > > > > > > because it pessimizes the ability to use move constructors, but > > > > > > > that's a very specific performance concern that I don't believe > > > > > > > is present in general. For instance, I don't know of any > > > > > > > performance concerns regarding `const int f();` as a declaration. > > > > > > > I'd be fine moving this to the performance module, but I think > > > > > > > the check would
[PATCH] D53654: [clang] Improve ctor initializer completions.
kadircet updated this revision to Diff 171899.
kadircet marked 8 inline comments as done.
kadircet added a comment.
- Address comments.
Repository:
rC Clang
https://reviews.llvm.org/D53654
Files:
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/ctor-initializer.cpp
test/Index/complete-ctor-inits.cpp
test/Index/complete-cxx-inline-methods.cpp
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -21,6 +21,13 @@
int value;
MyCls *object;
};
+
+template
+class X {};
+
+class Y : public X {
+ Y() : X() {}
+};
}
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
@@ -35,10 +42,12 @@
// CHECK-NEXT: Container Kind: StructDecl
// RUN: c-index-test -code-completion-at=%s:18:41 %s | FileCheck -check-prefix=CHECK-CTOR-INIT %s
-// CHECK-CTOR-INIT: NotImplemented:{TypedText MyCls}{LeftParen (}{Placeholder args}{RightParen )} (7)
-// CHECK-CTOR-INIT: MemberRef:{TypedText object}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CTOR-INIT: MemberRef:{TypedText value}{LeftParen (}{Placeholder args}{RightParen )} (35)
+// CHECK-CTOR-INIT: ClassDecl:{TypedText MyCls}{LeftParen (}{Placeholder MyCls}{RightParen )} (7)
+// CHECK-CTOR-INIT: MemberRef:{TypedText object}{LeftParen (}{Placeholder MyCls *}{RightParen )} (35)
+// CHECK-CTOR-INIT: MemberRef:{TypedText value}{LeftParen (}{Placeholder int}{RightParen )} (35)
// RUN: c-index-test -code-completion-at=%s:18:55 %s | FileCheck -check-prefix=CHECK-CTOR-INIT-2 %s
-// CHECK-CTOR-INIT-2-NOT: NotImplemented:{TypedText MyCls}{LeftParen (}{Placeholder args}{RightParen )}
-// CHECK-CTOR-INIT-2: MemberRef:{TypedText object}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CTOR-INIT-2: MemberRef:{TypedText value}{LeftParen (}{Placeholder args}{RightParen )} (7)
+// CHECK-CTOR-INIT-2-NOT: ClassDecl:{TypedText MyCls}{LeftParen (}{Placeholder MyCls}{RightParen )} (7)
+// CHECK-CTOR-INIT-2: MemberRef:{TypedText object}{LeftParen (}{Placeholder MyCls *}{RightParen )} (35)
+// CHECK-CTOR-INIT-2: MemberRef:{TypedText value}{LeftParen (}{Placeholder int}{RightParen )} (7)
+// RUN: c-index-test -code-completion-at=%s:29:9 %s | FileCheck -check-prefix=CHECK-CTOR-INIT-3 %s
+// CHECK-CTOR-INIT-3: ClassDecl:{TypedText X}{LeftParen (}{Placeholder X}{RightParen )} (7)
Index: test/Index/complete-ctor-inits.cpp
===
--- test/Index/complete-ctor-inits.cpp
+++ test/Index/complete-ctor-inits.cpp
@@ -30,27 +30,33 @@
};
// RUN: c-index-test -code-completion-at=%s:18:10 %s | FileCheck -check-prefix=CHECK-CC1 %s
-// CHECK-CC1: MemberRef:{TypedText a}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: MemberRef:{TypedText b}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: MemberRef:{TypedText c}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: NotImplemented:{TypedText Virt}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: NotImplemented:{TypedText X}{LeftParen (}{Placeholder args}{RightParen )} (7)
-// CHECK-CC1: NotImplemented:{TypedText Y}{LeftParen (}{Placeholder args}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText a}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText b}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText c}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder const Virt &}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder Virt &&}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText X}{LeftParen (}{Placeholder int}{RightParen )} (7)
+// CHECK-CC1: CXXConstructor:{TypedText Y}{LeftParen (}{Placeholder const Y &}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Y}{LeftParen (}{Placeholder Y &&}{RightParen )} (35)
// RUN: c-index-test -code-completion-at=%s:18:23 %s | FileCheck -check-prefix=CHECK-CC2 %s
-// CHECK-CC2: MemberRef:{TypedText a}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: MemberRef:{TypedText b}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: MemberRef:{TypedText c}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: NotImplemented:{TypedText Virt}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: NotImplemented:{TypedText Y}{LeftParen (}{Placeholder args}{RightParen )} (7)
+// CHECK-CC2: MemberRef:{TypedText a}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: MemberRef:{TypedText b}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: MemberRef:{TypedText c}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder const Virt &}{RightParen )} (35)
+// CHECK
[PATCH] D53025: [clang-tidy] implement new check for const return types.
ymandel updated this revision to Diff 171900.
ymandel added a comment.
Rename readability-const-value-return to readability-const-return-type
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53025
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ConstReturnTypeCheck.cpp
clang-tidy/readability/ConstReturnTypeCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-const-return-type.rst
test/clang-tidy/readability-const-return-type.cpp
Index: test/clang-tidy/readability-const-return-type.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-const-return-type.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s readability-const-return-type %t -- -- -isystem
+
+// p# = positive test
+// n# = negative test
+
+#include
+
+const int p1() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
+// CHECK-FIXES: int p1() {
+ return 1;
+}
+
+const int p15();
+// CHECK-FIXES: int p15();
+
+template
+const int p31(T v) { return 2; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// CHECK-FIXES: int p31(T v) { return 2; }
+
+// We detect const-ness even without instantiating T.
+template
+const T p32(T t) { return t; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const T' is 'const'-qual
+// CHECK-FIXES: T p32(T t) { return t; }
+
+// However, if the return type is itself a template instantiation, Clang does
+// not consider it const-qualified without knowing `T`.
+template
+typename std::add_const::type n15(T v) { return v; }
+
+template
+class Klazz {
+public:
+ Klazz(A) {}
+};
+
+class Clazz {
+ public:
+ Clazz *const p2() {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'Clazz *const' is 'co
+// CHECK-FIXES: Clazz *p2() {
+return this;
+ }
+
+ Clazz *const p3();
+ // CHECK-FIXES: Clazz *p3();
+
+ const int p4() const {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const
+// CHECK-FIXES: int p4() const {
+return 4;
+ }
+
+ const Klazz* const p5() const;
+ // CHECK-FIXES: const Klazz* p5() const;
+
+ const Clazz operator++(int x) { // p12
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz' is 'const
+ // CHECK-FIXES: Clazz operator++(int x) {
+ }
+
+ struct Strukt {
+int i;
+ };
+
+ const Strukt p6() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+ // CHECK-FIXES: Strukt p6() {}
+
+ // No warning is emitted here, because this is only the declaration. The
+ // warning will be associated with the definition, below.
+ const Strukt* const p7();
+ // CHECK-FIXES: const Strukt* p7();
+
+ // const-qualifier is the first `const` token, but not the first token.
+ static const int p8() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-
+ // CHECK-FIXES: static int p8() {}
+
+ static const Strukt p9() {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+ // CHECK-FIXES: static Strukt p9() {}
+
+ int n0() const { return 0; }
+ const Klazz& n11(const Klazz) const;
+};
+
+Clazz *const Clazz::p3() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+ // CHECK-FIXES: Clazz *Clazz::p3() {
+ return this;
+}
+
+const Klazz* const Clazz::p5() const {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* Clazz::p5() const {}
+
+const Clazz::Strukt* const Clazz::p7() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz::Strukt *con
+// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {}
+
+Clazz *const p10();
+// CHECK-FIXES: Clazz *p10();
+
+Clazz *const p10() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+ // CHECK-FIXES: Clazz *p10() {
+ return new Clazz();
+}
+
+const Clazz bar;
+const Clazz *const p11() {
+ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz *const' is
+ // CHECK-FIXES: const Clazz *p11() {
+ return &bar;
+}
+
+const Klazz p12() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz'
+// CHECK-FIXES: Klazz p12() {}
+
+const Klazz* const p13() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* p13() {}
+
+// re-declaration of p15.
+const int p15();
+// CHECK-FIXES: int p15();
+
+const int p15() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: int p15() {
+ return 0;
+}
+
+// Exercise the lexer.
+
+const /* comment */ /* another comment*/ int p16() { return 0; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: /*
[PATCH] D53025: [clang-tidy] implement new check for const return types.
ymandel added a comment. I've renamed the check. I noticed that the script did not rename the header guard macro, though. Please let me know if/where to file a bug. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53654: [clang] Improve ctor initializer completions.
kadircet added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:5136
+ auto AddDefaultCtorInit = [&](const char *TypedName,
+const char *TypeName,
+const NamedDecl* ND) {
ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Maybe use StringRef?
> > Don't think this looks any better, since we need to use `.data()` at chunk
> > additions than.
> >
> > Even if we try to push allocations all the way down until we add those
> > chunks, we still get bad looking code since types are returned as
> > `std::string` and we need to make a copy in the stack to be able to pass it
> > as `StringRef`.
> `char*` and `StringRef` have equivalent lifetime properties, so the set of
> cases you need to do extra copies is equivalent. Am I missing something?
> But yeah, since AddChunk methods accept a `char*`, `StringRef` is not
> suitable because it's not guaranteed to be null-terminated. LG
Yes, but I store those strings inside allocator and pass pointers of those
stored ones. If I were to defer the allocation, I would need to keep copies of
the strings myself until I call allocate on them.
Comment at: test/CodeCompletion/ctor-initializer.cpp:69
struct Base2 {
- Base2(int);
+ Base2(int, float x = 3);
};
ilya-biryukov wrote:
> Why do we want to change this test?
This was for something else I've been testing, nvm.
The change below is however, to get rid of the compile error.
Repository:
rC Clang
https://reviews.llvm.org/D53654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r345716 - [clang-tidy] Remove false decoupling in ClangTidyContext. NFC
Author: sammccall
Date: Wed Oct 31 06:08:19 2018
New Revision: 345716
URL: http://llvm.org/viewvc/llvm-project?rev=345716&view=rev
Log:
[clang-tidy] Remove false decoupling in ClangTidyContext. NFC
These getters/setters don't encapsulate any behavior, and can only be
called by friends.
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=345716&r1=345715&r2=345716&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Wed Oct
31 06:08:19 2018
@@ -199,10 +199,6 @@ DiagnosticBuilder ClangTidyContext::diag
return DiagEngine->Report(Loc, ID);
}
-void ClangTidyContext::setDiagnosticsEngine(DiagnosticsEngine *Engine) {
- DiagEngine = Engine;
-}
-
void ClangTidyContext::setSourceManager(SourceManager *SourceMgr) {
DiagEngine->setSourceManager(SourceMgr);
}
@@ -259,11 +255,6 @@ bool ClangTidyContext::treatAsError(Stri
return WarningAsErrorFilter->contains(CheckName);
}
-/// \brief Store a \c ClangTidyError.
-void ClangTidyContext::storeError(const ClangTidyError &Error) {
- Errors.push_back(Error);
-}
-
StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
llvm::DenseMap::const_iterator I =
CheckNamesByDiagnosticID.find(DiagnosticID);
@@ -281,7 +272,7 @@ ClangTidyDiagnosticConsumer::ClangTidyDi
Diags = llvm::make_unique(
IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, this,
/*ShouldOwnClient=*/false);
- Context.setDiagnosticsEngine(Diags.get());
+ Context.DiagEngine = Diags.get();
}
void ClangTidyDiagnosticConsumer::finalizeLastError() {
@@ -684,7 +675,6 @@ void ClangTidyDiagnosticConsumer::finish
if (RemoveIncompatibleErrors)
removeIncompatibleErrors(Errors);
- for (const ClangTidyError &Error : Errors)
-Context.storeError(Error);
+ std::move(Errors.begin(), Errors.end(), std::back_inserter(Context.Errors));
Errors.clear();
}
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=345716&r1=345715&r2=345716&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Wed Oct 31
06:08:19 2018
@@ -192,17 +192,10 @@ public:
}
private:
- // Calls setDiagnosticsEngine() and storeError().
+ // Sets DiagEngine and Errors.
friend class ClangTidyDiagnosticConsumer;
friend class ClangTidyPluginAction;
- /// \brief Sets the \c DiagnosticsEngine so that Diagnostics can be generated
- /// correctly.
- void setDiagnosticsEngine(DiagnosticsEngine *Engine);
-
- /// \brief Store an \p Error.
- void storeError(const ClangTidyError &Error);
-
std::vector Errors;
DiagnosticsEngine *DiagEngine;
std::unique_ptr OptionsProvider;
Modified: clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp?rev=345716&r1=345715&r2=345716&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp Wed Oct 31
06:08:19 2018
@@ -35,7 +35,7 @@ public:
std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
StringRef File) override {
// Insert the current diagnostics engine.
-Context->setDiagnosticsEngine(&Compiler.getDiagnostics());
+Context->DiagEngine = &Compiler.getDiagnostics();
// Create the AST consumer.
ClangTidyASTConsumerFactory Factory(*Context);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.
ioeric updated this revision to Diff 171901.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Merged with multi-threading changes. Added mutex for file digests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/dex/Dex.h
unittests/clangd/BackgroundIndexTests.cpp
unittests/clangd/DexTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -52,6 +52,7 @@
SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
+RefSlab getRefs(const SymbolIndex &Index, SymbolID ID);
} // namespace clangd
} // namespace clang
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "SyncAPI.h"
+#include "index/Index.h"
using namespace llvm;
namespace clang {
@@ -138,5 +139,14 @@
return std::move(Builder).build();
}
+RefSlab getRefs(const SymbolIndex &Index, SymbolID ID) {
+ RefsRequest Req;
+ Req.IDs = {ID};
+ RefSlab::Builder Slab;
+ Index.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); });
+ return std::move(Slab).build();
+}
+
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -14,6 +14,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "index/Index.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Frontend/Utils.h"
@@ -39,6 +40,7 @@
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.FileURI == U; }
+MATCHER_P(DefURI, U, "") { return arg.Definition.FileURI == U; }
MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
using namespace llvm;
@@ -73,14 +75,6 @@
return llvm::make_unique(std::move(Slab).build());
}
-RefSlab getRefs(const SymbolIndex &I, SymbolID ID) {
- RefsRequest Req;
- Req.IDs = {ID};
- RefSlab::Builder Slab;
- I.refs(Req, [&](const Ref &S) { Slab.insert(ID, S); });
- return std::move(Slab).build();
-}
-
TEST(FileSymbolsTest, UpdateAndGet) {
FileSymbols FS;
EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty());
@@ -102,6 +96,27 @@
QName("4"), QName("5")));
}
+TEST(FileSymbolsTest, MergeOverlap) {
+ FileSymbols FS;
+ auto OneSymboSlab = [](Symbol Sym) {
+SymbolSlab::Builder S;
+S.insert(Sym);
+return make_unique(std::move(S).build());
+ };
+ auto X1 = symbol("x");
+ X1.CanonicalDeclaration.FileURI = "file:///x1";
+ auto X2 = symbol("x");
+ X2.Definition.FileURI = "file:///x2";
+
+ FS.update("f1", OneSymboSlab(X1), nullptr);
+ FS.update("f2", OneSymboSlab(X2), nullptr);
+ for (auto Type : {IndexType::Light, IndexType::Heavy})
+EXPECT_THAT(
+runFuzzyFind(*FS.buildIndex(Type, DuplicateHandling::Merge), "x"),
+UnorderedElementsAre(
+AllOf(QName("x"), DeclURI("file:///x1"), DefURI("file:///x2";
+}
+
TEST(FileSymbolsTest, SnapshotAliveAfterRemove) {
FileSymbols FS;
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -491,15 +491,6 @@
"other::A"));
}
-TEST(DexTest, DexDeduplicate) {
- std::vector Symbols = {symbol("1"), symbol("2"), symbol("3"),
- symbol("2") /* duplicate */};
- FuzzyFindRequest Req;
- Req.Query = "2";
- Dex I(Symbols, RefSlab(), URISchemes);
- EXPECT_THAT(match(I, Req), ElementsAre("2"));
-}
-
TEST(DexTest, DexLimitedNumMatches) {
auto I = Dex::build(generateNumSymbols(0, 100), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -4,33 +4,76 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+using testing::_;
+using testing::AllOf;
+using testing::Not;
using testing::UnorderedElementsAre;
namespace clang {
namespace clangd {
MATCHER_P(Named, N, "") { return arg
[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.
ioeric added a comment.
Made some more changes to make the code work in multiple threads (add mutex for
digests, take snapshot of digests for each run etc). PTAL
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::move(Symbols)),
-llvm::make_unique(std::move(Refs)));
- FileHash[AbsolutePath] = Hash;
+ update(AbsolutePath, std::move(Symbols), std::move(Refs), FilesToUpdate);
+ IndexedFileDigests[AbsolutePath] = Hash;
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > It looks like you never write the FilesToUpdate hashes back to the
> > > IndexedFileDigests, so we'll always update headers?
> > It's updated in `update` when slabs are updated.
> In that case, do we need to update IndexedFileDigests here too?
>
> (I just realized there's an edge case: what if there are no symbols or
> references in a file? but I'm not sure if this is different between mainfile
> and headers, either)
The edge case seems to justify the update here. We want to make sure that hash
for main files are always updated, even when there's no index data from it.
Comment at: clangd/index/FileIndex.cpp:140
+ }
+ case DuplicateHandling::PickOne: {
+for (const auto &Slab : SymbolSlabs)
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > since we're deduplicating above, we could deduplicate here too and remove
> > > the deduplicating logic from Dex (MemIndex gets it by mistake). That
> > > would avoid duplicated deduplication (!) in the Merge + Dex case, and
> > > seems a little cleaner.
> > >
> > > Not something for this patch, but maybe add a FIXME.
> > Added the duduplication for the PickOne case.
> Thanks! The FIXME is still warranted, to remove deduping from Dex.
Done. I removed it in this patch as it seems trivial enough.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > Can you give an example for why this is needed?
> > > > Sorry, this change does not need. Removed.
> > > Sorry, this change is necessary still.
> > >
> > > Without this, definition of inline function in explicit template
> > > instantiation declaration is not be emitted, due to
> > > GVA_AvailableExternally linkage.
> > > But we stop exporting definition of inline function in explicit template
> > > instantiation definition too.
> > >
> > > So without this, definition of dllimported inline function of explicit
> > > template instantiation declaration won't be available.
> > >
> > Can you provide a code example of why this is needed?
> If we don't change function linkage, following code will be compiled like
> below with -fno-dllexport-inlines.
>
> ```
> template
> class M{
> public:
> void foo() {}
> };
>
> template class __declspec(dllexport) M;
>
> extern template class __declspec(dllimport) M;
>
> void f (){
> M mi;
> mi.foo();
>
> M ms;
> ms.foo();
> }
> ```
>
> ```
> $"?foo@?$M@H@@QEAAXXZ" = comdat any
>
> ; Function Attrs: noinline nounwind optnone
> define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> comdat align 2 {
> entry:
> %this.addr = alloca %class.M*, align 8
> store %class.M* %this, %class.M** %this.addr, align 8
> %this1 = load %class.M*, %class.M** %this.addr, align 8
> ret void
> }
>
> ; Function Attrs: noinline nounwind optnone
> define dso_local void @"?f@@YAXXZ"() #0 {
> entry:
> %mi = alloca %class.M, align 1
> %ms = alloca %class.M.0, align 1
> call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> ret void
> }
>
> declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> ```
>
> M::foo() is declared, but inline function is not dllexported (See
> M::foo() is not dllexported). So this code cannot be linked because
> definition of M::foo does not exist. If the function is properly
> inlined, we don't need to have this code. But I'm not sure why the function
> is not inlined.
Interesting. I wonder how -fvisibility-inlines-hidden handles this...
```
template struct S {
void foo() {}
};
template struct S;
void use() {
S s;
s.foo();
}
$ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
_ZN1SIiE3fooEv
ld .text._ZN1SIiE3fooEv
.text._ZN1SIiE3fooEv
wF .text._ZN1SIiE3fooEv 000b _ZN1SIiE3fooEv
<--- Not hidden.
```
(If I comment out the explicit instantiation definition above, foo() is hidden
as expected.)
Okay, it seems that for explicit instantiation definitions,
-fvisibility-inlines-hidden does not apply.
And thinking more about it, that makes sense.
I don't think we should change the linkage like this though, I think we should
just not apply the /Zc:dllexportInlines- for explicit instantiation decls and
definitions in checkClassLevelDLLAttribute(). I realize you had code to check
for this before, but now we have a good and well understood reason.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53666: [Tests] Updated tests for D53342
xbolva00 updated this revision to Diff 171906.
https://reviews.llvm.org/D53666
Files:
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
test/CodeGen/X86/combine-sdiv.ll
test/CodeGen/X86/combine-srem.ll
test/CodeGen/X86/combine-udiv.ll
test/CodeGen/X86/combine-urem.ll
Index: test/CodeGen/X86/combine-urem.ll
===
--- test/CodeGen/X86/combine-urem.ll
+++ test/CodeGen/X86/combine-urem.ll
@@ -89,69 +89,25 @@
ret <4 x i32> %1
}
-; TODO fold (urem 0, x) -> 0
+; fold (urem 0, x) -> 0
define i32 @combine_urem_zero(i32 %x) {
; CHECK-LABEL: combine_urem_zero:
; CHECK: # %bb.0:
; CHECK-NEXT:xorl %eax, %eax
-; CHECK-NEXT:xorl %edx, %edx
-; CHECK-NEXT:divl %edi
-; CHECK-NEXT:movl %edx, %eax
; CHECK-NEXT:retq
%1 = urem i32 0, %x
ret i32 %1
}
define <4 x i32> @combine_vec_urem_zero(<4 x i32> %x) {
; SSE-LABEL: combine_vec_urem_zero:
; SSE: # %bb.0:
-; SSE-NEXT:pextrd $1, %xmm0, %ecx
-; SSE-NEXT:xorl %eax, %eax
-; SSE-NEXT:xorl %edx, %edx
-; SSE-NEXT:divl %ecx
-; SSE-NEXT:movl %edx, %ecx
-; SSE-NEXT:movd %xmm0, %esi
-; SSE-NEXT:xorl %eax, %eax
-; SSE-NEXT:xorl %edx, %edx
-; SSE-NEXT:divl %esi
-; SSE-NEXT:movd %edx, %xmm1
-; SSE-NEXT:pinsrd $1, %ecx, %xmm1
-; SSE-NEXT:pextrd $2, %xmm0, %ecx
-; SSE-NEXT:xorl %eax, %eax
-; SSE-NEXT:xorl %edx, %edx
-; SSE-NEXT:divl %ecx
-; SSE-NEXT:pinsrd $2, %edx, %xmm1
-; SSE-NEXT:pextrd $3, %xmm0, %ecx
-; SSE-NEXT:xorl %eax, %eax
-; SSE-NEXT:xorl %edx, %edx
-; SSE-NEXT:divl %ecx
-; SSE-NEXT:pinsrd $3, %edx, %xmm1
-; SSE-NEXT:movdqa %xmm1, %xmm0
+; SSE-NEXT:xorps %xmm0, %xmm0
; SSE-NEXT:retq
;
; AVX-LABEL: combine_vec_urem_zero:
; AVX: # %bb.0:
-; AVX-NEXT:vpextrd $1, %xmm0, %ecx
-; AVX-NEXT:xorl %eax, %eax
-; AVX-NEXT:xorl %edx, %edx
-; AVX-NEXT:divl %ecx
-; AVX-NEXT:movl %edx, %ecx
-; AVX-NEXT:vmovd %xmm0, %esi
-; AVX-NEXT:xorl %eax, %eax
-; AVX-NEXT:xorl %edx, %edx
-; AVX-NEXT:divl %esi
-; AVX-NEXT:vmovd %edx, %xmm1
-; AVX-NEXT:vpinsrd $1, %ecx, %xmm1, %xmm1
-; AVX-NEXT:vpextrd $2, %xmm0, %ecx
-; AVX-NEXT:xorl %eax, %eax
-; AVX-NEXT:xorl %edx, %edx
-; AVX-NEXT:divl %ecx
-; AVX-NEXT:vpinsrd $2, %edx, %xmm1, %xmm1
-; AVX-NEXT:vpextrd $3, %xmm0, %ecx
-; AVX-NEXT:xorl %eax, %eax
-; AVX-NEXT:xorl %edx, %edx
-; AVX-NEXT:divl %ecx
-; AVX-NEXT:vpinsrd $3, %edx, %xmm1, %xmm0
+; AVX-NEXT:vxorps %xmm0, %xmm0, %xmm0
; AVX-NEXT:retq
%1 = urem <4 x i32> zeroinitializer, %x
ret <4 x i32> %1
Index: test/CodeGen/X86/combine-udiv.ll
===
--- test/CodeGen/X86/combine-udiv.ll
+++ test/CodeGen/X86/combine-udiv.ll
@@ -90,124 +90,30 @@
ret <4 x i32> %1
}
-; TODO fold (udiv 0, x) -> 0
+; fold (udiv 0, x) -> 0
define i32 @combine_udiv_zero(i32 %x) {
; CHECK-LABEL: combine_udiv_zero:
; CHECK: # %bb.0:
; CHECK-NEXT:xorl %eax, %eax
-; CHECK-NEXT:xorl %edx, %edx
-; CHECK-NEXT:divl %edi
; CHECK-NEXT:retq
%1 = udiv i32 0, %x
ret i32 %1
}
define <4 x i32> @combine_vec_udiv_zero(<4 x i32> %x) {
-; SSE2-LABEL: combine_vec_udiv_zero:
-; SSE2: # %bb.0:
-; SSE2-NEXT:pshufd {{.*#+}} xmm1 = xmm0[3,1,2,3]
-; SSE2-NEXT:movd %xmm1, %ecx
-; SSE2-NEXT:xorl %eax, %eax
-; SSE2-NEXT:xorl %edx, %edx
-; SSE2-NEXT:divl %ecx
-; SSE2-NEXT:movd %eax, %xmm1
-; SSE2-NEXT:pshufd {{.*#+}} xmm2 = xmm0[2,3,0,1]
-; SSE2-NEXT:movd %xmm2, %ecx
-; SSE2-NEXT:xorl %eax, %eax
-; SSE2-NEXT:xorl %edx, %edx
-; SSE2-NEXT:divl %ecx
-; SSE2-NEXT:movd %eax, %xmm2
-; SSE2-NEXT:punpckldq {{.*#+}} xmm2 = xmm2[0],xmm1[0],xmm2[1],xmm1[1]
-; SSE2-NEXT:movd %xmm0, %ecx
-; SSE2-NEXT:xorl %eax, %eax
-; SSE2-NEXT:xorl %edx, %edx
-; SSE2-NEXT:divl %ecx
-; SSE2-NEXT:movd %eax, %xmm1
-; SSE2-NEXT:pshufd {{.*#+}} xmm0 = xmm0[1,1,2,3]
-; SSE2-NEXT:movd %xmm0, %ecx
-; SSE2-NEXT:xorl %eax, %eax
-; SSE2-NEXT:xorl %edx, %edx
-; SSE2-NEXT:divl %ecx
-; SSE2-NEXT:movd %eax, %xmm0
-; SSE2-NEXT:punpckldq {{.*#+}} xmm1 = xmm1[0],xmm0[0],xmm1[1],xmm0[1]
-; SSE2-NEXT:punpcklqdq {{.*#+}} xmm1 = xmm1[0],xmm2[0]
-; SSE2-NEXT:movdqa %xmm1, %xmm0
-; SSE2-NEXT:retq
+; SSE-LABEL: combine_vec_udiv_zero:
+; SSE: # %bb.0:
+; SSE-NEXT:xorps %xmm0, %xmm0
+; SSE-NEXT:retq
;
-; SSE41-LABEL: combine_vec_udiv_zero:
-; SSE41: # %bb.0:
-; SSE41-NEXT:pextrd $1, %xmm0, %ecx
-; SSE41-NEXT:xorl %eax, %eax
-; SSE41-NEXT:xorl %edx, %edx
-; SSE41-NEXT:divl %ecx
-; SSE41-NEXT:movl %eax, %ecx
-; SSE41-NEXT:movd %xmm0, %esi
-; SSE41-NEXT:xorl %eax, %eax
-; SSE41-NEXT:xorl %edx, %edx
-; SSE41-NEXT:divl %esi
-; SSE41-NEXT:movd %eax, %xmm1
-; SSE41-NEXT:pinsrd $1, %ecx, %xmm1
-; SSE41-NEXT:pextrd $2, %xmm0, %
[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace
JonasToth added inline comments.
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:176
+
+ Warns when the `std` namespace is used, as its use is against Zircon's libc++
+ policy for the kernel.
alexfh wrote:
> JonasToth wrote:
> > s/its/it's/
> >
> > Could `std` be considered code here? Not sure, but maybe using quotes is
> > better?
> Actually, "its" is correct in this context ("its use" vs "it's used").
whupsi. sry for noise.
https://reviews.llvm.org/D53882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53666: [Tests] Updated tests for D53342
xbolva00 updated this revision to Diff 171908.
https://reviews.llvm.org/D53666
Files:
test/CodeGen/tbaa-struct.cpp
Index: test/CodeGen/tbaa-struct.cpp
===
--- test/CodeGen/tbaa-struct.cpp
+++ test/CodeGen/tbaa-struct.cpp
@@ -17,7 +17,7 @@
void copy(A *a1, A *a2) {
// CHECK-LABEL: _Z4copyP1AS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align
4 %{{.*}}, i64 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}},
i8* nonnull align 4 %{{.*}}, i64 16, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_A:![0-9]*]]
*a1 = *a2;
@@ -31,7 +31,7 @@
void copy2(B *b1, B *b2) {
// CHECK-LABEL: _Z5copy2P1BS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align
4 %{{.*}}, i64 24, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}},
i8* nonnull align 4 %{{.*}}, i64 24, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS2:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_B:![0-9]*]]
*b1 = *b2;
@@ -49,7 +49,7 @@
void copy3(U *u1, U *u2) {
// CHECK-LABEL: _Z5copy3P1US0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align
4 %{{.*}}, i64 12, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}},
i8* nonnull align 4 %{{.*}}, i64 12, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS3:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_U:![0-9]*]]
*u1 = *u2;
@@ -65,7 +65,7 @@
void copy4(C *c1, C *c2) {
// CHECK-LABEL: _Z5copy4P1CS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}}, i8* {{.*}}, i64 3,
i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull {{.*}}, i8* nonnull
{{.*}}, i64 3, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS4:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_C:![0-9]*]]
*c1 = *c2;
@@ -80,23 +80,23 @@
void copy5(D *d1, D *d2) {
// CHECK-LABEL: _Z5copy5P1DS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}}, i8* {{.*}}, i64 6,
i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull {{.*}}, i8* nonnull
{{.*}}, i64 6, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS5:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_D:![0-9]*]]
*d1 = *d2;
}
void copy6(AA *a1, A *a2) {
// CHECK-LABEL: _Z5copy6P1AS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align
4 %{{.*}}, i64 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}},
i8* nonnull align 4 %{{.*}}, i64 16, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS]]
// CHECK-NEW-SAME: !tbaa [[TAG_char:![0-9]*]]
*a1 = *a2;
}
void copy7(A *a1, AA *a2) {
// CHECK-LABEL: _Z5copy7P1AS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align
4 %{{.*}}, i64 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}},
i8* nonnull align 4 %{{.*}}, i64 16, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS]]
// CHECK-NEW-SAME: !tbaa [[TAG_char]]
*a1 = *a2;
@@ -126,4 +126,4 @@
// CHECK-NEW-DAG: [[TYPE_C:!.*]] = !{[[TYPE_char]], i64 3, !"_ZTS1C",
[[TYPE_char]], i64 0, i64 1, [[TYPE_int]], i64 1, i64 4, [[TYPE_char]], i64 1,
i64 1, [[TYPE_char]], i64 2, i64 1}
// CHECK-NEW-DAG: [[TAG_C]] = !{[[TYPE_C]], [[TYPE_C]], i64 0, i64 3}
// CHECK-NEW-DAG: [[TYPE_D:!.*]] = !{[[TYPE_char]], i64 6, !"_ZTS1D",
[[TYPE_char]], i64 0, i64 1, [[TYPE_int]], i64 4, i64 4, [[TYPE_char]], i64 4,
i64 1, [[TYPE_char]], i64 5, i64 1}
-// CHECK-NEW-DAG: [[TAG_D]] = !{[[TYPE_D]], [[TYPE_D]], i64 0, i64 6}
+// CHECK-NEW-DAG: [[TAG_D]] = !{[[TYPE_D]], [[TYPE_D]], i64 0, i64 6}
\ No newline at end of file
Index: test/CodeGen/tbaa-struct.cpp
===
--- test/CodeGen/tbaa-struct.cpp
+++ test/CodeGen/tbaa-struct.cpp
@@ -17,7 +17,7 @@
void copy(A *a1, A *a2) {
// CHECK-LABEL: _Z4copyP1AS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}}, i8* nonnull align 4 %{{.*}}, i64 16, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_A:![0-9]*]]
*a1 = *a2;
@@ -31,7 +31,7 @@
void copy2(B *b1, B *b2) {
// CHECK-LABEL: _Z5copy2P1BS0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 24, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}}, i8* nonnull align 4 %{{.*}}, i64 24, i1 false)
// CHECK-OLD-SAME: !tbaa.struct [[TS2:!.*]]
// CHECK-NEW-SAME: !tbaa [[TAG_B:![0-9]*]]
*b1 = *b2;
@@ -49,7 +49,7 @@
void copy3(U *u1, U *u2) {
// CHECK-LABEL: _Z5copy3P1US0_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 12, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 %{{.*}}, i8* n
[PATCH] D53025: [clang-tidy] implement new check for const return types.
JonasToth added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getReturnType(); ymandel wrote: > JonasToth wrote: > > ymandel wrote: > > > aaron.ballman wrote: > > > > ymandel wrote: > > > > > aaron.ballman wrote: > > > > > > ymandel wrote: > > > > > > > aaron.ballman wrote: > > > > > > > > ymandel wrote: > > > > > > > > > aaron.ballman wrote: > > > > > > > > > > ymandel wrote: > > > > > > > > > > > aaron.ballman wrote: > > > > > > > > > > > > It seems strange to me that this is in the readability > > > > > > > > > > > > module but the diagnostic here is about compiler > > > > > > > > > > > > optimizations. Should this be in the performance module > > > > > > > > > > > > instead? Or should this diagnostic be reworded about > > > > > > > > > > > > the readability concerns? > > > > > > > > > > > Good point. The readability angle is that the const > > > > > > > > > > > clutters the code to no benefit. That is, even if it was > > > > > > > > > > > performance neutral, I'd still want to discourage this > > > > > > > > > > > practice. But, it does seem like the primary impact is > > > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > > > > > I'm fine either way -- moving it to performance or > > > > > > > > > > > emphasizing the clutter of the unhelpful "const". I'm > > > > > > > > > > > inclined to moving it, but don't have good sense of how > > > > > > > > > > > strict these hierarchies are. What do you think? > > > > > > > > > > I'm not sold that `const`-qualified return types always > > > > > > > > > > pessimize optimizations. However, I'm also not sold that > > > > > > > > > > it's *always* a bad idea to have a top-level cv-qualifier > > > > > > > > > > on a return type either (for instance, this is one way to > > > > > > > > > > prevent callers from modifying a temp returned by a > > > > > > > > > > function). How about leaving this in readability and > > > > > > > > > > changing the diagnostic into: "top-level 'const' qualifier > > > > > > > > > > on a return type may reduce code readability with limited > > > > > > > > > > benefit" or something equally weasely? > > > > > > > > > I talked this over with other google folks and seems like the > > > > > > > > > consensus is: > > > > > > > > > > > > > > > > > > 1. The readability benefits may be controversial. Some > > > > > > > > > folks might not view `const` as clutter and there are some > > > > > > > > > corner cases where the qualifier may prevent something > > > > > > > > > harmful. > > > > > > > > > 2. The performance implication is real, if not guaranteed to > > > > > > > > > be a problem. > > > > > > > > > > > > > > > > > > Based on this, seems best to move it to performance, but > > > > > > > > > water down the performance claims. That keeps the focus to > > > > > > > > > an issue we can assume nearly everyone agrees on. > > > > > > > > I'm not convinced the performance implications are real > > > > > > > > compared to how the check is currently implemented. I know > > > > > > > > there are performance concerns when you return a const value of > > > > > > > > class type because it pessimizes the ability to use move > > > > > > > > constructors, but that's a very specific performance concern > > > > > > > > that I don't believe is present in general. For instance, I > > > > > > > > don't know of any performance concerns regarding `const int > > > > > > > > f();` as a declaration. I'd be fine moving this to the > > > > > > > > performance module, but I think the check would need to be > > > > > > > > tightened to only focus on actual performance concerns. > > > > > > > > > > > > > > > > FWIW, I do agree that the readability benefits may be > > > > > > > > controversial, but I kind of thought that was the point to the > > > > > > > > check and as such, it's a very reasonable check. Almost > > > > > > > > everything in readability is subjective to some degree and our > > > > > > > > metric has always been "if you agree with a style check, don't > > > > > > > > enable it". > > > > > > > > > > > > > > > > It's up to you how you want to proceed, but I see two ways > > > > > > > > forward: 1) keep this as a readability check that broadly finds > > > > > > > > top-level const qualifiers and removes them, 2) move this to > > > > > > > > the performance module and rework the check to focus on only > > > > > > > > the scenarios that have concrete performance impact. > > > > > > > > It's up to you how you want to proceed, but I see two ways > > > > > > > > forward: 1) keep this as a readability check that broadly finds > > > > > > > > top-level const qualifiers and removes them, 2) move this to > > > > > > > > the performance module and rework the chec
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
JonasToth marked an inline comment as done.
JonasToth added inline comments.
Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119
+ const LangOptions &LangOpts) {
+ std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
+ if (DeclCount < 2)
ZaMaZaN4iK wrote:
> Mark `DeclCount` as const
as `DeclCount` is a value, LLVM style does not mark it as const, only pointers
or references.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
martong added a comment. Hi Davide, I'd like to draw your attention to an important relation of this patch to a bug in LLDB which manifests as incorrect vtable layouts for LLDB expression code. Please see the conversation started with this message: https://lists.llvm.org/pipermail/cfe-dev/2018-August/058842.html I think that the solution to that problem would be exactly this patch, i.e. to put the methods in the proper order during import. Are the failing lldb tests related to ObjC code? Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
xazax.hun accepted this revision. xazax.hun added a comment. LGTM! Thanks, I think it is much easier to understand what is going on this way. https://reviews.llvm.org/D52742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
a.sidorin added a comment. Hello everyone. @martong : this version of patch reorders only fields and friends. No method reordering is done (I can check if it works, though, and add the feature). @davide: Unfortunately, I was not able to reproduce the problem on Linux. Could you please provide a link to a buildbot failure or some environment description so I can reproduce the issue or, at least, take a look? Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
xazax.hun added a comment.
Please add a test case where a bug path goes through a macro definition and
this macro is undefed at the end of the translation unit.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:667
+
+//===--===//
+// Declarations of helper functions and data structures for expanding macros.
Maybe it would be worth to move these helpers to a separate file and add some
documentation why we actually simulate the preprocessor rather than doing
something else.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:724
+ TokenPrinter Printer(OS, PP);
+ return { getExpandedMacroImpl(Printer, MacroLoc, PP), OS.str() };
+}
Maybe could directly return ExpansionInfo?
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:734
+ MacroNameAndInfo Info = getMacroNameAndInfo(SM.getExpansionLoc(MacroLoc),
PP);
+ std::string MacroName = std::move(Info.Name);
+ const MacroInfo *MI = Info.MI;
I think this local might be redundant.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:753
+// macro.
+if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getExpandedMacroImpl(Printer, T.getLocation(), PP);
This might not be completely correct. We are using the preprocessor state after
processing the whole translation unit rather than the preprocessor state at the
expansion location. This might not cause any problems unless there is a
collision between macro names and non-macro names. Also, undefs, redefs might
cause troubles.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:796
+ const MacroInfo *MI = PP.getMacroInfo(II);
+ assert(MI && "This IdentifierInfo should refer to a macro!");
+
Could this assertion fail due to undefs in the code?
https://reviews.llvm.org/D52794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53025: [clang-tidy] implement new check for const return types.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM, thank you for working on this check! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53933
Files:
clangd/FindSymbols.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
unittests/clangd/DexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SyncAPI.cpp
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -129,6 +129,7 @@
SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query) {
FuzzyFindRequest Req;
Req.Query = Query;
+ Req.AnyScope = true;
return runFuzzyFind(Index, Req);
}
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -90,14 +90,16 @@
symbol("2") /* duplicate */};
FuzzyFindRequest Req;
Req.Query = "2";
+ Req.AnyScope = true;
MemIndex I(Symbols, RefSlab());
EXPECT_THAT(match(I, Req), ElementsAre("2"));
}
TEST(MemIndexTest, MemIndexLimitedNumMatches) {
auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
FuzzyFindRequest Req;
Req.Query = "5";
+ Req.AnyScope = true;
Req.Limit = 3;
bool Incomplete;
auto Matches = match(*I, Req, &Incomplete);
@@ -112,6 +114,7 @@
RefSlab());
FuzzyFindRequest Req;
Req.Query = "lol";
+ Req.AnyScope = true;
Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
@@ -122,6 +125,7 @@
MemIndex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab());
FuzzyFindRequest Req;
Req.Query = "y";
+ Req.AnyScope = true;
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
}
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -485,6 +485,7 @@
UnorderedElementsAre("other::A", "other::ABC"));
Req.Query = "";
Req.Scopes = {};
+ Req.AnyScope = true;
EXPECT_THAT(match(*Index, Req),
UnorderedElementsAre("ns::ABC", "ns::BCD", "::ABC",
"ns::nested::ABC", "other::ABC",
@@ -496,14 +497,16 @@
symbol("2") /* duplicate */};
FuzzyFindRequest Req;
Req.Query = "2";
+ Req.AnyScope = true;
Dex I(Symbols, RefSlab(), URISchemes);
EXPECT_THAT(match(I, Req), ElementsAre("2"));
}
TEST(DexTest, DexLimitedNumMatches) {
auto I = Dex::build(generateNumSymbols(0, 100), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "5";
+ Req.AnyScope = true;
Req.Limit = 3;
bool Incomplete;
auto Matches = match(*I, Req, &Incomplete);
@@ -518,6 +521,7 @@
RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "lol";
+ Req.AnyScope = true;
Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
@@ -527,6 +531,7 @@
auto I =
Dex::build(generateSymbols({"OneTwoThreeFour"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
+ Req.AnyScope = true;
bool Incomplete;
EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("OneTwoThreeFour"));
@@ -549,6 +554,7 @@
auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
URISchemes);
FuzzyFindRequest Req;
+ Req.AnyScope = true;
Req.Query = "y";
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
}
@@ -593,9 +599,9 @@
auto I =
Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
+ Req.AnyScope = true;
Req.Query = "y";
Req.Scopes = {"a::"};
- Req.AnyScope = true;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
}
@@ -635,6 +641,7 @@
std::vector Symbols{CodeCompletionSymbol, NonCodeCompletionSymbol};
Dex I(Symbols, RefSlab(), URISchemes);
FuzzyFindRequest Req;
+ Req.AnyScope = true;
Req.RestrictForCodeCompletion = false;
EXPECT_THAT(match(I, Req), ElementsAre("Completion", "NoCompletion"));
Req.RestrictForCodeCompletion = true;
@@ -651,6 +658,7 @@
Dex I(Symbols, RefSlab(), URISchemes);
FuzzyFindRequest Req;
+ Req.AnyScope = true;
Req.Query = "abc";
// The best candidate can change depending on the proximity paths.
Req.Limit = 1;
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -178,7 +178,7 @@
std::vector> ScopeIterators;
for (const auto &Scope : Req.Scopes)
ScopeIterators.push_back(iterator(Token(Token::Kind::Sc
[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: docs/clang-tidy/checks/readability-isolate-declaration.rst:45
+
+Global variables and member variables are excluded.
+
JonasToth wrote:
> aaron.ballman wrote:
> > Why are global variables excluded from this check? It seems like they
> > should have the exact same behavior as local variables in terms of the
> > transformation, no?
> I thought so too, but take a look at the AST:
>
> ```
> int global_i, global_j = 42, global_k;
> void f() {
> int local_i, local_j = 42, local_k;
> }
> ```
>
> ```
> |-VarDecl 0x55c428c4ab08
> col:5 global_i
> 'int'
> |-VarDecl 0x55c428c4abc0 col:15 global_j 'int' cinit
> | `-IntegerLiteral 0x55c428c4ac20 'int' 42
> |-VarDecl 0x55c428c4ac58 col:30 global_k 'int'
> `-FunctionDecl 0x55c428c4ad30 line:2:6 f 'void ()'
> `-CompoundStmt 0x55c428c4af90
> `-DeclStmt 0x55c428c4af78
> |-VarDecl 0x55c428c4ade8 col:9 local_i 'int'
> |-VarDecl 0x55c428c4ae60 col:18 local_j 'int' cinit
> | `-IntegerLiteral 0x55c428c4aec0 'int' 42
> `-VarDecl 0x55c428c4aef8 col:32 local_k 'int'
> ```
>
> The locals create a `DeclStmt` and the globals don't, right now the
> transformation depends on the fact that its a `DeclStmt`, similar to class
> members that are `FieldDecl`.
>
> I did short investigation on the issue, but couldn't think of a good way to
> fix that issue. I was not capable of "grouping" these decls even though they
> are together. Maybe I just missed something obvious, but right it's not
> supported. I would love to actually support it tough.
Yeah, this looks like a deficiency with the AST representation, to me. It
doesn't need to be solved for this patch, but it may be an interesting next
step for the check for a future patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171916.
takuto.ikuta added a comment.
export/import explicit template instantiation function
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValu
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked an inline comment as done.
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Can you give an example for why this is needed?
> > > > > Sorry, this change does not need. Removed.
> > > > Sorry, this change is necessary still.
> > > >
> > > > Without this, definition of inline function in explicit template
> > > > instantiation declaration is not be emitted, due to
> > > > GVA_AvailableExternally linkage.
> > > > But we stop exporting definition of inline function in explicit
> > > > template instantiation definition too.
> > > >
> > > > So without this, definition of dllimported inline function of explicit
> > > > template instantiation declaration won't be available.
> > > >
> > > Can you provide a code example of why this is needed?
> > If we don't change function linkage, following code will be compiled like
> > below with -fno-dllexport-inlines.
> >
> > ```
> > template
> > class M{
> > public:
> > void foo() {}
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > void f (){
> > M mi;
> > mi.foo();
> >
> > M ms;
> > ms.foo();
> > }
> > ```
> >
> > ```
> > $"?foo@?$M@H@@QEAAXXZ" = comdat any
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> > comdat align 2 {
> > entry:
> > %this.addr = alloca %class.M*, align 8
> > store %class.M* %this, %class.M** %this.addr, align 8
> > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > ret void
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local void @"?f@@YAXXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> > call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> > ret void
> > }
> >
> > declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> > ```
> >
> > M::foo() is declared, but inline function is not dllexported (See
> > M::foo() is not dllexported). So this code cannot be linked because
> > definition of M::foo does not exist. If the function is properly
> > inlined, we don't need to have this code. But I'm not sure why the function
> > is not inlined.
> Interesting. I wonder how -fvisibility-inlines-hidden handles this...
>
>
> ```
> template struct S {
> void foo() {}
> };
>
> template struct S;
>
> void use() {
> S s;
> s.foo();
> }
>
> $ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
> _ZN1SIiE3fooEv
> ld .text._ZN1SIiE3fooEv
> .text._ZN1SIiE3fooEv
> wF .text._ZN1SIiE3fooEv 000b _ZN1SIiE3fooEv
> <--- Not hidden.
> ```
>
> (If I comment out the explicit instantiation definition above, foo() is
> hidden as expected.)
>
> Okay, it seems that for explicit instantiation definitions,
> -fvisibility-inlines-hidden does not apply.
>
> And thinking more about it, that makes sense.
>
> I don't think we should change the linkage like this though, I think we
> should just not apply the /Zc:dllexportInlines- for explicit instantiation
> decls and definitions in checkClassLevelDLLAttribute(). I realize you had
> code to check for this before, but now we have a good and well understood
> reason.
Thank you for confirmation. I revived the check.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: r345676 - [Win64] Handle passing i128 by value
Also, don't we usually put ABI changes under an ABI compatibility check? This
would be making Clang incompatible with itself.
--paulr
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
Richard Trieu via cfe-commits
Sent: Tuesday, October 30, 2018 10:16 PM
To: Reid Kleckner
Cc: cfe-commits
Subject: Re: r345676 - [Win64] Handle passing i128 by value
I have reverted this in r345691 because it caused test
CodeGen/mingw-long-double.c to start failing.
Command Output (stderr):
--
/usr/local/google/clang/install/llvm/tools/clang/test/CodeGen/mingw-long-double.c:36:11:
error: MSC64: expected string not found in input
// MSC64: define dso_local double @TestLD(double %x)
^
:12:1: note: scanning from here
; Function Attrs: noinline nounwind optnone
^
:35:1: note: possible intended match here
define dso_local void @TestLDC({ double, double }* noalias sret %agg.result, {
double, double }* %x) #2 {
^
--
I suspect your patch has changed the type of "double" to a different floating
point type, causing the failure.
On Tue, Oct 30, 2018 at 5:00 PM Reid Kleckner via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: rnk
Date: Tue Oct 30 16:58:41 2018
New Revision: 345676
URL: http://llvm.org/viewvc/llvm-project?rev=345676&view=rev
Log:
[Win64] Handle passing i128 by value
For arguments, pass it indirectly, since the ABI doc says pretty clearly
that arguments larger than 8 bytes are passed indirectly. This makes
va_list handling easier, anyway.
When returning, GCC returns in XMM0, and we match them.
Fixes PR39492.
Added:
cfe/trunk/test/CodeGen/win64-i128.c
Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=345676&r1=345675&r2=345676&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Oct 30 16:58:41 2018
@@ -3944,18 +3944,39 @@ ABIArgInfo WinX86_64ABIInfo::classify(Qu
return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(),
Width));
}
- // Bool type is always extended to the ABI, other builtin types are not
- // extended.
- const BuiltinType *BT = Ty->getAs();
- if (BT && BT->getKind() == BuiltinType::Bool)
-return ABIArgInfo::getExtend(Ty);
-
- // Mingw64 GCC uses the old 80 bit extended precision floating point unit. It
- // passes them indirectly through memory.
- if (IsMingw64 && BT && BT->getKind() == BuiltinType::LongDouble) {
-const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
-if (LDF == &llvm::APFloat::x87DoubleExtended())
- return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+ if (const BuiltinType *BT = Ty->getAs()) {
+switch (BT->getKind()) {
+case BuiltinType::Bool:
+ // Bool type is always extended to the ABI, other builtin types are not
+ // extended.
+ return ABIArgInfo::getExtend(Ty);
+
+case BuiltinType::LongDouble:
+ // Mingw64 GCC uses the old 80 bit extended precision floating point
+ // unit. It passes them indirectly through memory.
+ if (IsMingw64) {
+const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
+if (LDF == &llvm::APFloat::x87DoubleExtended())
+ return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+break;
+ }
+
+case BuiltinType::Int128:
+case BuiltinType::UInt128:
+ // If it's a parameter type, the normal ABI rule is that arguments larger
+ // than 8 bytes are passed indirectly. GCC follows it. We follow it too,
+ // even though it isn't particularly efficient.
+ if (!IsReturnType)
+return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+
+ // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
+ // Clang matches them for compatibility.
+ return ABIArgInfo::getDirect(
+ llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+
+default:
+ break;
+}
}
return ABIArgInfo::getDirect();
Added: cfe/trunk/test/CodeGen/win64-i128.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/win64-i128.c?rev=345676&view=auto
==
--- cfe/trunk/test/CodeGen/win64-i128.c (added)
+++ cfe/trunk/test/CodeGen/win64-i128.c Tue Oct 30 16:58:41 2018
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s \
+// RUN:| FileCheck %s --check-prefix=GNU64
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s \
+// RUN:| FileCheck %s --check-prefix=MSC64
+
+typedef int int128_t __attribute__((mode(TI)));
+
+int128_t foo() { return 0; }
+
+// GNU64: define dso_local <2 x i64> @foo()
+// MSC64: define dso_local <2 x i64> @foo()
+
+int128_t bar(int128_t a, int128_t b) { return a * b; }
+
+// GNU64: de
[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace
aaron.ballman added a comment.
I am a bit confused by what this check is trying to accomplish. It seems like
this is intended to catch use of anything declared within the standard library,
but that includes library support things that are needed to write useful code.
For instance, it seems this means users cannot use initializer lists or type
traits. I'm not even certain users can overload `operator new()` because that
uses `std::size_t` as an argument.
Can you expound on the requirements in a bit more detail? Is it truly "no
standard library functionality at all, including things required to be
supported in freestanding implementations"?
Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:57-60
+ Finder->addMatcher(callExpr(callee(functionDecl(hasDeclContext(
+ namespaceDecl(isStdNamespace())
+ .bind("stdCall"),
+ this);
Is the intention here to diagnose code like this?
```
#include
int main() {
abort(); // Diagnose this?
}
```
https://reviews.llvm.org/D53882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ilya-biryukov added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
Anonymous namespace inside other namespaces will also produce duplicate scopes.
Maybe simply remove the duplicates from the vector before we return it?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ioeric added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
ilya-biryukov wrote:
> Anonymous namespace inside other namespaces will also produce duplicate
> scopes.
> Maybe simply remove the duplicates from the vector before we return it?
`printNamespaceScope()` will return "" for all anonymous namespaces, which
should be covered as well.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
Szelethus marked 2 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:471
+DisplayMacroExpansions =
+getBooleanOption("expand-macros", /*Default=*/false);
+ return DisplayMacroExpansions.getValue();
NoQ wrote:
> Yup, np, let's wait until it lands then.
I'll commit as is, because D53277 is blocked by D53276, and I'd very much like
to get this project out of the way as soon as possible.
https://reviews.llvm.org/D52742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53866: [Preamble] Fix preamble for circular #includes
ilya-biryukov added a comment. > Because this case can be detected and handled without loosing the benefits of > the preamble. The cases where recursive includes make sense are incredibly rare in practice. Most of the time, recursively including the same file is unintentional should be considered a bug and fixed. Producing a warning that the file is being recursively included (along with an include chain that caused a recursive inclusion) seems to provide more benefit than preamble, especially when this is unintentional (I would argue that's the majority of the cases). > If we are in preamble-generation-mode and the main file #includes itself, > then it should see the full file content of the file, not the truncated > preamble version of it. > Would this be more consistent? Yes, but I can't see an easy way to make it happen. And I don't think we want a complex fix for something bizzare that very rarely happens in practice. Repository: rC Clang https://reviews.llvm.org/D53866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div
xbolva00 updated this revision to Diff 171920.
xbolva00 added a reviewer: MTC.
xbolva00 added a comment.
- New warning text, added another test line
https://reviews.llvm.org/D52949
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/div-sizeof-ptr.c
Index: test/Sema/div-sizeof-ptr.c
===
--- test/Sema/div-sizeof-ptr.c
+++ test/Sema/div-sizeof-ptr.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+void test(int *p, int **q) {
+int a1 = sizeof(p) / sizeof(*p); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+int a2 = sizeof p / sizeof *p;// expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}}
+int a3 = sizeof(*q) / sizeof(**q);// expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}}
+int a4 = sizeof(p) / sizeof(int); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+int a5 = sizeof(p) / sizeof(p[0]);// expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+
+int b1 = sizeof(int *) / sizeof(int);
+int b2 = sizeof(p) / sizeof(p);
+int b3 = sizeof(*q) / sizeof(q);
+int b4 = sizeof(p) / sizeof(char);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8726,6 +8726,37 @@
<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
}
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+ SourceLocation Loc) {
+ UnaryExprOrTypeTraitExpr *LUE =
+ dyn_cast_or_null(LHS);
+ UnaryExprOrTypeTraitExpr *RUE =
+ dyn_cast_or_null(RHS);
+
+ if (!LUE || !RUE)
+return;
+ if (LUE->getKind() != UETT_SizeOf || LUE->isArgumentType() ||
+ RUE->getKind() != UETT_SizeOf)
+return;
+
+ QualType LHSTy;
+ QualType RHSTy;
+ LHSTy = LUE->getArgumentExpr()->IgnoreParens()->getType();
+
+ if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+ } else {
+RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
+ }
+
+ if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+ if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+ S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
+}
+
static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8756,8 +8787,10 @@
if (compType.isNull() || !compType->isArithmeticType())
return InvalidOperands(Loc, LHS, RHS);
- if (IsDiv)
+ if (IsDiv) {
DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+ }
return compType;
}
@@ -16599,4 +16632,4 @@
return new (Context)
ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
-}
+}
\ No newline at end of file
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3294,6 +3294,10 @@
InGroup;
def note_reference_is_return_value : Note<"%0 returns a reference">;
+def warn_division_sizeof_ptr : Warning<
+ "'%0' will return the size of the pointer, not the array itself">,
+ InGroup, DefaultIgnore;
+
def note_function_warning_silence : Note<
"prefix with the address-of operator to silence this warning">;
def note_function_to_function_call : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -144,6 +144,7 @@
def : DiagGroup<"discard-qual">;
def DivZero : DiagGroup<"division-by-zero">;
def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
def DocumentationHTML : DiagGroup<"documentation-html">;
def DocumentationUnknownCommand : DiagGroup<"documentation-unknown-command">;
@@ -789,6 +790,7 @@
SelfMove,
SizeofArrayArgument,
SizeofArrayDecay,
+DivSizeofPtr,
StringPlusInt,
Trigraphs,
Uninitialized,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r345724 - [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
Author: szelethus
Date: Wed Oct 31 07:54:27 2018
New Revision: 345724
URL: http://llvm.org/viewvc/llvm-project?rev=345724&view=rev
Log:
[analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
This is the first part of the implementation of the inclusion of macro
expansions into the plist output. It adds a new flag that adds a new
"macro_expansions" entry to each report that has PathDiagnosticPieces that were
expanded from a macro. While there's an entry for each macro expansion, both
the name of the macro and what it expands to is missing, and will be implemented
in followup patches.
Differential Revision: https://reviews.llvm.org/D52742
Added:
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=345724&r1=345723&r2=345724&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Wed Oct 31
07:54:27 2018
@@ -307,6 +307,9 @@ private:
/// \sa shouldDisplayNotesAsEvents
Optional DisplayNotesAsEvents;
+ /// \sa shouldDisplayMacroExpansions
+ Optional DisplayMacroExpansions;
+
/// \sa shouldAggressivelySimplifyBinaryOperation
Optional AggressiveBinaryOperationSimplification;
@@ -693,6 +696,13 @@ public:
/// to false when unset.
bool shouldDisplayNotesAsEvents();
+ /// Returns true if macros related to the bugpath should be expanded and
+ /// included in the plist output.
+ ///
+ /// This is controlled by the 'expand-macros' option, which defaults to false
+ /// when unset.
+ bool shouldDisplayMacroExpansions();
+
/// Returns true if SValBuilder should rearrange comparisons and additive
/// operations of symbolic expressions which consist of a sum of a symbol and
/// a concrete integer into the format where symbols are on the left-hand
Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=345724&r1=345723&r2=345724&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Wed Oct 31 07:54:27
2018
@@ -464,6 +464,13 @@ bool AnalyzerOptions::shouldDisplayNotes
return DisplayNotesAsEvents.getValue();
}
+bool AnalyzerOptions::shouldDisplayMacroExpansions() {
+ if (!DisplayMacroExpansions.hasValue())
+DisplayMacroExpansions =
+getBooleanOption("expand-macros", /*Default=*/false);
+ return DisplayMacroExpansions.getValue();
+}
+
bool AnalyzerOptions::shouldAggressivelySimplifyBinaryOperation() {
if (!AggressiveBinaryOperationSimplification.hasValue())
AggressiveBinaryOperationSimplification =
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=345724&r1=345723&r2=345724&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Wed Oct 31 07:54:27 2018
@@ -546,7 +546,8 @@ static void updateStackPiecesWithMessage
}
}
-static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM);
+static void CompactMacroExpandedPieces(PathPieces &path,
+ const SourceManager& SM);
std::shared_ptr generateDiagForSwitchOP(
@@ -1972,8 +1973,6 @@ static std::unique_ptr g
PathDiagnosticLocation::createBegin(D, SM));
}
- if (!AddPathEdges && GenerateDiagnostics)
-CompactPathDiagnostic(PD->getMutablePieces(), SM);
// Finally, prune the diagnostic path of uninteresting stuff.
if (!PD->path.empty()) {
@@ -2007,6 +2006,10 @@ static std::unique_ptr g
removeRedundantMsgs(PD->getMutablePieces());
removeEdgesToDefaultInitializers(PD->getMutablePieces());
}
+
+ if (GenerateDiagnostics && Opts.shouldDisplayMacroExpansions())
+CompactMacroExpandedPieces(PD->getMutablePieces(), SM);
+
return PD;
}
@@ -2436,9 +2439,10 @@ bool TrimmedGraph::popNextReportGraph(Re
return true;
}
-/// CompactPathDiagnostic - This function postprocesses a PathDiagnostic object
-/// and collapses PathDiagosticPieces that are expanded by macros.
-static void CompactPa
[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345724: [analyzer][PlistMacroExpansion] Part 1.: New
expand-macros flag (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52742?vs=171533&id=171921#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52742
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -72,6 +72,7 @@
const FIDMap& FM;
AnalyzerOptions &AnOpts;
const Preprocessor &PP;
+ llvm::SmallVector MacroPieces;
public:
PlistPrinter(const FIDMap& FM, AnalyzerOptions &AnOpts,
@@ -86,6 +87,14 @@
(void)AnOpts;
}
+ /// Print the expansions of the collected macro pieces.
+ ///
+ /// Each time ReportDiag is called on a PathDiagnosticMacroPiece (or, if one
+ /// is found through a call piece, etc), it's subpieces are reported, and the
+ /// piece itself is collected. Call this function after the entire bugpath
+ /// was reported.
+ void ReportMacroExpansions(raw_ostream &o, unsigned indent);
+
private:
void ReportPiece(raw_ostream &o, const PathDiagnosticPiece &P,
unsigned indent, unsigned depth, bool includeControlFlow,
@@ -104,7 +113,8 @@
isKeyEvent);
break;
case PathDiagnosticPiece::Macro:
-ReportMacro(o, cast(P), indent, depth);
+ReportMacroSubPieces(o, cast(P), indent,
+ depth);
break;
case PathDiagnosticPiece::Note:
ReportNote(o, cast(P), indent);
@@ -123,14 +133,25 @@
unsigned indent, unsigned depth, bool isKeyEvent = false);
void ReportCall(raw_ostream &o, const PathDiagnosticCallPiece &P,
unsigned indent, unsigned depth);
- void ReportMacro(raw_ostream &o, const PathDiagnosticMacroPiece& P,
- unsigned indent, unsigned depth);
+ void ReportMacroSubPieces(raw_ostream &o, const PathDiagnosticMacroPiece& P,
+unsigned indent, unsigned depth);
void ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
unsigned indent);
};
} // end of anonymous namespace
+namespace {
+
+struct ExpansionInfo {
+ std::string MacroName;
+ std::string Expansion;
+ ExpansionInfo(std::string N, std::string E)
+: MacroName(std::move(N)), Expansion(std::move(E)) {}
+};
+
+} // end of anonymous namespace
+
static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM,
AnalyzerOptions &AnOpts,
const Preprocessor &PP,
@@ -143,6 +164,10 @@
SmallVectorImpl &Fids,
FIDMap &FM,
llvm::raw_fd_ostream &o);
+
+static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc,
+ const Preprocessor &PP);
+
//===--===//
// Methods of PlistPrinter.
//===--===//
@@ -299,16 +324,52 @@
ReportPiece(o, *callExit, indent, depth, /*includeControlFlow*/ true);
}
-void PlistPrinter::ReportMacro(raw_ostream &o,
- const PathDiagnosticMacroPiece& P,
- unsigned indent, unsigned depth) {
-
- for (PathPieces::const_iterator I = P.subPieces.begin(), E=P.subPieces.end();
- I!=E; ++I) {
+void PlistPrinter::ReportMacroSubPieces(raw_ostream &o,
+const PathDiagnosticMacroPiece& P,
+unsigned indent, unsigned depth) {
+ MacroPieces.push_back(&P);
+
+ for (PathPieces::const_iterator I = P.subPieces.begin(),
+ E = P.subPieces.end();
+ I != E; ++I) {
ReportPiece(o, **I, indent, depth, /*includeControlFlow*/ false);
}
}
+void PlistPrinter::ReportMacroExpansions(raw_ostream &o, unsigned indent) {
+
+ for (const PathDiagnosticMacroPiece *P : MacroPieces) {
+const SourceManager &SM = PP.getSourceManager();
+ExpansionInfo EI = getExpandedMacro(P->getLocation().asLocation(), PP);
+
+Indent(o, indent) << "\n";
+++indent;
+
+// Output the location.
+FullSourceLoc L = P->getLocation().asLocation();
+
+Indent(o, indent) << "location\n";
+EmitLocation(o,
[PATCH] D53934: [clangd] Improve code completion for ObjC methods
dgoldman created this revision.
dgoldman added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, jkorous,
MaskRay, ioeric, ilya-biryukov.
Previously code completion did not work well for Objective-C methods
which contained multiple arguments as clangd did not expect to see
multiple typed-text chunks when handling code completion.
Note that even with this change, we do not consider selector fragments
from previous arguments to be part of the signature (although we
could in the future).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53934
Files:
clangd/CodeCompletionStrings.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/CodeCompletionStringsTests.cpp
Index: unittests/clangd/CodeCompletionStringsTests.cpp
===
--- unittests/clangd/CodeCompletionStringsTests.cpp
+++ unittests/clangd/CodeCompletionStringsTests.cpp
@@ -109,6 +109,53 @@
EXPECT_EQ(Snippet, "");
}
+TEST_F(CompletionStringTest, ObjectiveCMethodNoArguments) {
+ Builder.AddResultTypeChunk("void");
+ Builder.AddTypedTextChunk("methodName");
+
+ auto *CCS = Builder.TakeString();
+ computeSignature(*CCS);
+ EXPECT_EQ(Signature, "");
+ EXPECT_EQ(Snippet, "");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodOneArgument) {
+ Builder.AddResultTypeChunk("void");
+ Builder.AddTypedTextChunk("methodWithArg:");
+ Builder.AddPlaceholderChunk("(type)");
+
+ auto *CCS = Builder.TakeString();
+ computeSignature(*CCS);
+ EXPECT_EQ(Signature, "(type)");
+ EXPECT_EQ(Snippet, "${1:(type)}");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodTwoArgumentsFromBeginning) {
+ Builder.AddResultTypeChunk("int");
+ Builder.AddTypedTextChunk("withFoo:");
+ Builder.AddPlaceholderChunk("(type)");
+ Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+ Builder.AddTypedTextChunk("bar:");
+ Builder.AddPlaceholderChunk("(type2)");
+
+ auto *CCS = Builder.TakeString();
+ computeSignature(*CCS);
+ EXPECT_EQ(Signature, "(type) bar:(type2)");
+ EXPECT_EQ(Snippet, "${1:(type)} bar:${2:(type2)}");
+}
+
+TEST_F(CompletionStringTest, ObjectiveCMethodTwoArgumentsFromMiddle) {
+ Builder.AddResultTypeChunk("int");
+ Builder.AddInformativeChunk("withFoo:");
+ Builder.AddTypedTextChunk("bar:");
+ Builder.AddPlaceholderChunk("(type2)");
+
+ auto *CCS = Builder.TakeString();
+ computeSignature(*CCS);
+ EXPECT_EQ(Signature, "(type2)");
+ EXPECT_EQ(Snippet, "${1:(type2)}");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -105,15 +105,16 @@
CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
std::vector IndexSymbols = {},
- clangd::CodeCompleteOptions Opts = {}) {
+ clangd::CodeCompleteOptions Opts = {},
+ PathRef FilePath = "foo.cpp") {
std::unique_ptr OverrideIndex;
if (!IndexSymbols.empty()) {
assert(!Opts.Index && "both Index and IndexSymbols given!");
OverrideIndex = memIndex(std::move(IndexSymbols));
Opts.Index = OverrideIndex.get();
}
- auto File = testPath("foo.cpp");
+ auto File = testPath(FilePath);
Annotations Test(Text);
runAddDocument(Server, File, Test.code());
auto CompletionList =
@@ -125,12 +126,14 @@
// If IndexSymbols is non-empty, an index will be built and passed to opts.
CodeCompleteResult completions(StringRef Text,
std::vector IndexSymbols = {},
- clangd::CodeCompleteOptions Opts = {}) {
+ clangd::CodeCompleteOptions Opts = {},
+ PathRef FilePath = "foo.cpp") {
MockFSProvider FS;
MockCompilationDatabase CDB;
IgnoreDiagnostics DiagConsumer;
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
- return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
+ return completions(Server, Text, std::move(IndexSymbols), std::move(Opts),
+ FilePath);
}
std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
@@ -2187,6 +2190,71 @@
{cls("naber"), cls("nx::naber")}, Opts);
EXPECT_THAT(Results.Completions, UnorderedElementsAre());
}
+
+TEST(CompletionTest, ObjectiveCMethodNoArguments) {
+ std::string Context = R"objc(
+@interface Foo
+@property(nonatomic) int value;
+@end
+Foo *foo = [Foo new]; int y = [foo v^]
+ )objc";
+
+ auto A = completions(Context, {}, {}, "Foo.m").Completions.front();
+ EXPECT_EQ(A.Name, "value");
+ EXPECT_EQ(A.Signature, "");
+ EXPECT_EQ(A.Kind, CompletionItemKind::Method);
+ EXPECT_EQ(A.ReturnType, "int");
+ EXPECT_
[PATCH] D53654: [clang] Improve ctor initializer completions.
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:176 public: +void AddConstructorResults(const CXXRecordDecl *Record, Result R); + AddConstructorResults is not defined and not used. Remove it? Repository: rC Clang https://reviews.llvm.org/D53654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions
xbolva00 added a comment. Ping Anything else to be done here? I would like to finish this patch to work on other things. https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ilya-biryukov added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
ioeric wrote:
> ilya-biryukov wrote:
> > Anonymous namespace inside other namespaces will also produce duplicate
> > scopes.
> > Maybe simply remove the duplicates from the vector before we return it?
> `printNamespaceScope()` will return "" for all anonymous namespaces, which
> should be covered as well.
I don't think that's the case.
My understanding is that
`printNamespaceScope("foobar::")` will return
`"foo::bar::"`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
davide added a comment. No worries :) l In https://reviews.llvm.org/D44100#1282148, @martong wrote: > Hi Davide, > > I'd like to draw your attention to an important relation of this patch to a > bug in LLDB which manifests as incorrect vtable layouts for LLDB expression > code. > Please see the conversation started with this message: > https://lists.llvm.org/pipermail/cfe-dev/2018-August/058842.html > I think that the solution to that problem would be exactly this patch, i.e. > to put the methods in the proper order during import. > > Are the failing lldb tests related to ObjC code? Some of them are, see e.g. http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/11972/console @shafik is the person who's driving better lldb C++ support on our side, so you might consider chatting with him about this. I just got the bots in a greener state again. Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ioeric updated this revision to Diff 171924.
ioeric added a comment.
- Clarify documentation for printNamespaceScope
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
Files:
clangd/AST.h
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
Index: clangd/AST.h
===
--- clangd/AST.h
+++ clangd/AST.h
@@ -39,7 +39,9 @@
/// like inline namespaces.
std::string printQualifiedName(const NamedDecl &ND);
-/// Returns the first enclosing namespace scope starting from \p DC.
+/// Returns the first enclosing namespace scope starting from \p DC. This
+/// returns "" if there is no enclosing namespace or the enclosing namespace is
+/// anonymousor or inline.
std::string printNamespaceScope(const DeclContext &DC);
/// Gets the symbol ID for a declaration, if possible.
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
Index: clangd/AST.h
===
--- clangd/AST.h
+++ clangd/AST.h
@@ -39,7 +39,9 @@
/// like inline namespaces.
std::string printQualifiedName(const NamedDecl &ND);
-/// Returns the first enclosing namespace scope starting from \p DC.
+/// Returns the first enclosing namespace scope starting from \p DC. This
+/// returns "" if there is no enclosing namespace or the enclosing namespace is
+/// anonymousor or inline.
std::string printNamespaceScope(const DeclContext &DC);
/// Gets the symbol ID for a declaration, if possible.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished
davide added a comment. In https://reviews.llvm.org/D44100#1282157, @a.sidorin wrote: > Hello everyone. > @martong : this version of patch reorders only fields and friends. No method > reordering is done (I can check if it works, though, and add the feature). > @davide: Unfortunately, I was not able to reproduce the problem on Linux. > Could you please provide a link to a buildbot failure or some environment > description so I can reproduce the issue or, at least, take a look? http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/11972/console It's unfortunate because this doesn't show a backtrace. I'm afraid at this point the easiest way to reproduce this is on MacOS directly. Repository: rC Clang https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53654: [clang] Improve ctor initializer completions.
kadircet updated this revision to Diff 171926.
kadircet added a comment.
- Revert back declaration.
Repository:
rC Clang
https://reviews.llvm.org/D53654
Files:
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/ctor-initializer.cpp
test/Index/complete-ctor-inits.cpp
test/Index/complete-cxx-inline-methods.cpp
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -21,6 +21,13 @@
int value;
MyCls *object;
};
+
+template
+class X {};
+
+class Y : public X {
+ Y() : X() {}
+};
}
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
@@ -35,10 +42,12 @@
// CHECK-NEXT: Container Kind: StructDecl
// RUN: c-index-test -code-completion-at=%s:18:41 %s | FileCheck -check-prefix=CHECK-CTOR-INIT %s
-// CHECK-CTOR-INIT: NotImplemented:{TypedText MyCls}{LeftParen (}{Placeholder args}{RightParen )} (7)
-// CHECK-CTOR-INIT: MemberRef:{TypedText object}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CTOR-INIT: MemberRef:{TypedText value}{LeftParen (}{Placeholder args}{RightParen )} (35)
+// CHECK-CTOR-INIT: ClassDecl:{TypedText MyCls}{LeftParen (}{Placeholder MyCls}{RightParen )} (7)
+// CHECK-CTOR-INIT: MemberRef:{TypedText object}{LeftParen (}{Placeholder MyCls *}{RightParen )} (35)
+// CHECK-CTOR-INIT: MemberRef:{TypedText value}{LeftParen (}{Placeholder int}{RightParen )} (35)
// RUN: c-index-test -code-completion-at=%s:18:55 %s | FileCheck -check-prefix=CHECK-CTOR-INIT-2 %s
-// CHECK-CTOR-INIT-2-NOT: NotImplemented:{TypedText MyCls}{LeftParen (}{Placeholder args}{RightParen )}
-// CHECK-CTOR-INIT-2: MemberRef:{TypedText object}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CTOR-INIT-2: MemberRef:{TypedText value}{LeftParen (}{Placeholder args}{RightParen )} (7)
+// CHECK-CTOR-INIT-2-NOT: ClassDecl:{TypedText MyCls}{LeftParen (}{Placeholder MyCls}{RightParen )} (7)
+// CHECK-CTOR-INIT-2: MemberRef:{TypedText object}{LeftParen (}{Placeholder MyCls *}{RightParen )} (35)
+// CHECK-CTOR-INIT-2: MemberRef:{TypedText value}{LeftParen (}{Placeholder int}{RightParen )} (7)
+// RUN: c-index-test -code-completion-at=%s:29:9 %s | FileCheck -check-prefix=CHECK-CTOR-INIT-3 %s
+// CHECK-CTOR-INIT-3: ClassDecl:{TypedText X}{LeftParen (}{Placeholder X}{RightParen )} (7)
Index: test/Index/complete-ctor-inits.cpp
===
--- test/Index/complete-ctor-inits.cpp
+++ test/Index/complete-ctor-inits.cpp
@@ -30,27 +30,33 @@
};
// RUN: c-index-test -code-completion-at=%s:18:10 %s | FileCheck -check-prefix=CHECK-CC1 %s
-// CHECK-CC1: MemberRef:{TypedText a}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: MemberRef:{TypedText b}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: MemberRef:{TypedText c}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: NotImplemented:{TypedText Virt}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC1: NotImplemented:{TypedText X}{LeftParen (}{Placeholder args}{RightParen )} (7)
-// CHECK-CC1: NotImplemented:{TypedText Y}{LeftParen (}{Placeholder args}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText a}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText b}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: MemberRef:{TypedText c}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder const Virt &}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder Virt &&}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText X}{LeftParen (}{Placeholder int}{RightParen )} (7)
+// CHECK-CC1: CXXConstructor:{TypedText Y}{LeftParen (}{Placeholder const Y &}{RightParen )} (35)
+// CHECK-CC1: CXXConstructor:{TypedText Y}{LeftParen (}{Placeholder Y &&}{RightParen )} (35)
// RUN: c-index-test -code-completion-at=%s:18:23 %s | FileCheck -check-prefix=CHECK-CC2 %s
-// CHECK-CC2: MemberRef:{TypedText a}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: MemberRef:{TypedText b}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: MemberRef:{TypedText c}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: NotImplemented:{TypedText Virt}{LeftParen (}{Placeholder args}{RightParen )} (35)
-// CHECK-CC2: NotImplemented:{TypedText Y}{LeftParen (}{Placeholder args}{RightParen )} (7)
+// CHECK-CC2: MemberRef:{TypedText a}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: MemberRef:{TypedText b}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: MemberRef:{TypedText c}{LeftParen (}{Placeholder int}{RightParen )} (35)
+// CHECK-CC2: CXXConstructor:{TypedText Virt}{LeftParen (}{Placeholder const Virt &}{RightParen )} (35)
+// CHECK-CC2: CXXConstructor:{TypedText Virt
[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'
astrelni updated this revision to Diff 171925.
astrelni marked 9 inline comments as done.
astrelni added a comment.
Thanks for the feedback so far.
Reply to review comments.
- Improve comments.
- Fix matcher to check for invalid source range.
- Improve test coverage for templates and macros.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
clang-tidy/abseil/UpgradeDurationConversionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-upgrade-duration-conversions.cpp
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -12,6 +12,7 @@
abseil-redundant-strcat-calls
abseil-string-find-startswith
abseil-str-cat-append
+ abseil-upgrade-duration-conversions
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
===
--- docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
+++ docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
@@ -0,0 +1,42 @@
+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+The operators and factories will be updated to only accept arithmetic types to
+prevent unintended behavior. Passing an argument of class type will result in
+compile error, even if the type is implicitly convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+ std::atomic a;
+ absl::Duration d = absl::Milliseconds(a);
+ d *= a;
+
+becomes
+
+.. code-block:: c++
+
+ std::atomic a;
+ absl::Duration d = absl::Milliseconds(static_cast(a));
+ d *= static_cast(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating point type.
Index: test/clang-tidy/abseil-upgrade-duration-conversions.cpp
===
--- test/clang-tidy/abseil-upgrade-duration-conversions.cpp
+++ test/clang-tidy/abseil-upgrade-duration-conversions.cpp
@@ -0,0 +1,400 @@
+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+ Duration &operator*=(int64_t r);
+ Duration &operator*=(float r);
+ Duration &operator*=(double r);
+ template Duration &operator*=(T r);
+
+ Duration &operator/=(int64_t r);
+ Duration &operator/=(float r);
+ Duration &operator/=(double r);
+ template Duration &operator/=(T r);
+};
+
+template Duration operator*(Duration lhs, T rhs);
+template Duration operator*(T lhs, Duration rhs);
+template Duration operator/(Duration lhs, T rhs);
+
+constexpr Duration Nanoseconds(int64_t n);
+constexpr Duration Microseconds(int64_t n);
+constexpr Duration Milliseconds(int64_t n);
+constexpr Duration Seconds(int64_t n);
+constexpr Duration Minutes(int64_t n);
+constexpr Duration Hours(int64_t n);
+
+template struct EnableIfFloatImpl {};
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template using EnableIfFloat = typename EnableIfFloatImpl::Type;
+
+template = 0> Duration Nanoseconds(T n);
+template = 0> Duration Microseconds(T n);
+template = 0> Duration Milliseconds(T n);
+template = 0> Duration Seconds(T n);
+template = 0> Duration Minutes(T n);
+template = 0> Duration Hours(T n);
+
+} // namespace absl
+
+template struct ConvertibleTo {
+ operator T() const;
+};
+
+void arithmeticOperatorBasicPositive() {
+ absl::D
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ioeric updated this revision to Diff 171928.
ioeric added a comment.
- revert wrong comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
Files:
clangd/AST.h
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
Index: clangd/AST.h
===
--- clangd/AST.h
+++ clangd/AST.h
@@ -39,7 +39,9 @@
/// like inline namespaces.
std::string printQualifiedName(const NamedDecl &ND);
-/// Returns the first enclosing namespace scope starting from \p DC.
+/// Returns the first enclosing namespace scope starting from \p DC. This
+/// returns "" if there is no enclosing namespace or the enclosing namespace is
+/// anonymousor or inline.
std::string printNamespaceScope(const DeclContext &DC);
/// Gets the symbol ID for a declaration, if possible.
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1142,6 +1142,18 @@
UnorderedElementsAre("";
}
+TEST(CompletionTest, NoDuplicatedGlobalScope) {
+ auto Requests = captureIndexRequests(R"cpp(
+ namespace { void f(); }
+ namespace ns {
+ ^
+ } // namespace ns
+ )cpp");
+
+ EXPECT_THAT(Requests, ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ ElementsAre("ns::", "";
+}
+
TEST(CompletionTest, NoIndexCompletionsInsideClasses) {
auto Completions = completions(
R"cpp(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -560,10 +560,13 @@
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ } else if (isa(Context)) {
+auto S = printNamespaceScope(*Context);
+if (!S.empty())
+ Info.AccessibleScopes.push_back(std::move(S));
+ }
}
return Info;
};
Index: clangd/AST.h
===
--- clangd/AST.h
+++ clangd/AST.h
@@ -39,7 +39,9 @@
/// like inline namespaces.
std::string printQualifiedName(const NamedDecl &ND);
-/// Returns the first enclosing namespace scope starting from \p DC.
+/// Returns the first enclosing namespace scope starting from \p DC. This
+/// returns "" if there is no enclosing namespace or the enclosing namespace is
+/// anonymousor or inline.
std::string printNamespaceScope(const DeclContext &DC);
/// Gets the symbol ID for a declaration, if possible.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, ioeric,
ilya-biryukov, krytarowski.
Since llvm/Config/config.h is not available on standalone builds,
use __USE_POSIX instead of HAVE_PTHREAD_H and get rid of the include.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53935
Files:
clangd/Threading.cpp
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#if defined(__USE_POSIX)
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#if defined(__USE_POSIX) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#if defined(__USE_POSIX)
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#if defined(__USE_POSIX) && defined(__linux__)
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'
astrelni marked an inline comment as done.
astrelni added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:118
+AST_MATCHER_P(Expr, hasSourceRange, SourceRange, Range) {
+ return Node.getSourceRange() == Range;
+}
JonasToth wrote:
> What happens on invalid ranges? Are they considered equal, or is it forbidden
> to pass them in?
Good point. I think this is a non-issue with the way the code is set up now,
but better make it explicitly work :). We only care about valid source ranges.
I've updated the name of the matcher and added a check here for the Node's
source range and another in the matcher below to return false early.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:123
+ auto HasMatchingDependentDescendant = hasDescendant(
+ expr(hasSourceRange(Node.getSourceRange()), isInstantiationDependent()));
+ return expr(anyOf(hasAncestor(
JonasToth wrote:
> Please add a test-case where the `Node.getSourceRange()` is a macro, if not
> already existing. I believe that is currently missing.
Yes, thank you, that was missing. Added a few macro + template interactions.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:158
+ *Result.Context)
+ .empty()) {
+ diag(ArgExpr->getBeginLoc(), Message);
JonasToth wrote:
> You could ellide these braces, but I feel that this matching code be merged
> into one matcher with `equalsBoundNode()` (see ASTMatcher reference).
Started with removing braces.
Sorry I had a look at `equalsBoundNode()`, but couldn't see exactly what you
meant. Could you please elaborate about the merging?
Comment at: test/clang-tidy/abseil-upgrade-duration-conversions.cpp:142
+
+template void templateForOpsSpecialization(T) {}
+template <>
JonasToth wrote:
> what about non-type template parameters? Do they need consideration for the
> check as well?
> If i am not mistaken floats are allowed in newwer standards as well.
IIUC non-type template parameters should be no different for this check. This
particular case is to make sure explicit specializations are treated
differently from instantiations and get fix-it hints.
https://reviews.llvm.org/D53830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.
ioeric planned changes to this revision. ioeric added a comment. I got the behavior of `printNamespaceScope` wrong in this patch. Will update. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
lebedev.ri added a comment. How does this play with `LLVM_ENABLE_THREADS` cmake option? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/Threading.cpp:7 #include -#ifdef HAVE_PTHREAD_H +#if defined(__USE_POSIX) #include nit: just ifdef? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
sammccall added inline comments.
Comment at: clangd/Threading.cpp:104
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#if defined(__USE_POSIX) && defined(__linux__)
sched_param priority;
the posix seems redundant here too
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53935
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53654: [clang] Improve ctor initializer completions.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the change! Repository: rC Clang https://reviews.llvm.org/D53654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
kadircet updated this revision to Diff 171931.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53935
Files:
clangd/Threading.cpp
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
sammccall added a comment. In https://reviews.llvm.org/D53935#1282303, @lebedev.ri wrote: > How does this play with `LLVM_ENABLE_THREADS` cmake option? Clangd doesn't support/respect that, and I don't think anyone's building in that configuration. If this causes actual problems e.g. with bots, we should sort out a cmake solution so such bots don't build clangd. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky marked 2 inline comments as done.
oleg.smolsky added inline comments.
Comment at: unittests/Format/FormatTest.cpp:11854
+ // case above.
+ {
+auto Style = getGoogleStyle();
djasper wrote:
> No need to use a scope here. Feel free to redefine Style.
>
> If in fact you feel like that is getting out of hand, maybe extract a
> separate TEST_F() for this.
OK, sure, removed the scoped block.
Comment at: unittests/Format/FormatTest.cpp:11865
+ }
+ {
+verifyFormat("SomeFunction(\n"
djasper wrote:
> No need for this scope.
Oh, right, that's vestigial. Removed.
Repository:
rC Clang
https://reviews.llvm.org/D52676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345729: Delete dependency on config.h (authored by
kadircet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53935?vs=171931&id=171933#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53935
Files:
clangd/Threading.cpp
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53935: Delete dependency on config.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345729: Delete dependency on config.h (authored by kadircet,
committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53935
Files:
clang-tools-extra/trunk/clangd/Threading.cpp
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r345729 - Delete dependency on config.h
Author: kadircet
Date: Wed Oct 31 08:37:09 2018
New Revision: 345729
URL: http://llvm.org/viewvc/llvm-project?rev=345729&view=rev
Log:
Delete dependency on config.h
Summary:
Since llvm/Config/config.h is not available on standalone builds,
use __USE_POSIX instead of HAVE_PTHREAD_H and get rid of the include.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: lebedev.ri, krytarowski, ilya-biryukov, ioeric, jkorous, arphaman,
jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53935
Modified:
clang-tools-extra/trunk/clangd/Threading.cpp
Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=345729&r1=345728&r2=345729&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Wed Oct 31 08:37:09 2018
@@ -1,11 +1,10 @@
#include "Threading.h"
#include "Trace.h"
#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Config/config.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Threading.h"
#include
-#ifdef HAVE_PTHREAD_H
+#ifdef __USE_POSIX
#include
#endif
@@ -102,7 +101,7 @@ void wait(std::unique_lock &
}
void setThreadPriority(std::thread &T, ThreadPriority Priority) {
-#if defined(HAVE_PTHREAD_H) && defined(__linux__)
+#ifdef __linux__
sched_param priority;
priority.sched_priority = 0;
pthread_setschedparam(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.
sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, xazax.hun.
This codifies the mostly-current state:
- The stuff in ClangTidy.h is reasonable for checks to depend on.
- The stuff in ClangTidyDiagnosticConsumer.h is a mish-mash of clang-tidy
- checks are expected to treat ClangTidyContext as opaque
implementation details that checks shouldn't depend on (and most don't!)
But because there's no comments and the former currently #includes the latter,
this isn't clear and is violated in places:
- exportReplacements is public but consumes ClangTidyError, a private API
- Context exports all sorts of details publicly that checks shouldn't depend
on, but checks must use Context->getOptions() in some cases
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53936
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyModule.cpp
clang-tidy/ClangTidyModule.h
clang-tidy/google/TodoCommentCheck.cpp
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/ClangTidyMain.cpp
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -16,6 +16,7 @@
//===--===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "clang/Config/config.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "llvm/Support/Process.h"
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "../ClangTidyModule.h"
#include "clang/Config/config.h"
#include "clang/Frontend/CompilerInstance.h"
Index: clang-tidy/google/TodoCommentCheck.cpp
===
--- clang-tidy/google/TodoCommentCheck.cpp
+++ clang-tidy/google/TodoCommentCheck.cpp
@@ -8,6 +8,8 @@
//===--===//
#include "TodoCommentCheck.h"
+// FIXME: provide some other way to get at the username.
+#include "../ClangTidyDiagnosticConsumer.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
@@ -52,9 +54,10 @@
};
TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
- Handler(llvm::make_unique(
- *this, Context->getOptions().User)) {}
+: ClangTidyCheck(Name, Context) {
+ Handler =
+ llvm::make_unique(*this, Context->getOptions().User);
+}
void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) {
Compiler.getPreprocessor().addCommentHandler(Handler.get());
Index: clang-tidy/ClangTidyModule.h
===
--- clang-tidy/ClangTidyModule.h
+++ clang-tidy/ClangTidyModule.h
@@ -63,11 +63,11 @@
});
}
- /// \brief Create instances of all checks matching \p CheckRegexString and
- /// store them in \p Checks.
+ /// Create instances of all checks that match the filter and store them.
///
/// The caller takes ownership of the return \c ClangTidyChecks.
- void createChecks(ClangTidyContext *Context,
+ void createChecks(llvm::function_ref Filter,
+ClangTidyContext *Context,
std::vector> &Checks);
typedef std::map FactoryMap;
Index: clang-tidy/ClangTidyModule.cpp
===
--- clang-tidy/ClangTidyModule.cpp
+++ clang-tidy/ClangTidyModule.cpp
@@ -22,10 +22,11 @@
}
void ClangTidyCheckFactories::createChecks(
+llvm::function_ref Filter,
ClangTidyContext *Context,
std::vector> &Checks) {
for (const auto &Factory : Factories) {
-if (Context->isCheckEnabled(Factory.first))
+if (Filter(Factory.first))
Checks.emplace_back(Factory.second(Factory.first, Context));
}
}
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -6,6 +6,9 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+// This file contains private APIs that checks should not depend on.
+// FIXME: the contents have diverged significantly from the name, split this up.
+//===--===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_C
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
krasimir added a comment. Looks good! Will stamp when the scopes are removed. Oleg, do you need someone to commit this for you? Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky updated this revision to Diff 171932.
oleg.smolsky marked 2 inline comments as done.
Repository:
rC Clang
https://reviews.llvm.org/D52676
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3277,11 +3277,12 @@
"});");
FormatStyle Style = getGoogleStyle();
Style.ColumnLimit = 45;
- verifyFormat("Debug(a,\n"
- " {\n"
- "if () return;\n"
- " },\n"
- " a);",
+ verifyFormat("Debug(\n"
+ "a,\n"
+ "{\n"
+ " if () return;\n"
+ "},\n"
+ "a);",
Style);
verifyFormat("SomeFunction({MACRO({ return output; }), b});");
@@ -11739,6 +11740,18 @@
"x.end(), //\n"
"[&](int, int) { return 1; });\n"
"}\n");
+ verifyFormat("void f() {\n"
+ " other.other.other.other.other(\n"
+ " x.begin(), x.end(),\n"
+ " [something, rather](int, int, int, int, int, int, int) { return 1; });\n"
+ "}\n");
+ verifyFormat("void f() {\n"
+ " other.other.other.other.other(\n"
+ " x.begin(), x.end(),\n"
+ " [something, rather](int, int, int, int, int, int, int) {\n"
+ "//\n"
+ " });\n"
+ "}\n");
verifyFormat("SomeFunction([]() { // A cool function...\n"
" return 43;\n"
"});");
@@ -11790,9 +11803,9 @@
verifyFormat("SomeFunction({[&] {\n"
" // comment\n"
"}});");
- verifyFormat("virtual (std::function =\n"
- " [&]() { return true; },\n"
- " a a);");
+ verifyFormat("virtual (\n"
+ "std::function = [&]() { return true; },\n"
+ "a a);");
// Lambdas with return types.
verifyFormat("int c = []() -> int { return 2; }();\n");
@@ -11819,17 +11832,76 @@
" return 1; //\n"
"};");
- // Multiple lambdas in the same parentheses change indentation rules.
+ // Multiple lambdas in the same parentheses change indentation rules. These
+ // lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
"[]() {\n"
- " int i = 42;\n"
- " return i;\n"
+ " //\n"
"},\n"
"[]() {\n"
- " int j = 43;\n"
- " return j;\n"
+ " //\n"
"});");
+ // A lambda passed as arg0 is always pushed to the next line.
+ verifyFormat("SomeFunction(\n"
+ "[this] {\n"
+ " //\n"
+ "},\n"
+ "1);\n");
+
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
+ // case above.
+ auto Style = getGoogleStyle();
+ Style.BinPackArguments = false;
+ verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ " //\n"
+ "},\n"
+ "b);\n",
+ Style);
+ verifyFormat("SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ " //\n"
+ "},\n"
+ "b);\n");
+
+ // A lambda with a very long line forces arg0 to be pushed out irrespective of
+ // the BinPackArguments value (as long as the code is wide enough).
+ verifyFormat("something->SomeFunction(\n"
+ "a,\n"
+ "[this] {\n"
+ " D1();\n"
+ "},\n"
+ "b);\n");
+
+ // A multi-line lambda is pulled up as long as the introducer fits on the previous
+ // line and there are no further args.
+ verifyFormat("function(1, [this, that] {\n"
+ " //\n"
+ "});\n");
+ verifyFormat("function([this, that] {\n"
+ " //\n"
+ "});\n");
+ // FIXME: this format is not ideal and we should consider forcing the first arg
+ // onto its own line.
+ verifyFormat("function(a, b, c, //\n"
+ " d, [this, that] {\n"
+ " //\n"
+ " });\n");
+
+ // Multiple lambdas are treated correctly even wh
[PATCH] D51223: Update tests for new YAMLIO polymorphic traits
scott.linder updated this revision to Diff 171936. scott.linder added a comment. Rebase @klimek is there anyone I should add to take a look at this? As far as the YAML is concerned I believe this is a non-functional change. https://reviews.llvm.org/D51223 Files: unittests/Tooling/DiagnosticsYamlTest.cpp unittests/Tooling/RefactoringActionRulesTest.cpp Index: unittests/Tooling/RefactoringActionRulesTest.cpp === --- unittests/Tooling/RefactoringActionRulesTest.cpp +++ unittests/Tooling/RefactoringActionRulesTest.cpp @@ -117,8 +117,8 @@ "Key: 'input.cpp:30'\n" "FilePath:input.cpp\n" "Error: ''\n" - "InsertedHeaders: \n" - "RemovedHeaders: \n" + "InsertedHeaders: []\n" + "RemovedHeaders: []\n" "Replacements:\n" // Extra whitespace here! " - FilePath:input.cpp\n" "Offset: 30\n" Index: unittests/Tooling/DiagnosticsYamlTest.cpp === --- unittests/Tooling/DiagnosticsYamlTest.cpp +++ unittests/Tooling/DiagnosticsYamlTest.cpp @@ -82,7 +82,7 @@ "Message: 'message #3'\n" "FileOffset: 72\n" "FilePath:'path/to/source2.cpp'\n" -"Replacements:\n" +"Replacements:[]\n" "...\n", YamlContentStream.str()); } @@ -113,7 +113,7 @@ "Message: 'message #3'\n" "FileOffset: 98\n" "FilePath:path/to/source.cpp\n" -"Replacements:\n" +"Replacements:[]\n" "...\n"; TranslationUnitDiagnostics TUDActual; yaml::Input YAML(YamlContent); Index: unittests/Tooling/RefactoringActionRulesTest.cpp === --- unittests/Tooling/RefactoringActionRulesTest.cpp +++ unittests/Tooling/RefactoringActionRulesTest.cpp @@ -117,8 +117,8 @@ "Key: 'input.cpp:30'\n" "FilePath:input.cpp\n" "Error: ''\n" - "InsertedHeaders: \n" - "RemovedHeaders: \n" + "InsertedHeaders: []\n" + "RemovedHeaders: []\n" "Replacements:\n" // Extra whitespace here! " - FilePath:input.cpp\n" "Offset: 30\n" Index: unittests/Tooling/DiagnosticsYamlTest.cpp === --- unittests/Tooling/DiagnosticsYamlTest.cpp +++ unittests/Tooling/DiagnosticsYamlTest.cpp @@ -82,7 +82,7 @@ "Message: 'message #3'\n" "FileOffset: 72\n" "FilePath:'path/to/source2.cpp'\n" -"Replacements:\n" +"Replacements:[]\n" "...\n", YamlContentStream.str()); } @@ -113,7 +113,7 @@ "Message: 'message #3'\n" "FileOffset: 98\n" "FilePath:path/to/source.cpp\n" -"Replacements:\n" +"Replacements:[]\n" "...\n"; TranslationUnitDiagnostics TUDActual; yaml::Input YAML(YamlContent); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1282335, @krasimir wrote: > Looks good! Will stamp when the scopes are removed. Cool, thanks, Krasimir. I've just posted the updated patch. > Oleg, do you need someone to commit this for you? I do, could you commit this please? Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.
sammccall updated this revision to Diff 171937.
sammccall added a comment.
Add FIXME to fuchsia check that uses private APIs.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53936
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyModule.cpp
clang-tidy/ClangTidyModule.h
clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
clang-tidy/google/TodoCommentCheck.cpp
clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tidy/tool/ClangTidyMain.cpp
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -16,6 +16,7 @@
//===--===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "clang/Config/config.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "llvm/Support/Process.h"
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "../ClangTidyModule.h"
#include "clang/Config/config.h"
#include "clang/Frontend/CompilerInstance.h"
Index: clang-tidy/google/TodoCommentCheck.cpp
===
--- clang-tidy/google/TodoCommentCheck.cpp
+++ clang-tidy/google/TodoCommentCheck.cpp
@@ -8,6 +8,8 @@
//===--===//
#include "TodoCommentCheck.h"
+// FIXME: provide some other way to get at the username.
+#include "../ClangTidyDiagnosticConsumer.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
@@ -52,9 +54,10 @@
};
TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
- Handler(llvm::make_unique(
- *this, Context->getOptions().User)) {}
+: ClangTidyCheck(Name, Context) {
+ Handler =
+ llvm::make_unique(*this, Context->getOptions().User);
+}
void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) {
Compiler.getPreprocessor().addCommentHandler(Handler.get());
Index: clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
===
--- clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
+++ clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
#include "../ClangTidy.h"
+// FIXME: stop using GlobList, it's not part of the public interface.
#include "../ClangTidyDiagnosticConsumer.h"
#include "../utils/OptionsUtils.h"
Index: clang-tidy/ClangTidyModule.h
===
--- clang-tidy/ClangTidyModule.h
+++ clang-tidy/ClangTidyModule.h
@@ -63,11 +63,11 @@
});
}
- /// \brief Create instances of all checks matching \p CheckRegexString and
- /// store them in \p Checks.
+ /// Create instances of all checks that match the filter and store them.
///
/// The caller takes ownership of the return \c ClangTidyChecks.
- void createChecks(ClangTidyContext *Context,
+ void createChecks(llvm::function_ref Filter,
+ClangTidyContext *Context,
std::vector> &Checks);
typedef std::map FactoryMap;
Index: clang-tidy/ClangTidyModule.cpp
===
--- clang-tidy/ClangTidyModule.cpp
+++ clang-tidy/ClangTidyModule.cpp
@@ -22,10 +22,11 @@
}
void ClangTidyCheckFactories::createChecks(
+llvm::function_ref Filter,
ClangTidyContext *Context,
std::vector> &Checks) {
for (const auto &Factory : Factories) {
-if (Context->isCheckEnabled(Factory.first))
+if (Filter(Factory.first))
Checks.emplace_back(Factory.second(Factory.first, Context));
}
}
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -6,6 +6,9 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+// This file contains private APIs that checks should not depend on.
+// FIXME: the contents have diverged significantly from the name, split this up.
+//===--===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CL
