[clang-tools-extra] r277577 - [clang-tidy] address concerns with rL277340
Author: omtcyfz
Date: Wed Aug 3 01:54:24 2016
New Revision: 277577
URL: http://llvm.org/viewvc/llvm-project?rev=277577&view=rev
Log:
[clang-tidy] address concerns with rL277340
alexfh raised a concern with https://reviews.llvm.org/rL277340
After retabbing indentation of .. code-block:: was increased to 8, 4 spaces
indentation should be enough.
Reviewers: alexfh
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst?rev=277577&r1=277576&r2=277577&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
Wed Aug 3 01:54:24 2016
@@ -20,25 +20,25 @@ Before:
.. code:: c++
-std::vector v;
-v.push_back(MyClass(21, 37));
+std::vector v;
+v.push_back(MyClass(21, 37));
-std::vector> w;
+std::vector> w;
-w.push_back(std::pair(21, 37));
-w.push_back(std::make_pair(21L, 37L));
+w.push_back(std::pair(21, 37));
+w.push_back(std::make_pair(21L, 37L));
After:
.. code:: c++
-std::vector v;
-v.emplace_back(21, 37);
+std::vector v;
+v.emplace_back(21, 37);
-std::vector> w;
-w.emplace_back(21, 37);
-// This will be fixed to w.push_back(21, 37); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+std::vector> w;
+w.emplace_back(21, 37);
+// This will be fixed to w.push_back(21, 37); in next version
+w.emplace_back(std::make_pair(21L, 37L);
The other situation is when we pass arguments that will be converted to a type
inside a container.
@@ -47,15 +47,15 @@ Before:
.. code:: c++
-std::vector > v;
-v.push_back("abc");
+std::vector > v;
+v.push_back("abc");
After:
.. code:: c++
-std::vector > v;
-v.emplace_back("abc");
+std::vector > v;
+v.emplace_back("abc");
In some cases the transformation would be valid, but the code
@@ -65,9 +65,9 @@ In this case the calls of ``push_back``
.. code:: c++
std::vector> v;
-v.push_back(std::unique_ptr(new int(0)));
-auto *ptr = new int(1);
-v.push_back(std::unique_ptr(ptr));
+v.push_back(std::unique_ptr(new int(0)));
+auto *ptr = new int(1);
+v.push_back(std::unique_ptr(ptr));
This is because replacing it with ``emplace_back`` could cause a leak of this
pointer if ``emplace_back`` would throw exception before emplacement
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r277578 - [extra-tools] Fix extra tools build bot warnings due to incorrect doc
Author: etienneb Date: Wed Aug 3 01:59:46 2016 New Revision: 277578 URL: http://llvm.org/viewvc/llvm-project?rev=277578&view=rev Log: [extra-tools] Fix extra tools build bot warnings due to incorrect doc /home/llvmbb/llvm-build-dir/clang-x86_64-debian-fast/llvm.src/tools/clang/tools/extra/clang-tidy/mpi/TypeMismatchCheck.cpp:172:12: warning: parameter 'Complex' not found in the function declaration [-Wdocumentation] /home/llvmbb/llvm-build-dir/clang-x86_64-debian-fast/llvm.src/tools/clang/tools/extra/clang-tidy/mpi/TypeMismatchCheck.cpp:206:12: warning: parameter 'Complex' not fo Modified: clang-tools-extra/trunk/clang-tidy/mpi/TypeMismatchCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/mpi/TypeMismatchCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/mpi/TypeMismatchCheck.cpp?rev=277578&r1=277577&r2=277578&view=diff == --- clang-tools-extra/trunk/clang-tidy/mpi/TypeMismatchCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/mpi/TypeMismatchCheck.cpp Wed Aug 3 01:59:46 2016 @@ -169,7 +169,7 @@ static bool isCComplexTypeMatching(const /// Check if a complex templated buffer type matches /// the MPI datatype. /// -/// \param Complex buffer type +/// \param Template buffer type /// \param BufferTypeName buffer type name, gets assigned /// \param MPIDatatype name of the MPI datatype /// \param LO language options @@ -203,7 +203,7 @@ isCXXComplexTypeMatching(const TemplateS /// Check if a fixed size width buffer type matches the MPI datatype. /// -/// \param Complex buffer type +/// \param Typedef buffer type /// \param BufferTypeName buffer type name, gets assigned /// \param MPIDatatype name of the MPI datatype /// ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23086: [OpenCL] Generate concrete struct type for ndrange_t
bader added inline comments. Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:12 @@ -11,3 +11,3 @@ unsigned flags = 0; - // CHECK: %ndrange = alloca %opencl.ndrange_t* + // CHECK: %ndrange = alloca %ndrange_t ndrange_t ndrange; Could you also add a regression test to validate that ndrange_t is defined in LLVM as struct type, please? Repository: rL LLVM https://reviews.llvm.org/D23086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23058: [clang-rename] add support for template instantiations
omtcyfz added inline comments. Comment at: test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp:2 @@ +1,3 @@ +// RUN: cat %s > %t.cpp +// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i -- +// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s alexfh wrote: > This test looks exactly as the one above except for the offset. Instead of > duplicating tests, we can run clang-rename multiple times and use different > -check-prefix with FileCheck. In this case moving the RUN lines to the bottom > makes even more sense. > > Also, instructions for updating -offset are incorrect, if you have multiple > different offsets. Will it work if I add multiple invocations while not adding different `-check-prefix`? The replacements are the same across the runs. https://reviews.llvm.org/D23058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23058: [clang-rename] add support for template instantiations
omtcyfz added inline comments. Comment at: test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp:2 @@ -1,3 +1,3 @@ // RUN: cat %s > %t.cpp -// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i -- +// RUN: clang-rename -offset=159 -new-name=Bar %t.cpp -i -- // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s alexfh wrote: > It makes sense to move RUN lines to the end of the file (and add > -fno-delayed-template-parsing, if windows buildbots start complaining). > It makes sense to move RUN lines to the end of the file You mean for all tests? https://reviews.llvm.org/D23058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23014: [analyzer] Model base to derived casts more precisely.
xazax.hun updated this revision to Diff 66632.
xazax.hun marked 4 inline comments as done.
xazax.hun added a comment.
- Improvements according to review comments.
https://reviews.llvm.org/D23014
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
lib/StaticAnalyzer/Core/CallEvent.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/Store.cpp
test/Analysis/NewDelete-checker-test.cpp
Index: test/Analysis/NewDelete-checker-test.cpp
===
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -377,3 +377,19 @@
delete foo;
delete foo; // expected-warning {{Attempt to delete released memory}}
}
+
+struct Base {
+ virtual ~Base() {}
+};
+
+struct Derived : Base {
+};
+
+Base *allocate() {
+ return new Derived;
+}
+
+void shouldNotReportLeak() {
+ Derived *p = (Derived *)allocate();
+ delete p;
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -292,7 +292,7 @@
return nullptr;
}
-SVal StoreManager::evalDynamicCast(SVal Base, QualType TargetType,
+SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
bool &Failed) {
Failed = false;
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -386,7 +386,7 @@
Failed = true;
// Else, evaluate the cast.
else
- val = getStoreManager().evalDynamicCast(val, T, Failed);
+ val = getStoreManager().attemptDownCast(val, T, Failed);
if (Failed) {
if (T->isReferenceType()) {
@@ -412,6 +412,28 @@
Bldr.generateNode(CastE, Pred, state);
continue;
}
+ case CK_BaseToDerived: {
+SVal val = state->getSVal(Ex, LCtx);
+QualType resultType = CastE->getType();
+if (CastE->isGLValue())
+ resultType = getContext().getPointerType(resultType);
+
+bool Failed = false;
+
+if (!val.isConstant()) {
+ val = getStoreManager().attemptDownCast(val, T, Failed);
+}
+
+// Failed to cast or the result is unknown, fall back to conservative.
+if (Failed || val.isUnknown()) {
+ val =
+svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
+ currBldrCtx->blockCount());
+}
+state = state->BindExpr(CastE, LCtx, val);
+Bldr.generateNode(CastE, Pred, state);
+continue;
+ }
case CK_NullToMemberPointer: {
// FIXME: For now, member pointers are represented by void *.
SVal V = svalBuilder.makeNull();
@@ -421,7 +443,6 @@
}
// Various C++ casts that are not handled yet.
case CK_ToUnion:
- case CK_BaseToDerived:
case CK_BaseToDerivedMemberPointer:
case CK_DerivedToBaseMemberPointer:
case CK_ReinterpretMemberPointer:
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -552,7 +552,7 @@
// FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
bool Failed;
- ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty, Failed);
+ ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
assert(!Failed && "Calling an incorrectly devirtualized method");
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -123,15 +123,18 @@
SVal evalDerivedToBase(SVal Derived, QualType DerivedPtrType,
bool IsVirtual);
- /// \brief Evaluates C++ dynamic_cast cast.
+ /// \brief Attempts to do a down cast. Used to model BaseToDerived and C++
+ ///dynamic_cast.
/// The callback may result in the following 3 scenarios:
/// - Successful cast (ex: derived is subclass of base).
/// - Failed cast (ex: derived is definitely not a subclass of base).
+ ///The distinction of this case from the next one is necessary to model
+ ///dynamic_cast.
/// - We don't know (base is a symbolic region and we don't have
///enough info to determine if the cast will succeed at run time).
/// The function returns an SVal representing the derived class; it's
/// valid only if Failed flag is set to false.
- SVal evalDynamicCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+ SVal attemptDownCast(SVal Base, QualType DerivedPtrT
Re: [PATCH] D23058: [clang-rename] add support for template instantiations
omtcyfz updated this revision to Diff 66634.
https://reviews.llvm.org/D23058
Files:
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
test/clang-rename/ComplexFunctionOverride.cpp
test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
Index: test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
===
--- /dev/null
+++ test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
@@ -0,0 +1,39 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+template
+class Foo { // CHECK: class Bar {
+public:
+ T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+ }
+ static void foo(T value) {}
+ T member;
+};
+
+template
+void func() {
+ Foo obj; // CHECK: Bar obj;
+ obj.member = T();
+ Foo::foo(); // CHECK: Bar::foo();
+}
+
+int main() {
+ Foo i; // CHECK: Bar i;
+ i.member = 0;
+ Foo::foo(0); // CHECK: Bar::foo(0);
+
+ Foo b;// CHECK: Bar b;
+ b.member = false;
+ Foo::foo(false); // CHECK: Bar::foo(false);
+
+ return 0;
+}
+
+// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
+++ test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
@@ -1,14 +1,9 @@
// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=703 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=575 -new-name=Bar %t.cpp -i --
// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
template
-class Foo { // CHECK: class Bar;
+class Foo { // CHECK: class Bar {
public:
T foo(T arg, T& ref, T* ptr) {
T value;
Index: test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
+++ test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
@@ -1,14 +1,9 @@
// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=159 -new-name=Bar %t.cpp -i --
// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
template
-class Foo { // CHECK: class Bar;
+class Foo { // CHECK: class Bar {
public:
T foo(T arg, T& ref, T* ptr) {
T value;
Index: test/clang-rename/ComplexFunctionOverride.cpp
===
--- /dev/null
+++ test/clang-rename/ComplexFunctionOverride.cpp
@@ -0,0 +1,23 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=307 -new-name=bar %t.cpp -i -- -std=c++11
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+struct A {
+ virtual void foo(); // CHECK: virtual void bar();
+};
+
+struct B : A {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct C : B {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct D : B {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct E : D {
+ void foo() override; // CHECK: void bar() override;
+};
Index: clang-rename/USRFindingAction.cpp
===
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -46,68 +46,102 @@
// AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
// Decl refers to class and adds USRs of all overridden methods if Decl refers
// to virtual method.
-class AdditionalUSRFinder : public MatchFinder::MatchCallback {
+class AdditionalUSRFinder : public RecursiveASTVisitor {
public:
explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
+: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
void Find() {
-USRSet.insert(getUSRForDecl(FoundDecl));
+// Fill OverriddenMethods and PartialSpecs storages.
+TraverseDecl(Context.getTranslationUnitDecl());
if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
- addUSRsFromOverrideSets(MethodDecl);
-}
-if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
- addUSRsOfCtorDtors(
[PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.
xazax.hun created this revision.
xazax.hun added reviewers: zaks.anna, dcoughlin, NoQ.
xazax.hun added a subscriber: cfe-commits.
Right now due to a missing brace error the assumptions that an index is inbound
in case we are under constrained is not added.
https://reviews.llvm.org/D23112
Files:
lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -157,13 +157,13 @@
// If we are under constrained and the index variables are tainted, report.
if (state_exceedsUpperBound && state_withinUpperBound) {
- if (state->isTainted(rawOffset.getByteOffset()))
+ if (state->isTainted(rawOffset.getByteOffset())) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
return;
-}
-
-// If we are constrained enough to definitely exceed the upper bound,
report.
-if (state_exceedsUpperBound) {
+ }
+} else if (state_exceedsUpperBound) {
+ // If we are constrained enough to definitely exceed the upper bound,
+ // report.
assert(!state_withinUpperBound);
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
return;
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -157,13 +157,13 @@
// If we are under constrained and the index variables are tainted, report.
if (state_exceedsUpperBound && state_withinUpperBound) {
- if (state->isTainted(rawOffset.getByteOffset()))
+ if (state->isTainted(rawOffset.getByteOffset())) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
return;
-}
-
-// If we are constrained enough to definitely exceed the upper bound, report.
-if (state_exceedsUpperBound) {
+ }
+} else if (state_exceedsUpperBound) {
+ // If we are constrained enough to definitely exceed the upper bound,
+ // report.
assert(!state_withinUpperBound);
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23071: [OpenCL] Remove extra native_ functions from opencl-c.h
Anastasia added a comment. LGTM! Repository: rL LLVM https://reviews.llvm.org/D23071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
omtcyfz updated this revision to Diff 66642.
omtcyfz added a comment.
Removed `dump()` call used for debugging purposes.
Removed unneeded header inclusion in `USRFindingAction.cpp`.
https://reviews.llvm.org/D23058
Files:
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
test/clang-rename/ComplexFunctionOverride.cpp
test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
Index: test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
===
--- /dev/null
+++ test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp
@@ -0,0 +1,39 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+template
+class Foo { // CHECK: class Bar {
+public:
+ T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+ }
+ static void foo(T value) {}
+ T member;
+};
+
+template
+void func() {
+ Foo obj; // CHECK: Bar obj;
+ obj.member = T();
+ Foo::foo(); // CHECK: Bar::foo();
+}
+
+int main() {
+ Foo i; // CHECK: Bar i;
+ i.member = 0;
+ Foo::foo(0); // CHECK: Bar::foo(0);
+
+ Foo b;// CHECK: Bar b;
+ b.member = false;
+ Foo::foo(false); // CHECK: Bar::foo(false);
+
+ return 0;
+}
+
+// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
+++ test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
@@ -1,14 +1,9 @@
// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=703 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=575 -new-name=Bar %t.cpp -i --
// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
template
-class Foo { // CHECK: class Bar;
+class Foo { // CHECK: class Bar {
public:
T foo(T arg, T& ref, T* ptr) {
T value;
Index: test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
+++ test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
@@ -1,14 +1,9 @@
// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=159 -new-name=Bar %t.cpp -i --
// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
template
-class Foo { // CHECK: class Bar;
+class Foo { // CHECK: class Bar {
public:
T foo(T arg, T& ref, T* ptr) {
T value;
Index: test/clang-rename/ComplexFunctionOverride.cpp
===
--- /dev/null
+++ test/clang-rename/ComplexFunctionOverride.cpp
@@ -0,0 +1,23 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=307 -new-name=bar %t.cpp -i -- -std=c++11
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+struct A {
+ virtual void foo(); // CHECK: virtual void bar();
+};
+
+struct B : A {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct C : B {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct D : B {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct E : D {
+ void foo() override; // CHECK: void bar() override;
+};
Index: clang-rename/USRFindingAction.cpp
===
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -20,7 +20,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/FileManager.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
@@ -36,7 +35,6 @@
using namespace llvm;
-using namespace clang::ast_matchers;
namespace clang {
namespace rename {
@@ -46,68 +44,101 @@
// AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
// Decl refers to class and adds USRs of all overridden methods if Decl refers
// to virtual method.
-class AdditionalUSRFinder : public MatchFinder::MatchCallback {
+class AdditionalUSRFinder : public RecursiveASTVisitor {
public:
explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Con
Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor
hiraditya added a comment. In https://reviews.llvm.org/D22782#504416, @EricWF wrote: > The change itself LGTM, although we probably want to inline the forward/input > iterator __init's as well. > > However I would like to see a small benchmark that demonstrates the > performance change. Please try and write the benchmark using Google Benchmark. > Some helpful links: > > - http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks > - http://github.com/google/benchmark Sure, We'll come up with a synthetic benchmark to expose performance improvements. Thanks, https://reviews.llvm.org/D22782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22834: Added 'inline' attribute to basic_string's destructor
hiraditya added a comment. In https://reviews.llvm.org/D22834#504425, @EricWF wrote: > LGTM. > > However I would like to see a small benchmark that demonstrates the > performance change. Please try and write the benchmark using Google Benchmark. > Some helpful links: > > http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks > http://github.com/google/benchmark > > Let me know if I can help. Sure, We'll come up with a synthetic benchmark to expose performance improvements. Thanks, https://reviews.llvm.org/D22834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
klimek created this revision.
klimek added reviewers: djasper, bkramer, ioeric.
klimek added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,26 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
TEST_F(ReplacementTest, CanApplyReplacements) {
FileID ID = Context.createInMemoryFile("input.cpp",
"line1\nline2\nline3\nline4");
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -138,25 +138,53 @@
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
-for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
-return llvm::make_error(
-"All replacements must have the same file path. New replacement: " +
-R.getFilePath() + ", existing replacements: " +
-Replace.getFilePath() + "\n",
-llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength(
-return llvm::make_error(
-"New replacement:\n" + R.toString() +
-"\nconflicts with existing replacement:\n" + Replace.toString(),
-llvm::inconvertibleErrorCode());
-}
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+return llvm::make_error(
+"All replacements must have the same file path. New replacement: " +
+R.getFilePath() + ", existing replacements: " +
+Replaces.begin()->getFilePath() + "\n",
+llvm::inconvertibleErrorCode());
+
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
- Replaces.insert(R);
- return llvm::Error::success();
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+ auto I = Replaces.upper_bound(AtEnd);
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ --I;
+ // If the previous entry's end is left of or touches R's start, or
+ // if the previous entry's start is right of or touches R's end, there is no
+ // overlap.
+ // The exception is if one of them is an insertion; in that case, the offsets
+ // must be equal.
+ if (I->getOffset() != R.getOffset() &&
+ (I->getOffset() + I->getLength() <= R.getOffset() ||
+ R.getOffset() + R.getLength() <= I->getOffset())) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ // Otherwise there must be overlap.
+ assert(R.getOffset() == I->getOffset() ||
+ Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength(;
+ return llvm::make_error(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
klimek updated this revision to Diff 66651.
klimek added a comment.
Remove re-implementation of overlaps check.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,26 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
TEST_F(ReplacementTest, CanApplyReplacements) {
FileID ID = Context.createInMemoryFile("input.cpp",
"line1\nline2\nline3\nline4");
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -138,25 +138,46 @@
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
-for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
-return llvm::make_error(
-"All replacements must have the same file path. New replacement: "
+
-R.getFilePath() + ", existing replacements: " +
-Replace.getFilePath() + "\n",
-llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength(
-return llvm::make_error(
-"New replacement:\n" + R.toString() +
-"\nconflicts with existing replacement:\n" +
Replace.toString(),
-llvm::inconvertibleErrorCode());
-}
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+return llvm::make_error(
+"All replacements must have the same file path. New replacement: " +
+R.getFilePath() + ", existing replacements: " +
+Replaces.begin()->getFilePath() + "\n",
+llvm::inconvertibleErrorCode());
+
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
- Replaces.insert(R);
- return llvm::Error::success();
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the
current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+ auto I = Replaces.upper_bound(AtEnd);
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ --I;
+ // If the previous entry does not overlap, we know that entries before it
+ // can also not overlap.
+ if (R.getOffset() != I->getOffset() &&
+ !Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength( {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ return llvm::make_error(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,26 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(R
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
alexfh added inline comments. Comment at: test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp:2 @@ -1,3 +1,3 @@ // RUN: cat %s > %t.cpp -// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i -- +// RUN: clang-rename -offset=159 -new-name=Bar %t.cpp -i -- // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s Yes, but it's better to modify unrelated tests in a separate patch. Comment at: test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp:3 @@ +2,3 @@ +// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i -- +// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s + Yep, should also work with the same check prefix, if the replacements are the same. https://reviews.llvm.org/D23058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23120: [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
bader created this revision.
bader added reviewers: Anastasia, yaxunl.
bader added a subscriber: cfe-commits.
In order to re-define OpenCL built-in functions
'to_{private,local,global}' in OpenCL run-time library LLVM names must
be different from the clang built-in function names.
https://reviews.llvm.org/D23120
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGenOpenCL/to_addr_builtin.cl
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- test/CodeGenOpenCL/to_addr_builtin.cl
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -14,74 +14,74 @@
generic int *gen;
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
glob = to_global(glob);
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
glob = to_global(loc);
//CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
glob = to_global(priv);
//CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
glob = to_global(gen);
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
loc = to_local(glob);
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
loc = to_local(loc);
//CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
loc = to_local(priv);
//CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
loc = to_local(gen);
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
priv = to_private(glob);
//CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
priv = to_private(loc);
//CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
priv = to_private(priv);
//CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
//CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
priv = to_private(gen);
//CHECK: %[[ARG:.*]] = addrspacecast %[[A]]* %{{.*}} to i8 addrspace(4)*
- //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[
Re: [PATCH] D23120: [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks! https://reviews.llvm.org/D23120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22766: Handle -mlong-calls on Hexagon
kparzysz added a comment. Ping? Repository: rL LLVM https://reviews.llvm.org/D22766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
aaron.ballman requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:25
@@ +24,3 @@
+
+static QualType getStrippedType(QualType T) {
+ while (const auto *PtrType = T->getAs())
I'd like to see some test cases involving attributed types, typedefs to
pointers and non-pointers, and multidimensional arrays. For instance, I'm
curious how well this handles `typedef int some_type; typedef some_type * const
* volatile *foo_ptr;` or `typedef void (__cdecl fn_ptr)(int);`
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:49-50
@@ +48,4 @@
+ const StringRef Arg2) {
+ return Function.str() + "(" + Arg0.str() + ", " + Arg1.str() + ", " +
+ Arg2.str() + ")";
+}
Please use Twine for this sort of thing.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:65
@@ +64,3 @@
+void UseAlgorithmCheck::registerMatchers(MatchFinder *Finder) {
+ // Only register the matchers for C++
+ if (!getLangOpts().CPlusPlus)
Missing full stop.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:93
@@ +92,3 @@
+
+ const auto MatchedName = Callee->getNameAsString();
+
Please do not use auto here as the type is not spelled out in the RHS (same
elsewhere).
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:96
@@ +95,3 @@
+ // Check if matched name is in map of replacements.
+ const auto it = Replacements.find(MatchedName);
+ assert(it != Replacements.end());
You should ensure that the callee is actually the function we care about.
Consider:
```
namespace awful {
void memcpy(int, int, int);
}
using namespace awful;
```
Such a use of memcpy() would trigger your check and create an invalid
replacement, I think (and there should be a test for this).
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:97
@@ +96,3 @@
+ const auto it = Replacements.find(MatchedName);
+ assert(it != Replacements.end());
+ const auto ReplacedName = it->second;
Please add some "help text" to the assert in case it ever triggers. e.g., &&
"Replacements list does not match list of registered matcher names" or some
such.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:101
@@ +100,3 @@
+ const auto Loc = MatchedExpr->getExprLoc();
+ auto Diag = diag(Loc, "Use " + ReplacedName + " instead of " + MatchedName);
+
Diagnostics should start with a lowercase letter.
More importantly, this warning doesn't actually describe a problem to the user.
Given the fact that this only should be triggered on uses where memcpy and
std::copy are interchangeable, it's not really a *warning* at all (due to the
interchangeability) as there is nothing dangerous about the original code that
the suggestion will fix. @alexfh, what do you think of the idea of this being a
Note rather than a Warning? I know it's unorthodox, but literally every
instance of this diagnostic can be ignored or replaced and there should be no
semantic effect on the code either way. Most other checks in modernize have
more obvious benefits to modifying the code and so Warning is reasonably
appropriate.
If this text remains a warning, it should describe what is dangerous with the
code, not simply how to transform the code. Perhaps "'memcpy' reduces
type-safety, consider using 'std::copy' instead" or something along those lines?
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:107-108
@@ +106,4 @@
+
+ const auto Arg0Type = MatchedExpr->getArg(0)->IgnoreImpCasts()->getType();
+ const auto Arg1Type = MatchedExpr->getArg(1)->IgnoreImpCasts()->getType();
+
What about parens? e.g., memcpy((foo + 12), (bar), (a + b - 10));
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:118
@@ +117,3 @@
+
+ // Cannot do arithmetic on void pointer.
+ if (getStrippedType(Arg0Type)->isVoidType() ||
Same for function pointer types.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:137
@@ +136,3 @@
+ } else {
+// Rearrangement of arguments for memcpy
+// (dest, src, count) becomes (src, src + count, dest).
Missing a colon at the end of this line.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.h:1
@@ +1,2 @@
+//===--- UseAlgorithmCheck.h - clang-tidy-*- C++
-*-===//
+//
Line length does not match the closing ASCII art line length.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.h:20
@@ +19,3 @@
+
+/// Replaces memcpy with std::copy
+///
This is no longer accurate, and is also missing the full stop.
Comment at: docs/clang-tidy/checks/modernize-use-algorithm.rst:6
@@ +5,3 @@
+
+Replaces cal
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
klimek updated this revision to Diff 1.
klimek added a comment.
Fix bugs around marker replacements that neither insert nor delete anything.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,48 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
+TEST_F(ReplacementTest, FailAddRegression) {
+ Replacements Replaces;
+ // Create two replacements, where the second one is an insertion of the empty
+ // string exactly at the end of the first one.
+ auto Err = Replaces.add(Replacement("x.cc", 0, 10, "1"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+
+ // Make sure we find the overlap with the first entry when inserting a
+ // replacement that ends exactly at the seam of the existing replacements.
+ Err = Replaces.add(Replacement("x.cc", 5, 5, "fail"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
TEST_F(ReplacementTest, CanApplyReplacements) {
FileID ID = Context.createInMemoryFile("input.cpp",
"line1\nline2\nline3\nline4");
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -138,25 +138,55 @@
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
-for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
-return llvm::make_error(
-"All replacements must have the same file path. New replacement: " +
-R.getFilePath() + ", existing replacements: " +
-Replace.getFilePath() + "\n",
-llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength(
-return llvm::make_error(
-"New replacement:\n" + R.toString() +
-"\nconflicts with existing replacement:\n" + Replace.toString(),
-llvm::inconvertibleErrorCode());
-}
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+return llvm::make_error(
+"All replacements must have the same file path. New replacement: " +
+R.getFilePath() + ", existing replacements: " +
+Replaces.begin()->getFilePath() + "\n",
+llvm::inconvertibleErrorCode());
+
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
- Replaces.insert(R);
- return llvm::Error::success();
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+
+ // Find the first entry that starts after the end of R.
+ // We cannot use upper_bound for that, as there might an element equal to
+ // AtEnd in Replaces, and AtEnd does not overlap.
+ // Instead, we use lower_bound and special-case finding AtEnd below.
+ auto I = Replaces.lower_bound(AtEnd);
+ // If *I == R (which can only happen if R == AtEnd) the first entry that
+ // starts after R is (I+1).
+ if (I != Replaces.end() && *I == R)
+++I;
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ --I;
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
akuegel added a subscriber: akuegel. akuegel added a comment. lg Comment at: lib/Tooling/Core/Replacement.cpp:163 @@ +162,3 @@ + // Find the first entry that starts after the end of R. + // We cannot use upper_bound for that, as there might an element equal to + // AtEnd in Replaces, and AtEnd does not overlap. nit: "as there might an" -> "as there might be an". https://reviews.llvm.org/D23119 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277597 - Fix quadratic runtime when adding items to tooling::Replacements.
Author: klimek
Date: Wed Aug 3 09:12:17 2016
New Revision: 277597
URL: http://llvm.org/viewvc/llvm-project?rev=277597&view=rev
Log:
Fix quadratic runtime when adding items to tooling::Replacements.
Previously, we would search through all replacements when inserting a
new one to check for overlaps. Instead, make use of the fact that we
already have a set of replacments without overlaps to find the potential
overlap with lower_bound.
Differential Revision: https://reviews.llvm.org/D23119
Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=277597&r1=277596&r2=277597&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Aug 3 09:12:17 2016
@@ -138,25 +138,55 @@ void Replacement::setFromSourceRange(con
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
-for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
-return llvm::make_error(
-"All replacements must have the same file path. New replacement: "
+
-R.getFilePath() + ", existing replacements: " +
-Replace.getFilePath() + "\n",
-llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength(
-return llvm::make_error(
-"New replacement:\n" + R.toString() +
-"\nconflicts with existing replacement:\n" +
Replace.toString(),
-llvm::inconvertibleErrorCode());
-}
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+return llvm::make_error(
+"All replacements must have the same file path. New replacement: " +
+R.getFilePath() + ", existing replacements: " +
+Replaces.begin()->getFilePath() + "\n",
+llvm::inconvertibleErrorCode());
- Replaces.insert(R);
- return llvm::Error::success();
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the
current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+
+ // Find the first entry that starts after the end of R.
+ // We cannot use upper_bound for that, as there might be an element equal to
+ // AtEnd in Replaces, and AtEnd does not overlap.
+ // Instead, we use lower_bound and special-case finding AtEnd below.
+ auto I = Replaces.lower_bound(AtEnd);
+ // If *I == R (which can only happen if R == AtEnd) the first entry that
+ // starts after R is (I+1).
+ if (I != Replaces.end() && *I == R)
+++I;
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ --I;
+ // If the previous entry does not overlap, we know that entries before it
+ // can also not overlap.
+ if (R.getOffset() != I->getOffset() &&
+ !Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength( {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ return llvm::make_error(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=277597&r1=277596&r2=277597&view=diff
==
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Aug 3 09:12:17 2016
@@ -115,6 +115,48 @@ TEST_F(ReplacementTest, FailAddReplaceme
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::con
Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.
NoQ added a comment.
Nice catch!
Now, this needs a test. How about this one:
// enable the debug.ExprInspection checker?
void clang_analyzer_eval(int);
void test_asume_after_access(unsigned long x) {
char buf[100];
buf[x] = 1;
clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}}
}
By the way, if we replace `char` with `int`, this test fails even with your
patch. The reason is, the assumption is added on `(4 * x)` rather than on `x`,
and the constraint manager explodes. Does anybody volunteer to fix this (eg. on
the checker side - throw easier equations at the solver)?
https://reviews.llvm.org/D23112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277597: Fix quadratic runtime when adding items to
tooling::Replacements. (authored by klimek).
Changed prior to commit:
https://reviews.llvm.org/D23119?vs=1&id=3#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23119
Files:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -138,25 +138,55 @@
}
llvm::Error Replacements::add(const Replacement &R) {
- if (R.getOffset() != UINT_MAX)
-for (auto Replace : Replaces) {
- if (R.getFilePath() != Replace.getFilePath())
-return llvm::make_error(
-"All replacements must have the same file path. New replacement: " +
-R.getFilePath() + ", existing replacements: " +
-Replace.getFilePath() + "\n",
-llvm::inconvertibleErrorCode());
- if (R.getOffset() == Replace.getOffset() ||
- Range(R.getOffset(), R.getLength())
- .overlapsWith(Range(Replace.getOffset(), Replace.getLength(
-return llvm::make_error(
-"New replacement:\n" + R.toString() +
-"\nconflicts with existing replacement:\n" + Replace.toString(),
-llvm::inconvertibleErrorCode());
-}
+ // Check the file path.
+ if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
+return llvm::make_error(
+"All replacements must have the same file path. New replacement: " +
+R.getFilePath() + ", existing replacements: " +
+Replaces.begin()->getFilePath() + "\n",
+llvm::inconvertibleErrorCode());
+
+ // Special-case header insertions.
+ if (R.getOffset() == UINT_MAX) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
- Replaces.insert(R);
- return llvm::Error::success();
+ // This replacement cannot conflict with replacements that end before
+ // this replacement starts or start after this replacement ends.
+ // We also know that there currently are no overlapping replacements.
+ // Thus, we know that all replacements that start after the end of the current
+ // replacement cannot overlap.
+ Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
+
+ // Find the first entry that starts after the end of R.
+ // We cannot use upper_bound for that, as there might be an element equal to
+ // AtEnd in Replaces, and AtEnd does not overlap.
+ // Instead, we use lower_bound and special-case finding AtEnd below.
+ auto I = Replaces.lower_bound(AtEnd);
+ // If *I == R (which can only happen if R == AtEnd) the first entry that
+ // starts after R is (I+1).
+ if (I != Replaces.end() && *I == R)
+++I;
+ // I is the smallest iterator whose entry cannot overlap.
+ // If that is begin(), there are no overlaps.
+ if (I == Replaces.begin()) {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ --I;
+ // If the previous entry does not overlap, we know that entries before it
+ // can also not overlap.
+ if (R.getOffset() != I->getOffset() &&
+ !Range(R.getOffset(), R.getLength())
+ .overlapsWith(Range(I->getOffset(), I->getLength( {
+Replaces.insert(R);
+return llvm::Error::success();
+ }
+ return llvm::make_error(
+ "New replacement:\n" + R.toString() +
+ "\nconflicts with existing replacement:\n" + I->toString(),
+ llvm::inconvertibleErrorCode());
}
namespace {
Index: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
===
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp
@@ -115,6 +115,48 @@
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+ Replacements Replaces;
+ // Test adding an insertion at the offset of an existing replacement.
+ auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+
+ Replaces.clear();
+ // Test overlap with an existing insertion.
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
+TEST_F(ReplacementTest, FailAddRegression) {
+ Replacements Replaces;
+ // Create two replacements, where the second one is an insertion of the empty
+ // string exactly at the end of the first one.
+ auto Err = Replaces.add(Replacement("x.cc", 0, 10, "1"));
+
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
ioeric added inline comments. Comment at: cfe/trunk/lib/Tooling/Core/Replacement.cpp:169 @@ +168,3 @@ + // starts after R is (I+1). + if (I != Replaces.end() && *I == R) +++I; I think we should ignore replacement text when checking equality between `*I` and `R` here. Repository: rL LLVM https://reviews.llvm.org/D23119 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23086: [OpenCL] Generate concrete struct type for ndrange_t
yaxunl added inline comments. Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:43 @@ +42,3 @@ + + return llvm::StructType::create(EleTypes, "ndrange_t"); +} yaxunl wrote: > struct name should be "struct.ndrange_t" to allow library code to access it. Sorry, should be "struct.__ndrange_t" to avoid conflict with builtin type ndrange_t. Repository: rL LLVM https://reviews.llvm.org/D23086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23119: Fix quadratic runtime when adding items to tooling::Replacements.
klimek added inline comments. Comment at: cfe/trunk/lib/Tooling/Core/Replacement.cpp:169 @@ +168,3 @@ + // starts after R is (I+1). + if (I != Replaces.end() && *I == R) +++I; ioeric wrote: > I think we should ignore replacement text when checking equality between `*I` > and `R` here. lower_bound already compares the replacement text, so as the comment says, unless I'm mistaken, this can only happen if R == AtEnd, which means that R has an empty replacement text. Can you come up with a test that would break? Repository: rL LLVM https://reviews.llvm.org/D23119 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.
NoQ accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/Analysis/CloneDetection.cpp:99
@@ +98,3 @@
+ /// Every item in this list is unique.
+ std::vector Variables;
+
NoQ wrote:
> I've a feeling this is essentially a `map`, because
> that harmonizes with our access patterns (lookup number by variable, insert
> the new value `Variables.size()` if not found). So i think a map would
> express the intention more clearly.
Discussed that we're going to have lookups in both directions in the next patch.
Comment at: lib/Analysis/CloneDetection.cpp:168
@@ +167,3 @@
+assert(Other.Occurences.size() == Occurences.size());
+for (unsigned i = 0; i < Occurences.size(); ++i) {
+ if (Occurences[i].KindID != Other.Occurences[i].KindID) {
NoQ wrote:
> Because your code uses a lot of fancy C++11, i'd probably suggest to make it
> even prettier with `std::all_of()`.
Uhm, never mind, iterating two arrays here.
https://reviews.llvm.org/D22982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors
baloghadamsoftware added a comment. I agree with you. Do I have to modify the checker (in a separate patch), or someone else can do it? I do not know how difficult it is to unpack the store of a LazyCompoundVal (it probably has to be done recursively). https://reviews.llvm.org/D22374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.
v.g.vassilev accepted this revision. v.g.vassilev added a comment. LGTM. My comments were clarified in a Skype chat. https://reviews.llvm.org/D22982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277603 - Fix bug in conflict check for Replacements::add().
Author: klimek
Date: Wed Aug 3 10:12:00 2016
New Revision: 277603
URL: http://llvm.org/viewvc/llvm-project?rev=277603&view=rev
Log:
Fix bug in conflict check for Replacements::add().
We would not detect conflicts when inserting insertions at the same
offset as previously contained replacements.
Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=277603&r1=277602&r2=277603&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Aug 3 10:12:00 2016
@@ -159,15 +159,16 @@ llvm::Error Replacements::add(const Repl
// replacement cannot overlap.
Replacement AtEnd(R.getFilePath(), R.getOffset() + R.getLength(), 0, "");
- // Find the first entry that starts after the end of R.
- // We cannot use upper_bound for that, as there might be an element equal to
- // AtEnd in Replaces, and AtEnd does not overlap.
- // Instead, we use lower_bound and special-case finding AtEnd below.
+ // Find the first entry that starts after or at the end of R. Note that
+ // entries that start at the end can still be conflicting if R is an
+ // insertion.
auto I = Replaces.lower_bound(AtEnd);
- // If *I == R (which can only happen if R == AtEnd) the first entry that
- // starts after R is (I+1).
- if (I != Replaces.end() && *I == R)
+ // If it starts at the same offset as R (can only happen if R is an
+ // insertion), we have a conflict. In that case, increase I to fall through
+ // to the conflict check.
+ if (I != Replaces.end() && R.getOffset() == I->getOffset())
++I;
+
// I is the smallest iterator whose entry cannot overlap.
// If that is begin(), there are no overlaps.
if (I == Replaces.begin()) {
Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=277603&r1=277602&r2=277603&view=diff
==
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Aug 3 10:12:00 2016
@@ -157,6 +157,26 @@ TEST_F(ReplacementTest, FailAddRegressio
llvm::consumeError(std::move(Err));
}
+TEST_F(ReplacementTest, FailAddInsertAtOffsetOfReplacement) {
+ Replacements Replaces;
+ auto Err = Replaces.add(Replacement("x.cc", 10, 2, ""));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
+TEST_F(ReplacementTest, FailAddInsertAtOtherInsert) {
+ Replacements Replaces;
+ auto Err = Replaces.add(Replacement("x.cc", 10, 0, "a"));
+ EXPECT_TRUE(!Err);
+ llvm::consumeError(std::move(Err));
+ Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
+ EXPECT_TRUE((bool)Err);
+ llvm::consumeError(std::move(Err));
+}
+
TEST_F(ReplacementTest, CanApplyReplacements) {
FileID ID = Context.createInMemoryFile("input.cpp",
"line1\nline2\nline3\nline4");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23060: [analyzer] Show enabled checker list
NoQ added a comment.
Like!
Do you think that if we rename the ~~bike shed~~ option to
`-analyzer-list-enabled-checkers` it'd be more natural and easy to remember? (i
barely remember how to spell `-analyzer-checker-help`, it'd be much easier if
it was called `-analyzer-list-checkers`).
Comment at: test/Analysis/analyzer-enabled-checkers.c:5
@@ +4,3 @@
+void bar() {}
+void foo() {
+ // Call bar 33 times so max-times-inline-large is met and
This part of the test seems to have made it here accidentally.
https://reviews.llvm.org/D23060
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
omtcyfz updated this revision to Diff 66670.
omtcyfz added a comment.
Merged multiple identical tests into a single test with multiple `clang-rename`
invocations.
https://reviews.llvm.org/D23058
Files:
clang-rename/USRFinder.cpp
clang-rename/USRFindingAction.cpp
test/clang-rename/ComplexFunctionOverride.cpp
test/clang-rename/TemplateClassInstantiation.cpp
test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
Index: test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=703 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
-template
-class Foo { // CHECK: class Bar;
-public:
- T foo(T arg, T& ref, T* ptr) {
-T value;
-int number = 42;
-value = (T)number;
-value = static_cast(number);
-return value;
- }
- static void foo(T value) {}
- T member;
-};
-
-template
-void func() {
- Foo obj; // CHECK: Bar obj;
- obj.member = T();
- Foo::foo(); // CHECK: Bar::foo();
-}
-
-int main() {
- Foo i; // CHECK: Bar i;
- i.member = 0;
- Foo::foo(0); // CHECK: Bar::foo(0);
-
- Foo b;// CHECK: Bar b;
- b.member = false;
- Foo::foo(false); // CHECK: Bar::foo(false);
-
- return 0;
-}
-
-// Use grep -FUbo 'Foo' to get the correct offset of foo when changing
-// this file.
Index: test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
===
--- test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
-template
-class Foo { // CHECK: class Bar;
-public:
- T foo(T arg, T& ref, T* ptr) {
-T value;
-int number = 42;
-value = (T)number;
-value = static_cast(number);
-return value;
- }
- static void foo(T value) {}
- T member;
-};
-
-template
-void func() {
- Foo obj; // CHECK: Bar obj;
- obj.member = T();
- Foo::foo(); // CHECK: Bar::foo();
-}
-
-int main() {
- Foo i; // CHECK: Bar i;
- i.member = 0;
- Foo::foo(0); // CHECK: Bar::foo(0);
-
- Foo b;// CHECK: Bar b;
- b.member = false;
- Foo::foo(false); // CHECK: Bar::foo(false);
-
- return 0;
-}
-
-// Use grep -FUbo 'C' to get the correct offset of foo when changing
-// this file.
Index: test/clang-rename/TemplateClassInstantiation.cpp
===
--- /dev/null
+++ test/clang-rename/TemplateClassInstantiation.cpp
@@ -0,0 +1,44 @@
+template
+class Foo { // CHECK: class Bar {
+public:
+ T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+ }
+ static void foo(T value) {}
+ T member;
+};
+
+template
+void func() {
+ Foo obj; // CHECK: Bar obj;
+ obj.member = T();
+ Foo::foo(); // CHECK: Bar::foo();
+}
+
+int main() {
+ Foo i; // CHECK: Bar i;
+ i.member = 0;
+ Foo::foo(0); // CHECK: Bar::foo(0);
+
+ Foo b;// CHECK: Bar b;
+ b.member = false;
+ Foo::foo(false); // CHECK: Bar::foo(false);
+
+ return 0;
+}
+
+// RUN: cat %s > %t-0.cpp
+// RUN: clang-rename -offset=29 -new-name=Bar %t-0.cpp -i -- -fno-delayed-template-parsing
+// RUN: sed 's,//.*,,' %t-0.cpp | FileCheck %s
+
+// RUN: cat %s > %t-1.cpp
+// RUN: clang-rename -offset=311 -new-name=Bar %t-1.cpp -i -- -fno-delayed-template-parsing
+// RUN: sed 's,//.*,,' %t-1.cpp | FileCheck %s
+
+// RUN: cat %s > %t-2.cpp
+// RUN: clang-rename -offset=445 -new-name=Bar %t-2.cpp -i -- -fno-delayed-template-parsing
+// RUN: sed 's,//.*,,' %t-2.cpp | FileCheck %s
Index: test/clang-rename/ComplexFunctionOverride.cpp
===
--- /dev/null
+++ test/clang-rename/ComplexFunctionOverride.cpp
@@ -0,0 +1,23 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=307 -new-name=bar %t.cpp -i -- -std=c++11
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+struct A {
+ virtual void foo(); // CHECK: virtual void bar();
+};
+
+struct B : A {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct C : B {
+ void foo() override; // CHECK: void bar() override;
+};
+
+struct D : B {
+ void foo() override; // CHECK: v
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
omtcyfz marked 2 inline comments as done. Comment at: test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp:3 @@ +2,3 @@ +// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i -- +// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s + alexfh wrote: > Yep, should also work with the same check prefix, if the replacements are the > same. Done. https://reviews.llvm.org/D23058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23105: [clang/test] Fix a flaky unittest on windows
rnk added a comment. Thanks! lgtm Repository: rL LLVM https://reviews.llvm.org/D23105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23060: [analyzer] Show enabled checker list
xazax.hun added a comment. In https://reviews.llvm.org/D23060#504832, @NoQ wrote: > Like! > > Do you think that if we rename the ~~bike shed~~ option to > `-analyzer-list-enabled-checkers` it'd be more natural and easy to remember? > (i barely remember how to spell `-analyzer-checker-help`, it'd be much easier > if it was called `-analyzer-list-checkers`). I am absolutely in favor of a more natural name! :) I wanted to comply with existing flags, but maybe making it easier to remember is a better idea. https://reviews.llvm.org/D23060 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors
NoQ added a comment. Hmm. I suggest: 1. Change this test's constructor so that it was no longer almost-trivial. Because it isn't significant for this test if the constructor is almost-trivial or not. The test would pass. 2. Add a FIXME-test for this checker, in which a completely undefined structure is being copied. Perhaps somebody implements this feature some day. 3. Leave out the situation that the current test checks as a grey-area. I'm still not convinced that this situation (copying a partially-initialized almost-trivial structure) deserves a warning from this checker. Does this make any sense? :) https://reviews.llvm.org/D22374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14274: Add alloc_size attribute to clang
ahatanak added a subscriber: ahatanak.
ahatanak added a comment.
I'm wondering what the status of this patch is since someone has asked us to
add support for this attribute in clang. Are you still waiting for review?
Comment at: test/CodeGenCXX/alloc-size.cpp:66
@@ +65,3 @@
+ // CHECK: ret i32 122
+ return __builtin_object_size(my_malloc(), 0) +
+ __builtin_object_size(my_calloc(5), 0) +
Is it necessary to compute __builtin_object_size in the front-end (rather than
in some IR passes like instcombine) when it takes the pointer returned by a
function marked alloc_size?
Also, is the IR optimization smart enough to get the exact object size in the
following case?
```
void foo(int a, int b) {
void *p0 = my_malloc(a);
g0 = __builtin_object_size(p0, 0);
void *p1 = my_calloc(a, b);
g1 = __builtin_object_size(p1, 1);
}
void foo1() {
foo(10, 50);
}
```
https://reviews.llvm.org/D14274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21453: Add support for attribute "overallocated"
ahatanak added a comment. ping https://reviews.llvm.org/D21453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map
manmanren created this revision.
manmanren added a reviewer: benlangmuir.
manmanren added a subscriber: cfe-commits.
We add -fmodules-use-prebuilt-modules to support using prebuilt modules. In
this mode, there is no need to load any module map and the programmer can
simply use "@import" syntax to load the module directly from module cache
without parsing any module map. We don't support rebuilding of the modules and
ignore configuration mismatches.
https://reviews.llvm.org/D23125
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Options.td
include/clang/Lex/HeaderSearchOptions.h
lib/Driver/Tools.cpp
lib/Frontend/CompilerInstance.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTReader.cpp
test/Driver/modules.m
test/Modules/Inputs/prebuilt-module/a.h
test/Modules/Inputs/prebuilt-module/module.modulemap
test/Modules/prebuilt-module.m
Index: test/Modules/prebuilt-module.m
===
--- test/Modules/prebuilt-module.m
+++ test/Modules/prebuilt-module.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+//
+// RUN: %clang_cc1 -fmodules -x objective-c -I %S/Inputs/prebuilt-module -triple %itanium_abi_triple -emit-module %S/Inputs/prebuilt-module/module.modulemap -fmodule-name=prebuilt -o %t/prebuilt.pcm
+// RUN: %clang_cc1 -fmodules -fmodules-use-prebuilt-modules -fdisable-module-hash -fmodules-cache-path=%t/ %s -verify
+
+// expected-no-diagnostics
+@import prebuilt;
+int test() {
+ return a;
+}
Index: test/Modules/Inputs/prebuilt-module/module.modulemap
===
--- test/Modules/Inputs/prebuilt-module/module.modulemap
+++ test/Modules/Inputs/prebuilt-module/module.modulemap
@@ -0,0 +1 @@
+module prebuilt { header "a.h" }
Index: test/Modules/Inputs/prebuilt-module/a.h
===
--- test/Modules/Inputs/prebuilt-module/a.h
+++ test/Modules/Inputs/prebuilt-module/a.h
@@ -0,0 +1 @@
+const int a = 1;
Index: test/Driver/modules.m
===
--- test/Driver/modules.m
+++ test/Driver/modules.m
@@ -39,6 +39,16 @@
// RUN: %clang -fmodules-disable-diagnostic-validation -### %s 2>&1 | FileCheck -check-prefix=MODULES_DISABLE_DIAGNOSTIC_VALIDATION %s
// MODULES_DISABLE_DIAGNOSTIC_VALIDATION: -fmodules-disable-diagnostic-validation
+// RUN: %clang -fmodules -### %s 2>&1 | FileCheck -check-prefix=MODULES_USE_PREBUILT_MODULES_DEFAULT %s
+// MODULES_USE_PREBUILT_MODULES_DEFAULT-NOT: -fmodules-use-prebuilt-modules
+
+// RUN: %clang -fmodules -fmodules-use-prebuilt-modules -### %s 2>&1 | FileCheck -check-prefix=MODULES_USE_PREBUILT_MODULES %s
+// MODULES_USE_PREBUILT_MODULES: "-fdisable-module-hash"
+// MODULES_USE_PREBUILT_MODULES: "-fmodules-use-prebuilt-modules"
+
+// RUN: %clang -fmodules -fmodules-use-prebuilt-modules -fimplicit-module-maps -### %s 2>&1 | FileCheck -check-prefix=MODULES_USE_PREBUILT_MODULES_ERR %s
+// MODULES_USE_PREBUILT_MODULES_ERR: option '-fmodules-use-prebuilt-modules' conflicts with '-fimplicit-module-maps'
+
// RUN: %clang -fmodules -fmodule-map-file=foo.map -fmodule-map-file=bar.map -### %s 2>&1 | FileCheck -check-prefix=CHECK-MODULE-MAP-FILES %s
// CHECK-MODULE-MAP-FILES: "-fmodules"
// CHECK-MODULE-MAP-FILES: "-fmodule-map-file=foo.map"
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -492,6 +492,11 @@
}
}
+static bool isPrebuiltModule(Preprocessor &PP, ModuleKind MK) {
+ return (MK == MK_ExplicitModule || MK == MK_ImplicitModule) &&
+ PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesUsePrebuiltModules;
+}
+
/// \brief Check the preprocessor options deserialized from the control block
/// against the preprocessor options in an existing preprocessor.
///
@@ -2194,7 +2199,8 @@
// All user input files reside at the index range [0, NumUserInputs), and
// system input files reside at [NumUserInputs, NumInputs). For explicitly
// loaded module files, ignore missing inputs.
- if (!DisableValidation && F.Kind != MK_ExplicitModule) {
+ if (!DisableValidation && F.Kind != MK_ExplicitModule &&
+ !isPrebuiltModule(PP, F.Kind)) {
bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0;
// If we are reading a module, we will create a verification timestamp,
@@ -2225,7 +2231,8 @@
bool IsSystem = I >= NumUserInputs;
InputFileInfo FI = readInputFileInfo(F, I+1);
Listener->visitInputFile(FI.Filename, IsSystem, FI.Overridden,
- F.Kind == MK_ExplicitModule);
+ F.Kind == MK_ExplicitModule ||
+ isPrebuiltModule(PP, F.Kind));
}
}
@@ -2255,7 +2262,7 @@
//
Re: [PATCH] D14274: Add alloc_size attribute to clang
george.burgess.iv added a comment.
> I'm wondering what the status of this patch is since someone has asked us to
> add support for this attribute in clang. Are you still waiting for review?
Waiting for an LGTM from Richard, though this patch currently doesn't pass all
tests. One of the changes in it causes some craziness with how we codegen
blocks (in e.g. objc/OpenCL). Basically, we'll end up emitting the same block
definition N times, since this makes the constexpr evaluator more accurate in
some cases. I hope to fix that + get back to this in the nearish future.
(Happy to accept an LGTM now, though, since the bug isn't with this patch. ;)
Naturally, I'll submit after the block-related bug is fixed.)
Comment at: test/CodeGenCXX/alloc-size.cpp:66
@@ +65,3 @@
+ // CHECK: ret i32 122
+ return __builtin_object_size(my_malloc(), 0) +
+ __builtin_object_size(my_calloc(5), 0) +
ahatanak wrote:
> Is it necessary to compute __builtin_object_size in the front-end (rather
> than in some IR passes like instcombine) when it takes the pointer returned
> by a function marked alloc_size?
>
> Also, is the IR optimization smart enough to get the exact object size in the
> following case?
>
> ```
> void foo(int a, int b) {
> void *p0 = my_malloc(a);
> g0 = __builtin_object_size(p0, 0);
> void *p1 = my_calloc(a, b);
> g1 = __builtin_object_size(p1, 1);
> }
>
> void foo1() {
> foo(10, 50);
> }
> ```
> Is it necessary to compute __builtin_object_size in the front-end (rather
> than in some IR passes like instcombine)
It's necessary to try in the frontend. If that fails, we'll lower calls to
__bos to `@llvm.objectsize`, and let LLVM try it.
The main reason that we care for the frontend is, among other reasons, so we
can use it with the `enable_if` attribute, like so:
```
void strncat(char *buf, int n, const char *from)
__attribute__((overloadable,
enable_if(__builtin_object_size(buf, 0) < n, ""),
unavailable("'n' is larger than the target buffer!")));
void strncat(char *buf, int n, const char *from)
__attribute__((overloadable));
int main() {
char buf[4];
strncat(buf, sizeof(buf)+1, "hi"); // expected-error{{'n' is larger than the
target buffer!}}
}
```
> Also, is the IR optimization smart enough to get the exact object size in the
> following case?
`__builtin_object_size` *always* gets lowered to a constant by either clang or
LLVM. That is, if `foo` gets inlined into `foo1`, then LLVM should be able to
determine accurate values for `g0` and `g1`. If `foo` isn't inlined, then no:
you'll get -1 for both.
If you'd like to see what we can and can't get, LLVM already has the
`allocsize` attribute (and `@llvm.objectsize` intrinsic) in it. :)
https://reviews.llvm.org/D14274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23096: [Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope
sepavloff added inline comments.
Comment at: test/SemaTemplate/default-expr-arguments-3.cpp:20
@@ +19,2 @@
+ }
+}
Please add the following test to the patch:
```
template void f1() {
enum class foo { a, b };
struct S {
int g1(foo n = foo::a);
};
}
template void f1();
```
It must also compile correctly.
https://reviews.llvm.org/D23096
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map
manmanren added inline comments.
Comment at: lib/Driver/Tools.cpp:5416
@@ -5408,1 +5415,3 @@
+ if (Args.getLastArg(options::OPT_fmodules_use_prebuilt_modules)) {
+// When using prebuilt modules, we disable module hash.
benlangmuir wrote:
> ```
> if (IsPrebuilt) {
> ```
Will do.
Comment at: lib/Frontend/CompilerInstance.cpp:1491
@@ +1490,3 @@
+false/*IsExplicit*/).first;
+Module->IsSystem = true;
+Module->IsFromModuleFile = true;
benlangmuir wrote:
> Why do we assume this?
Thanks for reviewing so quickly!
I was not sure about what value we should set IsSystem. Since we are mostly
prebuilding system modules, I assumed "true". Do you have any suggestion here?
https://reviews.llvm.org/D23125
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map
benlangmuir added a comment.
How about -fmodules-use-prebuilt-**module-cache** for the flag name? Saying
"prebuilt-modules" is confusing to me, since -fmodule-file can also be used to
load a prebuilt module, but doesn't use a cache.
Comment at: lib/Driver/Tools.cpp:5416
@@ -5408,1 +5415,3 @@
+ if (Args.getLastArg(options::OPT_fmodules_use_prebuilt_modules)) {
+// When using prebuilt modules, we disable module hash.
```
if (IsPrebuilt) {
```
Comment at: lib/Frontend/CompilerInstance.cpp:1491
@@ +1490,3 @@
+false/*IsExplicit*/).first;
+Module->IsSystem = true;
+Module->IsFromModuleFile = true;
Why do we assume this?
https://reviews.llvm.org/D23125
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r277623 - [docs] fix typo in clang-rename docs
Author: omtcyfz Date: Wed Aug 3 13:15:07 2016 New Revision: 277623 URL: http://llvm.org/viewvc/llvm-project?rev=277623&view=rev Log: [docs] fix typo in clang-rename docs clang-rename is a refactoring tool, not "linter" tool. Fix typo in docs. Modified: clang-tools-extra/trunk/docs/clang-rename.rst Modified: clang-tools-extra/trunk/docs/clang-rename.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-rename.rst?rev=277623&r1=277622&r2=277623&view=diff == --- clang-tools-extra/trunk/docs/clang-rename.rst (original) +++ clang-tools-extra/trunk/docs/clang-rename.rst Wed Aug 3 13:15:07 2016 @@ -10,9 +10,9 @@ See also: :maxdepth: 1 -:program:`clang-rename` is a clang-based C++ "linter" tool. Its purpose is to -perform efficient renaming actions in large-scale projects such as renaming -classes, functions, variables, arguments, namespaces etc. +:program:`clang-rename` is a C++ refactoring tool. Its purpose is to perform +efficient renaming actions in large-scale projects such as renaming classes, +functions, variables, arguments, namespaces etc. The tool is in a very early development stage, so you might encounter bugs and crashes. Submitting reports with information about how to reproduce the issue ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23096: [Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope
ahatanak added a comment.
OK, I'll add the test.
When I apply this patch and compile your test, I see DependentScopeDeclRefExpr
gets converted to DeclRefExpr when the function is instantiated. This doesn't
happen without this patch, but somehow clang still manages to emit the same
code.
Also, Erik Pilkington pointed out that clang still crashes when it compiles the
following code. I'm trying to figure out why that happens.
// Template variable case:
enum class foo { a, b };
template auto a = [](foo f = foo::a) { return f; }();
auto foo1() {
return a;
}
// Template struct case:
template struct func {
void bar() {
enum class foo { a, b };
[](foo f = foo::a) { return f; }();
}
};
template struct func;
https://reviews.llvm.org/D23096
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
JDevlieghere updated this revision to Diff 66688.
JDevlieghere marked 21 inline comments as done.
JDevlieghere added a comment.
Addresses comments from Aaron Ballman
@aaron.ballman Thanks for the thorough review! Can you check whether the tests
I added address your concerns? Could you also elaborate on the case with the
C-function pointer? Unless I explicitly cast it to void* the compiler rejects
will reject it as an argument to memcpy. Am I missing a case where this could
go wrong? I still added it to the pointer arithmetic check though, just to be
sure.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseAlgorithmCheck.cpp
clang-tidy/modernize/UseAlgorithmCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-algorithm.rst
test/clang-tidy/modernize-use-algorithm.cpp
Index: test/clang-tidy/modernize-use-algorithm.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-algorithm.cpp
@@ -0,0 +1,125 @@
+// RUN: %check_clang_tidy %s modernize-use-algorithm %t
+// CHECK-FIXES: #include
+
+namespace std {
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *src, std::size_t count);
+void *memset(void *dest, int ch, std::size_t count);
+
+template
+OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
+
+template
+OutputIt move(InputIt first, InputIt last, OutputIt d_first);
+
+template
+void fill(ForwardIt first, ForwardIt last, const T &value);
+}
+
+namespace awful {
+void memcpy(int, int, int);
+}
+
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *source, size_t size);
+
+void a() {
+ char foo[] = "foo", bar[4];
+ std::memcpy(bar, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+ void *baz = bar;
+ std::memcpy(baz, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+
+ memcpy(bar, foo, sizeof bar);
+ std::copy(foo, foo + sizeof bar, bar);
+}
+
+void b() {
+ char foo[] = "foobar";
+ std::memset(foo, 'a', 3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::fill(foo, foo + 3, 'a');
+
+ std::memset(foo, 1, 3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+
+ std::fill(foo, foo + 2, 'a');
+}
+
+#define MEMCPY(dest, src) std::memcpy((dest), (src), sizeof(dest))
+void c() {
+ char foo[] = "foo", bar[3];
+ MEMCPY(bar, foo);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+}
+
+void d() {
+ typedef char foo_t;
+ typedef char bar_t;
+ foo_t foo[] = "foo";
+ bar_t bar[4];
+ std::memcpy(bar, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void e() {
+ typedef const char *foo_t;
+ typedef const char *bar_t;
+ foo_t foo[] = {"foo", "bar"};
+ bar_t bar[2];
+ std::memcpy(bar, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void f() {
+ typedef int some_type;
+ typedef some_type *const *volatile *foo_ptr;
+
+ typedef int *const some_other_type;
+ typedef some_other_type *volatile *bar_ptr;
+
+ foo_ptr foo[4];
+ bar_ptr bar[3];
+ std::memcpy(bar, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void g() {
+ int foo[3][4] = {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 10, 11}};
+ int bar[2][4];
+
+ std::memcpy(bar, foo, sizeof bar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+ // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+ int baz[3][3] = {{0, 1, 2}, {3, 4, 5}, {6, 7, 8}};
+ int qux[2][4];
+ std::memcpy(qux, baz, sizeof qux);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+}
+
+void h() {
+ int a = 1;
+ int b = 2;
+ int c = 3;
+
+ char foo[] = "foobar";
+ char bar[3];
+
+ std::memcpy((bar), (foo + b), (a + c - 1));
+ // CHECK-MESSAGES: :[[@LINE-
[PATCH] D23130: Add a check for definitions in the global namespace.
bkramer created this revision.
bkramer added a reviewer: alexfh.
bkramer added a subscriber: cfe-commits.
This is prone to ODR violations and generally frowned upon in many
codebases (e.g. LLVM). The checker flags definitions, variables and
classes in the global namespace. Common false positives like extern "C"
functions are filtered, symbols with internal linkage or hidden
visibility are not warned on either.
On LLVM most of the instances are helper functions that should be just
made static or globals that belong into a namespace.
https://reviews.llvm.org/D23130
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/GlobalNamespaceCheck.cpp
clang-tidy/misc/GlobalNamespaceCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-global-namespace.rst
test/clang-tidy/misc-global-namespace.cpp
Index: test/clang-tidy/misc-global-namespace.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-global-namespace.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s misc-global-namespace %t
+
+void f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: definition for 'f' in the global
+// namespace
+class F {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: definition for 'F' in the global
+// namespace
+int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: definition for 'i' in the global
+// namespace
+
+// No warnings below.
+void g();
+
+extern "C" void h() {}
+
+extern "C" {
+void j() {}
+}
+
+struct Clike {
+ int i;
+};
+
+void *operator new(__SIZE_TYPE__, int) { return 0; }
+void operator delete(void *, int) {}
+
+__attribute__((visibility("hidden"))) void k() {}
+static void l() {}
+namespace {
+void m() {}
+}
+namespace x {
+void n() {}
+}
+
+int main() {}
Index: docs/clang-tidy/checks/misc-global-namespace.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-global-namespace.rst
@@ -0,0 +1,7 @@
+.. title:: clang-tidy - misc-global-namespace
+
+misc-global-namespace
+=
+
+Finds definitions in the global namespace. Those definitions are prone to ODR
+conflicts.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -59,6 +59,7 @@
misc-definitions-in-headers
misc-fold-init-type
misc-forward-declaration-namespace
+ misc-global-namespace
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,18 +12,18 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
-#include "MisplacedConstCheck.h"
-#include "UnconventionalAssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "DanglingHandleCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
+#include "GlobalNamespaceCheck.h"
#include "InaccurateEraseCheck.h"
#include "IncorrectRoundings.h"
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
+#include "MisplacedConstCheck.h"
#include "MisplacedWideningCastCheck.h"
#include "MoveConstantArgumentCheck.h"
#include "MoveConstructorInitCheck.h"
@@ -44,6 +44,7 @@
#include "SuspiciousStringCompareCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
+#include "UnconventionalAssignOperatorCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
@@ -62,6 +63,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-global-namespace");
CheckFactories.registerCheck(
"misc-misplaced-const");
CheckFactories.registerCheck(
Index: clang-tidy/misc/GlobalNamespaceCheck.h
===
--- /dev/null
+++ clang-tidy/misc/GlobalNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- GlobalNamespaceCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds definitions in the global name
Re: [PATCH] D14274: Add alloc_size attribute to clang
ahatanak added inline comments.
Comment at: test/CodeGenCXX/alloc-size.cpp:66
@@ +65,3 @@
+ // CHECK: ret i32 122
+ return __builtin_object_size(my_malloc(), 0) +
+ __builtin_object_size(my_calloc(5), 0) +
george.burgess.iv wrote:
> ahatanak wrote:
> > Is it necessary to compute __builtin_object_size in the front-end (rather
> > than in some IR passes like instcombine) when it takes the pointer returned
> > by a function marked alloc_size?
> >
> > Also, is the IR optimization smart enough to get the exact object size in
> > the following case?
> >
> > ```
> > void foo(int a, int b) {
> > void *p0 = my_malloc(a);
> > g0 = __builtin_object_size(p0, 0);
> > void *p1 = my_calloc(a, b);
> > g1 = __builtin_object_size(p1, 1);
> > }
> >
> > void foo1() {
> > foo(10, 50);
> > }
> > ```
> > Is it necessary to compute __builtin_object_size in the front-end (rather
> > than in some IR passes like instcombine)
>
> It's necessary to try in the frontend. If that fails, we'll lower calls to
> __bos to `@llvm.objectsize`, and let LLVM try it.
>
> The main reason that we care for the frontend is, among other reasons, so we
> can use it with the `enable_if` attribute, like so:
>
> ```
> void strncat(char *buf, int n, const char *from)
> __attribute__((overloadable,
> enable_if(__builtin_object_size(buf, 0) < n, ""),
> unavailable("'n' is larger than the target buffer!")));
>
> void strncat(char *buf, int n, const char *from)
> __attribute__((overloadable));
>
> int main() {
> char buf[4];
> strncat(buf, sizeof(buf)+1, "hi"); // expected-error{{'n' is larger than
> the target buffer!}}
> }
> ```
>
>
>
>
> > Also, is the IR optimization smart enough to get the exact object size in
> > the following case?
>
> `__builtin_object_size` *always* gets lowered to a constant by either clang
> or LLVM. That is, if `foo` gets inlined into `foo1`, then LLVM should be able
> to determine accurate values for `g0` and `g1`. If `foo` isn't inlined, then
> no: you'll get -1 for both.
>
> If you'd like to see what we can and can't get, LLVM already has the
> `allocsize` attribute (and `@llvm.objectsize` intrinsic) in it. :)
Thanks for the explanation. I see why you want to evaluate it in the front-end.
It looks like llvm optimizes function foo before it gets inlined into function
foo1 and, as a result, g0 and g1 both get -1 instead of 10 and 500. I guess
llvm should defer optimizing the function until after inliner is run.
https://reviews.llvm.org/D14274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: Add a check for definitions in the global namespace.
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). https://reviews.llvm.org/D23130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23041: Un-XFAIL GCC atomics.align
jfb added a comment. @EricWF ran this on configuration `cxx_under_test=g++-4.9`. It generates the following error: libcxx/test/libcxx/atomics/atomics.align/align.pass.sh.cpp:32: atomic_test::atomic_test() [with T = std::nullptr_t]: Assertion `alignof(this->__a_) >= sizeof(this->__a_) && "expected natural alignment for lock-free type"' failed. I think GCC is broken. See the following (semi-relevant) test: https://godbolt.org/g/pFUqVY That's not quite what I'm testing (this used libstdc++ and looks at `std::atomic`, not the value contained), but I'd nonetheless expect the `std::atomic` container to be sufficiently aligned. WDYT? Is that only happening for `nullptr_t`? I can conditionally disable the `nullptr_t` test for GCC, and file a bug on GCC. https://reviews.llvm.org/D23041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277647 - [OpenCL] Fix size of image type
Author: yaxunl
Date: Wed Aug 3 15:38:06 2016
New Revision: 277647
URL: http://llvm.org/viewvc/llvm-project?rev=277647&view=rev
Log:
[OpenCL] Fix size of image type
The size of image type is reported incorrectly as size of a pointer to address
space 0, which causes error when casting image type to pointers by
__builtin_astype.
The fix is to get image address space from TargetInfo then report the size
accordingly.
Differential Revision: https://reviews.llvm.org/D22927
Modified:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=277647&r1=277646&r2=277647&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Wed Aug 3 15:38:06 2016
@@ -986,6 +986,11 @@ public:
return getTargetOpts().SupportedOpenCLOptions;
}
+ /// \brief Get OpenCL image type address space.
+ virtual LangAS::ID getOpenCLImageAddrSpace() const {
+return LangAS::opencl_global;
+ }
+
/// \brief Check the target is valid after it is fully initialized.
virtual bool validateTarget(DiagnosticsEngine &Diags) const {
return true;
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=277647&r1=277646&r2=277647&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Aug 3 15:38:06 2016
@@ -1750,14 +1750,18 @@ TypeInfo ASTContext::getTypeInfoImpl(con
case BuiltinType::OCLQueue:
case BuiltinType::OCLNDRange:
case BuiltinType::OCLReserveID:
-#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
-case BuiltinType::Id:
-#include "clang/Basic/OpenCLImageTypes.def"
-
// Currently these types are pointers to opaque types.
Width = Target->getPointerWidth(0);
Align = Target->getPointerAlign(0);
break;
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
+case BuiltinType::Id:
+#include "clang/Basic/OpenCLImageTypes.def"
+ {
+auto AS = getTargetAddressSpace(Target->getOpenCLImageAddrSpace());
+Width = Target->getPointerWidth(AS);
+Align = Target->getPointerAlign(AS);
+ }
}
break;
case Type::ObjCObjectPointer:
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=277647&r1=277646&r2=277647&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Wed Aug 3 15:38:06 2016
@@ -2131,6 +2131,10 @@ public:
}
}
+ LangAS::ID getOpenCLImageAddrSpace() const override {
+return LangAS::opencl_constant;
+ }
+
CallingConvCheckResult checkCallingConvention(CallingConv CC) const override
{
switch (CC) {
default:
Modified: cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp?rev=277647&r1=277646&r2=277647&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp Wed Aug 3 15:38:06 2016
@@ -35,8 +35,8 @@ llvm::Type *CGOpenCLRuntime::convertOpen
"Not an OpenCL specific type!");
llvm::LLVMContext& Ctx = CGM.getLLVMContext();
- uint32_t ImgAddrSpc =
-CGM.getTargetCodeGenInfo().getOpenCLImageAddrSpace(CGM);
+ uint32_t ImgAddrSpc = CGM.getContext().getTargetAddressSpace(
+CGM.getTarget().getOpenCLImageAddrSpace());
switch (cast(T)->getKind()) {
default:
llvm_unreachable("Unexpected opencl builtin type!");
Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=277647&r1=277646&r2=277647&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Aug 3 15:38:06 2016
@@ -401,10 +401,6 @@ unsigned TargetCodeGenInfo::getOpenCLKer
return llvm::CallingConv::C;
}
-unsigned TargetCodeGenInfo::getOpenCLImageAddrSpace(CodeGen::CodeGenModule
&CGM) const {
- return CGM.getContext().getTargetAddressSpace(LangAS::opencl_global);
-}
-
static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays);
/// isEmptyField - Return true iff a the field is "empty", that is it
@@ -6883,7 +6879,6 @@ public:
void setTargetAt
Re: [PATCH] D22927: [OpenCL] Fix size of image type
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277647: [OpenCL] Fix size of image type (authored by yaxunl).
Changed prior to commit:
https://reviews.llvm.org/D22927?vs=65964&id=66705#toc
Repository:
rL LLVM
https://reviews.llvm.org/D22927
Files:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
Index: cfe/trunk/include/clang/Basic/TargetInfo.h
===
--- cfe/trunk/include/clang/Basic/TargetInfo.h
+++ cfe/trunk/include/clang/Basic/TargetInfo.h
@@ -986,6 +986,11 @@
return getTargetOpts().SupportedOpenCLOptions;
}
+ /// \brief Get OpenCL image type address space.
+ virtual LangAS::ID getOpenCLImageAddrSpace() const {
+return LangAS::opencl_global;
+ }
+
/// \brief Check the target is valid after it is fully initialized.
virtual bool validateTarget(DiagnosticsEngine &Diags) const {
return true;
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -2131,6 +2131,10 @@
}
}
+ LangAS::ID getOpenCLImageAddrSpace() const override {
+return LangAS::opencl_constant;
+ }
+
CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
switch (CC) {
default:
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -401,10 +401,6 @@
return llvm::CallingConv::C;
}
-unsigned TargetCodeGenInfo::getOpenCLImageAddrSpace(CodeGen::CodeGenModule &CGM) const {
- return CGM.getContext().getTargetAddressSpace(LangAS::opencl_global);
-}
-
static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays);
/// isEmptyField - Return true iff a the field is "empty", that is it
@@ -6883,7 +6879,6 @@
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
unsigned getOpenCLKernelCallingConv() const override;
- unsigned getOpenCLImageAddrSpace(CodeGen::CodeGenModule &CGM) const override;
};
}
@@ -6920,10 +6915,6 @@
return llvm::CallingConv::AMDGPU_KERNEL;
}
-unsigned AMDGPUTargetCodeGenInfo::getOpenCLImageAddrSpace(CodeGen::CodeGenModule &CGM) const {
- return CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);
-}
-
//===--===//
// SPARC v8 ABI Implementation.
// Based on the SPARC Compliance Definition version 2.4.1.
Index: cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
@@ -35,8 +35,8 @@
"Not an OpenCL specific type!");
llvm::LLVMContext& Ctx = CGM.getLLVMContext();
- uint32_t ImgAddrSpc =
-CGM.getTargetCodeGenInfo().getOpenCLImageAddrSpace(CGM);
+ uint32_t ImgAddrSpc = CGM.getContext().getTargetAddressSpace(
+CGM.getTarget().getOpenCLImageAddrSpace());
switch (cast(T)->getKind()) {
default:
llvm_unreachable("Unexpected opencl builtin type!");
Index: cfe/trunk/lib/CodeGen/TargetInfo.h
===
--- cfe/trunk/lib/CodeGen/TargetInfo.h
+++ cfe/trunk/lib/CodeGen/TargetInfo.h
@@ -220,9 +220,6 @@
/// Get LLVM calling convention for OpenCL kernel.
virtual unsigned getOpenCLKernelCallingConv() const;
-
- /// Get LLVM Image Address Space for OpenCL kernel.
- virtual unsigned getOpenCLImageAddrSpace(CodeGen::CodeGenModule &CGM) const;
};
} // namespace CodeGen
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -1750,14 +1750,18 @@
case BuiltinType::OCLQueue:
case BuiltinType::OCLNDRange:
case BuiltinType::OCLReserveID:
-#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
-case BuiltinType::Id:
-#include "clang/Basic/OpenCLImageTypes.def"
-
// Currently these types are pointers to opaque types.
Width = Target->getPointerWidth(0);
Align = Target->getPointerAlign(0);
break;
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
+case BuiltinType::Id:
+#include "clang/Basic/OpenCLImageTypes.def"
+ {
+auto AS = getTargetAddressSpace(Target->getOpenCLImageAddrSpace());
+Width = Target->getPointerWidth(AS);
+Align = Target->getPointerAlign(AS);
+ }
}
break;
case Type::ObjCObjectPointer:
___
Re: [PATCH] D14274: Add alloc_size attribute to clang
george.burgess.iv added inline comments.
Comment at: test/CodeGenCXX/alloc-size.cpp:66
@@ +65,3 @@
+ // CHECK: ret i32 122
+ return __builtin_object_size(my_malloc(), 0) +
+ __builtin_object_size(my_calloc(5), 0) +
ahatanak wrote:
> george.burgess.iv wrote:
> > ahatanak wrote:
> > > Is it necessary to compute __builtin_object_size in the front-end (rather
> > > than in some IR passes like instcombine) when it takes the pointer
> > > returned by a function marked alloc_size?
> > >
> > > Also, is the IR optimization smart enough to get the exact object size in
> > > the following case?
> > >
> > > ```
> > > void foo(int a, int b) {
> > > void *p0 = my_malloc(a);
> > > g0 = __builtin_object_size(p0, 0);
> > > void *p1 = my_calloc(a, b);
> > > g1 = __builtin_object_size(p1, 1);
> > > }
> > >
> > > void foo1() {
> > > foo(10, 50);
> > > }
> > > ```
> > > Is it necessary to compute __builtin_object_size in the front-end (rather
> > > than in some IR passes like instcombine)
> >
> > It's necessary to try in the frontend. If that fails, we'll lower calls to
> > __bos to `@llvm.objectsize`, and let LLVM try it.
> >
> > The main reason that we care for the frontend is, among other reasons, so
> > we can use it with the `enable_if` attribute, like so:
> >
> > ```
> > void strncat(char *buf, int n, const char *from)
> > __attribute__((overloadable,
> > enable_if(__builtin_object_size(buf, 0) < n, ""),
> > unavailable("'n' is larger than the target buffer!")));
> >
> > void strncat(char *buf, int n, const char *from)
> > __attribute__((overloadable));
> >
> > int main() {
> > char buf[4];
> > strncat(buf, sizeof(buf)+1, "hi"); // expected-error{{'n' is larger than
> > the target buffer!}}
> > }
> > ```
> >
> >
> >
> >
> > > Also, is the IR optimization smart enough to get the exact object size in
> > > the following case?
> >
> > `__builtin_object_size` *always* gets lowered to a constant by either clang
> > or LLVM. That is, if `foo` gets inlined into `foo1`, then LLVM should be
> > able to determine accurate values for `g0` and `g1`. If `foo` isn't
> > inlined, then no: you'll get -1 for both.
> >
> > If you'd like to see what we can and can't get, LLVM already has the
> > `allocsize` attribute (and `@llvm.objectsize` intrinsic) in it. :)
> Thanks for the explanation. I see why you want to evaluate it in the
> front-end.
>
> It looks like llvm optimizes function foo before it gets inlined into
> function foo1 and, as a result, g0 and g1 both get -1 instead of 10 and 500.
> I guess llvm should defer optimizing the function until after inliner is run.
Hm. Looks like how we handle allocsize in LLVM may be slightly broken, then,
because it works with regular allocas.
Filed PR28834. Thanks for the heads-up. :)
https://reviews.llvm.org/D14274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [libcxx] r273034 - Add Filesystem TS -- Complete
Hi Eric,
After we upgraded our green dragon bots to El Captain (10.11), the
test below started to fail on some of our machines:
--
test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
TEST_CASE(hard_link_count_for_directory)
{
uintmax_t DirExpect = 3;
uintmax_t Dir3Expect = 2;
#if defined(__APPLE__)
DirExpect += 2;
Dir3Expect += 1;
#endif
TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect);
TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect);
std::error_code ec;
TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect);
TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect);
}
You can see the issue in every recent (past 10 days) run on
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/433
I noticed that commenting out the snippet around "#if
defined(__APPLE__)" would make it work. What's the rationale behind
the "#if defined(__APPLE__)" above?
Thanks,
On Tue, Jun 21, 2016 at 3:03 PM, Eric Fiselier via cfe-commits
wrote:
> The issue should be fixed in r273323.
>
> Thanks for the report and for your patience.
>
> /Eric
>
> On Mon, Jun 20, 2016 at 11:27 PM, Eric Fiselier wrote:
>>
>> Hi Artem,
>>
>> Sorry for the delay, I've been busy with school all day.
>> I'll check in a fix tomorrow morning.
>>
>> Sorry again,
>>
>> /Eric
>>
>> On Mon, Jun 20, 2016 at 2:31 PM, Eric Fiselier wrote:
>>>
>>> Oh shoot, I definitely didn't take that into account. I'll put together a
>>> fix.
>>>
>>> /Eric
>>>
>>>
>>>
>>> On Mon, Jun 20, 2016 at 2:27 PM, Artem Belevich wrote:
Eric,
Some tests appear to fail if the path to the tests' current directory
has some symlinks in it.
In my case source and build tree are in directory 'work' that's
symlinked to from my home directory:
/usr/local/home/tra/work -> /work/tra
This causes multiple failures in libcxx tests. One example:
projects/libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.rename/rename.pass.cpp:121
121TEST_CHECK(read_symlink(bad_sym_dest) == dne);
dne:
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
bad_sym_dest:
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
These are the paths that traverse the 'work' symlink in my home
directory. However, the symlink that we're reading contains physical path
to
the directory. While it does link to the right place, it's not the path the
test expects.
#readlink
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
/work/tra/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
I think we need to normalize paths before we compare them.
--Artem
On Sat, Jun 18, 2016 at 12:03 PM, Eric Fiselier via cfe-commits
wrote:
>
> > I assume the correct way to fix this is to disable
> > -Wcovered-switch-default while compiling
> > libcxx/src/experimental/filesystem/operations.cpp
>
> Agreed. Disabled in r273092.
>
> Thanks for your patience with this latest change,
>
> /Eric
>
> On Sat, Jun 18, 2016 at 12:54 PM, Adrian Prantl
> wrote:
>>
>> Hello Eric,
>>
>> this commit causes new warnings on our bots:
>>
>> clang/src/projects/libcxx/include/fstream:816:5: warning: default
>> label in switch which covers all enumeration values
>> [-Wcovered-switch-default]
>> default:
>>
>> The problem is with this defensive default statement in fstream:
>>
>>
>> template
>> 0792 typename basic_filebuf<_CharT, _Traits>::pos_type
>> 0793 basic_filebuf<_CharT, _Traits>::seekoff(off_type __off,
>> ios_base::seekdir __way,
>> 0794 ios_base::openmode)
>> 0795 {
>> 0796 #ifndef _LIBCPP_NO_EXCEPTIONS
>> 0797 if (!__cv_)
>> 0798 throw bad_cast();
>> 0799 #endif
>> 0800 int __width = __cv_->encoding();
>> 0801 if (__file_ == 0 || (__width <= 0 && __off != 0) || sync())
>> 0802 return pos_type(off_type(-1));
>> 0803 // __width > 0 || __off == 0
>> 0804 int __whence;
>> 0805 switch (__way)
>> 0806 {
>> 0807 case ios_base::beg:
>> 0808 __whence = SEEK_SET;
>> 0809 break;
>> 0810 case ios_base::cur:
>> 0811 __whence = SEEK_CUR;
>> 0812 break;
>> 0813 case ios_base::end:
>> 0814 __whence = SEEK_END;
>> 0815 break;
>> 0816 default:
>> 0817 return pos_type(off_type(-1));
>> 0818 }
>>
>>
>> I assume th
[PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode
alexfh created this revision.
alexfh added a reviewer: hokein.
alexfh added a subscriber: cfe-commits.
The misc-argument-comment check now ignores leading and trailing underscores and
case. The new `StrictMode` local/global option can be used to switch back to
strict checking.
Add getLocalOrGlobal version for integral types, minor cleanups.
https://reviews.llvm.org/D23135
Files:
clang-tidy/ClangTidy.h
clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tidy/misc/ArgumentCommentCheck.h
test/clang-tidy/misc-argument-comment-strict.cpp
test/clang-tidy/misc-argument-comment.cpp
unittests/clang-tidy/MiscModuleTest.cpp
Index: unittests/clang-tidy/MiscModuleTest.cpp
===
--- unittests/clang-tidy/MiscModuleTest.cpp
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -23,10 +23,10 @@
TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) {
EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }",
runCheckOnCode(
-"void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }"));
+"void f(int xxx, int yyy); void g() { f(/*Zxx=*/0, 0); }"));
EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
runCheckOnCode(
-"struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
+"struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);"));
}
TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
Index: test/clang-tidy/misc-argument-comment.cpp
===
--- test/clang-tidy/misc-argument-comment.cpp
+++ test/clang-tidy/misc-argument-comment.cpp
@@ -36,13 +36,18 @@
void templates() {
variadic(/*xxx=*/0, /*yyy=*/1);
- variadic2(/*zzZ=*/0, /*xxx=*/1, /*yyy=*/2);
- // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzZ' in comment does not match parameter name 'zzz'
+ variadic2(/*zzU=*/0, /*xxx=*/1, /*yyy=*/2);
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzU' in comment does not match parameter name 'zzz'
// CHECK-FIXES: variadic2(/*zzz=*/0, /*xxx=*/1, /*yyy=*/2);
}
#define FALSE 0
void qqq(bool aaa);
void f() { qqq(/*bbb=*/FALSE); }
// CHECK-MESSAGES: [[@LINE-1]]:16: warning: argument name 'bbb' in comment does not match parameter name 'aaa'
// CHECK-FIXES: void f() { qqq(/*bbb=*/FALSE); }
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+ f(/*With_Underscores=*/false);
+}
Index: test/clang-tidy/misc-argument-comment-strict.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-argument-comment-strict.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s misc-argument-comment %t -- \
+// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" --
+
+void f(int _with_underscores_);
+void g(int x_);
+void ignores_underscores() {
+ f(/*With_Underscores=*/0);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'With_Underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/0);
+ f(/*with_underscores=*/1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'with_underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/1);
+ f(/*_With_Underscores_=*/2);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name '_With_Underscores_' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/2);
+ g(/*X=*/3);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'X' in comment does not match parameter name 'x_'
+// CHECK-FIXES: g(/*x_=*/3);
+}
Index: clang-tidy/misc/ArgumentCommentCheck.h
===
--- clang-tidy/misc/ArgumentCommentCheck.h
+++ clang-tidy/misc/ArgumentCommentCheck.h
@@ -37,8 +37,10 @@
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap& Opts) override;
private:
+ const bool StrictMode;
llvm::Regex IdentRE;
bool isLikelyTypo(llvm::ArrayRef Params, StringRef ArgName,
Index: clang-tidy/misc/ArgumentCommentCheck.cpp
===
--- clang-tidy/misc/ArgumentCommentCheck.cpp
+++ clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@
ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
+ StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+}
+
void ArgumentCommentCheck::regi
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
bkramer updated this revision to Diff 66716.
bkramer added a comment.
Add relnote.
https://reviews.llvm.org/D23130
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/GlobalNamespaceCheck.cpp
clang-tidy/misc/GlobalNamespaceCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-global-namespace.rst
test/clang-tidy/misc-global-namespace.cpp
Index: test/clang-tidy/misc-global-namespace.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-global-namespace.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s misc-global-namespace %t
+
+void f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: definition for 'f' in the global namespace
+class F {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: definition for 'F' in the global namespace
+int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: definition for 'i' in the global namespace
+
+// No warnings below.
+void g();
+
+extern "C" void h() {}
+
+extern "C" {
+void j() {}
+}
+
+struct Clike {
+ int i;
+};
+
+void *operator new(__SIZE_TYPE__, int) { return 0; }
+void operator delete(void *, int) {}
+
+__attribute__((visibility("hidden"))) void k() {}
+static void l() {}
+namespace {
+void m() {}
+}
+namespace x {
+void n() {}
+}
+
+int main() {}
Index: docs/clang-tidy/checks/misc-global-namespace.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-global-namespace.rst
@@ -0,0 +1,7 @@
+.. title:: clang-tidy - misc-global-namespace
+
+misc-global-namespace
+=
+
+Finds definitions in the global namespace. Those definitions are prone to ODR
+conflicts.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -59,6 +59,7 @@
misc-definitions-in-headers
misc-fold-init-type
misc-forward-declaration-namespace
+ misc-global-namespace
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -69,6 +69,11 @@
Flags classes where some, but not all, special member functions are user-defined.
+- New `misc-global-namespace
+ `_ check
+
+ Flags definitions in the global namespace.
+
Improvements to include-fixer
-
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,18 +12,18 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
-#include "MisplacedConstCheck.h"
-#include "UnconventionalAssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "DanglingHandleCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
+#include "GlobalNamespaceCheck.h"
#include "InaccurateEraseCheck.h"
#include "IncorrectRoundings.h"
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
+#include "MisplacedConstCheck.h"
#include "MisplacedWideningCastCheck.h"
#include "MoveConstantArgumentCheck.h"
#include "MoveConstructorInitCheck.h"
@@ -44,6 +44,7 @@
#include "SuspiciousStringCompareCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
+#include "UnconventionalAssignOperatorCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
@@ -62,6 +63,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-global-namespace");
CheckFactories.registerCheck(
"misc-misplaced-const");
CheckFactories.registerCheck(
Index: clang-tidy/misc/GlobalNamespaceCheck.h
===
--- /dev/null
+++ clang-tidy/misc/GlobalNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- GlobalNamespaceCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
alexfh added a comment. Do we want to merge this check with google/GlobalNamesInHeadersCheck.cpp that targets a similar set of issues? Repository: rL LLVM https://reviews.llvm.org/D23130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
On Wed, Aug 3, 2016 at 5:17 PM, Alexander Kornienko wrote: > alexfh added a comment. > > Do we want to merge this check with google/GlobalNamesInHeadersCheck.cpp that > targets a similar set of issues? We also have misc/DefinitionsInHeadersCheck.cpp which is likely similar. ~Aaron > > > Repository: > rL LLVM > > https://reviews.llvm.org/D23130 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
bkramer added a comment. DefinitionsInHeaders is tackling a different problem. IMO DefinitionsInHeaders is something that should be on by default everywhere, while this check for definitions in the global namespace is more of a coding style issue. GlobalNamesInHeaders is a bit of a misnomer, it looks for using declarations in headers. I don't think it makes sense to merge this check into it either, with the new check we mostly find things exported by accident in .cpp files, it has little to do with headers. I'm welcome to naming suggestions for this check, btw ;) Repository: rL LLVM https://reviews.llvm.org/D23130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23041: Un-XFAIL GCC atomics.align
jfb added a comment. I wrote a quick test, and I think this is technically OK for x86 because alignment "Just Works", but I think it's borked on GCC+ARM (I don't have a system to test that here): https://github.com/jfbastien/atomic_nullptr/blob/master/atomic_nullptr.cc https://reviews.llvm.org/D23041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277658 - [Concepts] remove default argument for RequiresClause; NFC
Author: hubert.reinterpretcast Date: Wed Aug 3 17:07:50 2016 New Revision: 277658 URL: http://llvm.org/viewvc/llvm-project?rev=277658&view=rev Log: [Concepts] remove default argument for RequiresClause; NFC Modified: cfe/trunk/include/clang/AST/DeclTemplate.h Modified: cfe/trunk/include/clang/AST/DeclTemplate.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=277658&r1=277657&r2=277658&view=diff == --- cfe/trunk/include/clang/AST/DeclTemplate.h (original) +++ cfe/trunk/include/clang/AST/DeclTemplate.h Wed Aug 3 17:07:50 2016 @@ -80,13 +80,12 @@ protected: Expr *RequiresClause); public: - // FIXME: remove default argument for RequiresClause static TemplateParameterList *Create(const ASTContext &C, SourceLocation TemplateLoc, SourceLocation LAngleLoc, ArrayRef Params, SourceLocation RAngleLoc, - Expr *RequiresClause = nullptr); + Expr *RequiresClause); /// \brief Iterates through the template parameters in this list. typedef NamedDecl** iterator; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D23058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
alexfh added a comment. In https://reviews.llvm.org/D23130#505242, @bkramer wrote: > DefinitionsInHeaders is tackling a different problem. IMO > DefinitionsInHeaders is something that should be on by default everywhere, > while this check for definitions in the global namespace is more of a coding > style issue. Yes, DefinitionsInHeaders is a different beast. > GlobalNamesInHeaders is a bit of a misnomer, it looks for using declarations > in headers. The `google-global-names-in-headers` check aims to "flag global namespace pollution in header files" including declarations of classes, functions and other entities. However, "right now it only triggers on using declarations and directives", so it's basically under-implemented. The only problem, I guess, is keeping false positives under control. The check you're proposing is somewhat close. It isn't specifically limited to headers (the `google-global-names-in-headers` check can run on all files if properly configured, and we can rename it to remove the -in-headers part). And the second difference is that it is limited to definitions, but I don't yet understand, why it is important, since declarations in the global namespace (except for using declarations, typedefs, etc.) will have a corresponding definition in the global namespace. I guess, these checks, if somewhat different, may be similar enough to share some implementation. Repository: rL LLVM https://reviews.llvm.org/D23130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r277663 - [clang-rename] improve USRFindingAction
Author: omtcyfz
Date: Wed Aug 3 18:00:32 2016
New Revision: 277663
URL: http://llvm.org/viewvc/llvm-project?rev=277663&view=rev
Log:
[clang-rename] improve USRFindingAction
1. Improve templated class renaming, namely add capabilities of finding partial
and full specializations. Every class partial specialization has reference to
the specialized class. Thus, storing all partial specializations and
comparing specialized class decls to the FoundDecl solves this. All full class
specializations can be found by calling ClassTemplateDecl::specializations().
2. Fix virtual function and its overriding functions renaming. Renaming a
virtual function requires renaming every other function in its "overriding
graph".
3. Merge TemplateClassInstantiationFindBy{Declaration|TypeUse}.cpp tests into
one test by adding multiple invocations of clang-rename to one test, because
the only different thing across these tests is -offset passed to clang-rename.
Reviewers: alexfh
Differential Revision: https://reviews.llvm.org/D23058
Added:
clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
Removed:
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
Modified:
clang-tools-extra/trunk/clang-rename/USRFinder.cpp
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
Modified: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFinder.cpp?rev=277663&r1=277662&r2=277663&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp Wed Aug 3 18:00:32 2016
@@ -81,6 +81,11 @@ public:
dyn_cast(Loc.getType())) {
return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
}
+if (const auto *TemplateSpecType =
+dyn_cast(Loc.getType())) {
+ return setResult(TemplateSpecType->getTemplateName().getAsTemplateDecl(),
+ TypeBeginLoc, TypeEndLoc);
+}
return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
TypeEndLoc);
}
Modified: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp?rev=277663&r1=277662&r2=277663&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp Wed Aug 3
18:00:32 2016
@@ -20,7 +20,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/FileManager.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
@@ -36,7 +35,6 @@
using namespace llvm;
-using namespace clang::ast_matchers;
namespace clang {
namespace rename {
@@ -46,45 +44,66 @@ namespace {
// AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
// Decl refers to class and adds USRs of all overridden methods if Decl refers
// to virtual method.
-class AdditionalUSRFinder : public MatchFinder::MatchCallback {
+class AdditionalUSRFinder : public RecursiveASTVisitor {
public:
explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
+: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
void Find() {
-USRSet.insert(getUSRForDecl(FoundDecl));
+// Fill OverriddenMethods and PartialSpecs storages.
+TraverseDecl(Context.getTranslationUnitDecl());
if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
- addUSRsFromOverrideSets(MethodDecl);
-}
-if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
- addUSRsOfCtorDtors(RecordDecl);
+ addUSRsOfOverridenFunctions(MethodDecl);
+ for (const auto &OverriddenMethod : OverriddenMethods) {
+if (checkIfOverriddenFunctionAscends(OverriddenMethod)) {
+ USRSet.insert(getUSRForDecl(OverriddenMethod));
+}
+ }
+} else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
+ handleCXXRecordDecl(RecordDecl);
+} else if (const auto *TemplateDecl =
+ dyn_cast(FoundDecl)) {
+ handleClassTemplateDecl(TemplateDecl);
+} else {
+ USRSet.insert(getUSRForDecl(FoundDecl));
}
-addMatchers();
-Finder.matchAST(Context);
USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
}
+ bool VisitCXXMethodDecl(const C
Re: [PATCH] D23058: [clang-rename] improve USRFindingAction
This revision was automatically updated to reflect the committed changes.
omtcyfz marked an inline comment as done.
Closed by commit rL277663: [clang-rename] improve USRFindingAction (authored by
omtcyfz).
Changed prior to commit:
https://reviews.llvm.org/D23058?vs=66670&id=66728#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23058
Files:
clang-tools-extra/trunk/clang-rename/USRFinder.cpp
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
clang-tools-extra/trunk/test/clang-rename/ComplexFunctionOverride.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiation.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp
clang-tools-extra/trunk/test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp
Index: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp
@@ -81,6 +81,11 @@
dyn_cast(Loc.getType())) {
return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
}
+if (const auto *TemplateSpecType =
+dyn_cast(Loc.getType())) {
+ return setResult(TemplateSpecType->getTemplateName().getAsTemplateDecl(),
+ TypeBeginLoc, TypeEndLoc);
+}
return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
TypeEndLoc);
}
Index: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
@@ -20,7 +20,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/FileManager.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
@@ -36,7 +35,6 @@
using namespace llvm;
-using namespace clang::ast_matchers;
namespace clang {
namespace rename {
@@ -46,68 +44,101 @@
// AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
// Decl refers to class and adds USRs of all overridden methods if Decl refers
// to virtual method.
-class AdditionalUSRFinder : public MatchFinder::MatchCallback {
+class AdditionalUSRFinder : public RecursiveASTVisitor {
public:
explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
+: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
void Find() {
-USRSet.insert(getUSRForDecl(FoundDecl));
+// Fill OverriddenMethods and PartialSpecs storages.
+TraverseDecl(Context.getTranslationUnitDecl());
if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
- addUSRsFromOverrideSets(MethodDecl);
-}
-if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
- addUSRsOfCtorDtors(RecordDecl);
+ addUSRsOfOverridenFunctions(MethodDecl);
+ for (const auto &OverriddenMethod : OverriddenMethods) {
+if (checkIfOverriddenFunctionAscends(OverriddenMethod)) {
+ USRSet.insert(getUSRForDecl(OverriddenMethod));
+}
+ }
+} else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
+ handleCXXRecordDecl(RecordDecl);
+} else if (const auto *TemplateDecl =
+ dyn_cast(FoundDecl)) {
+ handleClassTemplateDecl(TemplateDecl);
+} else {
+ USRSet.insert(getUSRForDecl(FoundDecl));
}
-addMatchers();
-Finder.matchAST(Context);
USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
}
+ bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
+if (MethodDecl->isVirtual()) {
+ OverriddenMethods.push_back(MethodDecl);
+}
+return true;
+ }
+
+ bool VisitClassTemplatePartialSpecializationDecl(
+ const ClassTemplatePartialSpecializationDecl *PartialSpec) {
+PartialSpecs.push_back(PartialSpec);
+return true;
+ }
+
private:
- void addMatchers() {
-const auto CXXMethodDeclMatcher =
-cxxMethodDecl(forEachOverridden(cxxMethodDecl().bind("cxxMethodDecl")));
-Finder.addMatcher(CXXMethodDeclMatcher, this);
+ void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+RecordDecl = RecordDecl->getDefinition();
+if (const auto *ClassTemplateSpecDecl
+= dyn_cast(RecordDecl)) {
+ handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
+}
+addUSRsOfCtorDtors(RecordDecl);
}
- // FIXME: Implement matchesUSR matchers to make lookups more efficient.
- virtual void run(const MatchFinder::MatchResult &Result) {
-const auto *VirtualMethod =
-Result.Nodes.getNodeAs("cxxMethodDecl");
-bool Found = false;
-
Re: [PATCH] D19909: [Attr] Add support for the `ms_hook_prologue` attribute.
cdavis5x updated this revision to Diff 66729.
cdavis5x added a comment.
Update for merge conflicts.
- Add a blurb to the docs advising against using this attribute with
`__forceinline`, `always_inline`, or `naked`.
https://reviews.llvm.org/D19909
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
test/CodeGen/function-attributes.c
test/Sema/attr-ms-hook-prologue-wrong-arch.c
test/Sema/attr-ms-hook-prologue.c
Index: test/Sema/attr-ms-hook-prologue.c
===
--- /dev/null
+++ test/Sema/attr-ms-hook-prologue.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple i386-pc-linux -fms-extensions -fsyntax-only -verify %s
+
+int __attribute__((ms_hook_prologue)) foo(int a, int b) {
+ return a+b;
+}
+
+// expected-note@+2{{conflicting attribute is here}}
+// expected-error@+1{{'naked' and 'ms_hook_prologue' attributes are not compatible}}
+__declspec(naked) int __attribute__((ms_hook_prologue)) bar(int a, int b) {
+}
+
+// expected-note@+2{{conflicting attribute is here}}
+// expected-error@+1{{'__forceinline' and 'ms_hook_prologue' attributes are not compatible}}
+__forceinline int __attribute__((ms_hook_prologue)) baz(int a, int b) {
+ return a-b;
+}
+
+// expected-warning@+1{{'ms_hook_prologue' attribute only applies to functions}}
+int x __attribute__((ms_hook_prologue));
+
+// expected-error@+1{{'ms_hook_prologue' attribute takes no arguments}}
+int f(int a, int b) __attribute__((ms_hook_prologue(2)));
Index: test/Sema/attr-ms-hook-prologue-wrong-arch.c
===
--- /dev/null
+++ test/Sema/attr-ms-hook-prologue-wrong-arch.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple s390x-unknown-linux -fms-extensions -fsyntax-only -verify %s
+
+// expected-warning@+1{{unknown attribute 'ms_hook_prologue' ignored}}
+int __attribute__((ms_hook_prologue)) foo(int a, int b) {
+ return a+b;
+}
Index: test/CodeGen/function-attributes.c
===
--- test/CodeGen/function-attributes.c
+++ test/CodeGen/function-attributes.c
@@ -108,11 +108,18 @@
_setjmp(0);
}
+// CHECK-LABEL: define void @f21
+// CHECK: [[HOTPATCH:#[0-9]+]]
+// CHECK: {
+void __attribute__((ms_hook_prologue)) f21(void) {
+}
+
// CHECK: attributes [[NUW]] = { nounwind optsize{{.*}} }
// CHECK: attributes [[AI]] = { alwaysinline nounwind optsize{{.*}} }
// CHECK: attributes [[NUW_OS_RN]] = { nounwind optsize readnone{{.*}} }
// CHECK: attributes [[ALIGN]] = { nounwind optsize alignstack=16{{.*}} }
// CHECK: attributes [[RT]] = { nounwind optsize returns_twice{{.*}} }
+// CHECK: attributes [[HOTPATCH]] = { nounwind optsize{{.*}}"patchable-function"="ms-hotpatch"{{.*}} }
// CHECK: attributes [[NR]] = { noreturn optsize }
// CHECK: attributes [[NUW_RN]] = { nounwind optsize readnone }
// CHECK: attributes [[RT_CALL]] = { optsize returns_twice }
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1664,15 +1664,6 @@
D->addAttr(CA);
}
-static void handleNakedAttr(Sema &S, Decl *D, const AttributeList &Attr) {
- if (checkAttrMutualExclusion(S, D, Attr.getRange(),
- Attr.getName()))
-return;
-
- D->addAttr(::new (S.Context) NakedAttr(Attr.getRange(), S.Context,
- Attr.getAttributeSpellingListIndex()));
-}
-
static void handleNoReturnAttr(Sema &S, Decl *D, const AttributeList &attr) {
if (hasDeclarator(D)) return;
@@ -3673,7 +3664,9 @@
static void handleAlwaysInlineAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
if (checkAttrMutualExclusion(S, D, Attr.getRange(),
- Attr.getName()))
+ Attr.getName()) ||
+ checkAttrMutualExclusion(S, D, Attr.getRange(),
+ Attr.getName()))
return;
if (AlwaysInlineAttr *Inline = S.mergeAlwaysInlineAttr(
@@ -5552,7 +5545,8 @@
handleHotAttr(S, D, Attr);
break;
case AttributeList::AT_Naked:
-handleNakedAttr(S, D, Attr);
+handleSimpleAttributeWithExclusions(S, D, Attr);
break;
case AttributeList::AT_NoReturn:
handleNoReturnAttr(S, D, Attr);
@@ -5780,6 +5774,9 @@
break;
case AttributeList::AT_LayoutVersion:
handleLayoutVersion(S, D, Attr);
+ case AttributeList::AT_MSHookPrologue:
+handleSimpleAttributeWithExclusions(S, D, Attr);
break;
case AttributeList::AT_MSNoVTable:
handleSimpleAttribute(S, D, Attr);
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1783,6 +1783,10 @@
Re: [PATCH] D19909: [Attr] Add support for the `ms_hook_prologue` attribute.
cdavis5x marked an inline comment as done. Comment at: include/clang/Basic/AttrDocs.td:560 @@ +559,3 @@ + +This attribute cannot be used in conjunction with the ``naked``, +``always_inline``, or ``__forceinline`` attributes. Done. https://reviews.llvm.org/D19909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation
Author: alexfh
Date: Wed Aug 3 18:06:03 2016
New Revision: 277677
URL: http://llvm.org/viewvc/llvm-project?rev=277677&view=rev
Log:
[clang-tidy] Inefficient string operation
Patch by Bittner Barni!
Differential revision: https://reviews.llvm.org/D20196
Added:
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt?rev=277677&r1=277676&r2=277677&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt Wed Aug 3
18:06:03 2016
@@ -4,6 +4,7 @@ add_clang_library(clangTidyPerformanceMo
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
ImplicitCastInLoopCheck.cpp
+ InefficientStringConcatenationCheck.cpp
PerformanceTidyModule.cpp
UnnecessaryCopyInitialization.cpp
UnnecessaryValueParamCheck.cpp
Added:
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp?rev=277677&view=auto
==
---
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
(added)
+++
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
Wed Aug 3 18:06:03 2016
@@ -0,0 +1,86 @@
+//===--- InefficientStringConcatenationCheck.cpp -
clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "InefficientStringConcatenationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+void InefficientStringConcatenationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+}
+
+InefficientStringConcatenationCheck::InefficientStringConcatenationCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context), StrictMode(Options.get("StrictMode", 0))
{}
+
+void InefficientStringConcatenationCheck::registerMatchers(
+MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+return;
+
+ const auto BasicStringType =
+ hasType(cxxRecordDecl(hasName("::std::basic_string")));
+
+ const auto BasicStringPlusOperator = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("+"),
+ hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType;
+
+ const auto PlusOperator =
+ cxxOperatorCallExpr(
+ hasOverloadedOperatorName("+"),
+ hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
+ hasDescendant(BasicStringPlusOperator))
+ .bind("plusOperator");
+
+ const auto AssignOperator = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("="),
+ hasArgument(0, declRefExpr(BasicStringType,
+ hasDeclaration(decl().bind("lhsStrT")))
+ .bind("lhsStr")),
+ hasArgument(1, stmt(hasDescendant(declRefExpr(
+ hasDeclaration(decl(equalsBoundNode("lhsStrT"))),
+ hasDescendant(BasicStringPlusOperator));
+
+ if (StrictMode) {
+Finder->addMatcher(cxxOperatorCallExpr(anyOf(AssignOperator,
PlusOperator)),
+ this);
+ } else {
+Finder->addMatcher(
+cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator),
+hasAncestor(stmt(anyOf(cxxForRangeStmt(),
+ whileStmt(), forStmt(),
+this);
+ }
+}
+
+void InefficientStringConcatenationCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *LhsStr = Result.Nodes.getNodeAs("lhsStr");
+ const auto *PlusOperator =
+ Result.Nodes.getNodeAs("plusOperator");
+ const auto DiagMsg =
+ "string concatenation results in alloc
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277677: [clang-tidy] Inefficient string operation (authored
by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D20196?vs=66492&id=66730#toc
Repository:
rL LLVM
https://reviews.llvm.org/D20196
Files:
clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-string-concatenation.cpp
Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.h
@@ -0,0 +1,41 @@
+//===--- InefficientStringConcatenationCheck.h - clang-tidy---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// This check is to warn about the performance overhead arising from
+/// concatenating strings, using the operator+, instead of operator+=.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-string-concatenation.html
+class InefficientStringConcatenationCheck : public ClangTidyCheck {
+public:
+ InefficientStringConcatenationCheck(StringRef Name,
+ ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const bool StrictMode;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTSTRINGCONCATENATION_H
Index: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "InefficientStringConcatenationCheck.h"
#include "FasterStringFindCheck.h"
#include "ForRangeCopyCheck.h"
@@ -30,6 +31,8 @@
"performance-for-range-copy");
CheckFactories.registerCheck(
"performance-implicit-cast-in-loop");
+CheckFactories.registerCheck(
+"performance-inefficient-string-concatenation");
CheckFactories.registerCheck(
"performance-unnecessary-copy-initialization");
CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
@@ -0,0 +1,86 @@
+//===--- InefficientStringConcatenationCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "InefficientStringConcatenationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+void InefficientStringConcatenationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+}
+
+InefficientStringConcatenationCheck::InefficientStringConcatenationCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context), StrictM
Re: [PATCH] D23096: [Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope
ahatanak added a comment. In each of these cases, "foo::a" is a non-dependent type, so it seems that clang shouldn't use DependentScopeDeclRefExpr or CXXDependentScopeMemberExpr in the AST when the template definitions are parsed. https://reviews.llvm.org/D23096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23147: [ADT] Migrate DepthFirstIterator to use NodeRef
timshen created this revision.
timshen added reviewers: dblaikie, chandlerc.
timshen added a subscriber: cfe-commits.
The corresponding LLVM change is D23146.
https://reviews.llvm.org/D23147
Files:
include/clang/AST/StmtGraphTraits.h
include/clang/Analysis/Analyses/Dominators.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -451,6 +451,7 @@
namespace llvm {
template<> struct GraphTraits {
typedef clang::ento::ExplodedNode NodeType;
+typedef clang::ento::ExplodedNode *NodeRef;
typedef NodeType::succ_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
@@ -477,6 +478,7 @@
template<> struct GraphTraits {
typedef const clang::ento::ExplodedNode NodeType;
+typedef const clang::ento::ExplodedNode *NodeRef;
typedef NodeType::const_succ_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
Index: include/clang/Analysis/Analyses/Dominators.h
===
--- include/clang/Analysis/Analyses/Dominators.h
+++ include/clang/Analysis/Analyses/Dominators.h
@@ -168,6 +168,7 @@
namespace llvm {
template <> struct GraphTraits< ::clang::DomTreeNode* > {
typedef ::clang::DomTreeNode NodeType;
+ typedef ::clang::DomTreeNode *NodeRef;
typedef NodeType::iterator ChildIteratorType;
static NodeType *getEntryNode(NodeType *N) {
Index: include/clang/AST/StmtGraphTraits.h
===
--- include/clang/AST/StmtGraphTraits.h
+++ include/clang/AST/StmtGraphTraits.h
@@ -26,6 +26,7 @@
template <> struct GraphTraits {
typedef clang::Stmt NodeType;
+ typedef clang::Stmt * NodeRef;
typedef clang::Stmt::child_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
@@ -53,6 +54,7 @@
template <> struct GraphTraits {
typedef const clang::Stmt NodeType;
+ typedef const clang::Stmt * NodeRef;
typedef clang::Stmt::const_child_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -451,6 +451,7 @@
namespace llvm {
template<> struct GraphTraits {
typedef clang::ento::ExplodedNode NodeType;
+typedef clang::ento::ExplodedNode *NodeRef;
typedef NodeType::succ_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
@@ -477,6 +478,7 @@
template<> struct GraphTraits {
typedef const clang::ento::ExplodedNode NodeType;
+typedef const clang::ento::ExplodedNode *NodeRef;
typedef NodeType::const_succ_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
Index: include/clang/Analysis/Analyses/Dominators.h
===
--- include/clang/Analysis/Analyses/Dominators.h
+++ include/clang/Analysis/Analyses/Dominators.h
@@ -168,6 +168,7 @@
namespace llvm {
template <> struct GraphTraits< ::clang::DomTreeNode* > {
typedef ::clang::DomTreeNode NodeType;
+ typedef ::clang::DomTreeNode *NodeRef;
typedef NodeType::iterator ChildIteratorType;
static NodeType *getEntryNode(NodeType *N) {
Index: include/clang/AST/StmtGraphTraits.h
===
--- include/clang/AST/StmtGraphTraits.h
+++ include/clang/AST/StmtGraphTraits.h
@@ -26,6 +26,7 @@
template <> struct GraphTraits {
typedef clang::Stmt NodeType;
+ typedef clang::Stmt * NodeRef;
typedef clang::Stmt::child_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
@@ -53,6 +54,7 @@
template <> struct GraphTraits {
typedef const clang::Stmt NodeType;
+ typedef const clang::Stmt * NodeRef;
typedef clang::Stmt::const_child_iterator ChildIteratorType;
typedef llvm::df_iterator nodes_iterator;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20196: [clang-tidy] Inefficient string operation
Eugene.Zelenko added a comment. Please see PR28836. In some cases check should recommend to use insert(). Repository: rL LLVM https://reviews.llvm.org/D20196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
Prazek added a comment. In https://reviews.llvm.org/D22725#505020, @JDevlieghere wrote: > Addresses comments from Aaron Ballman > > @aaron.ballman Thanks for the thorough review! Can you check whether the > tests I added address your concerns? Could you also elaborate on the case > with the C-function pointer? Unless I explicitly cast it to void* the > compiler rejects will reject it as an argument to memcpy. Am I missing a case > where this could go wrong? I still added it to the pointer arithmetic check > though, just to be sure. Did you manage to see what was wrong in the transformation that you did on LLVM? I have one idea, which should be fixed. Memset takes char as an argument, so if you would fill the array of ints with memset(arr, 0, sizeof(arr)), then it will set all the elements to zero. But if you would do it with with set say 1, you will end up having int val = 1 | (1 << 8) | (1 << 16) | (1 << 24) for each elemnt of the array. I don't think tho that any LLVM code depend od this, you usually set everything to zero, or to another value if you operate on char type. So allow transformation and diagnostic if the argument equals 0, or when the type is char. Repository: rL LLVM https://reviews.llvm.org/D22725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode
Prazek added a subscriber: Prazek. Prazek added a comment. LG(TM) https://reviews.llvm.org/D23135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.
Prazek added inline comments. Comment at: clang-tidy/misc/GlobalNamespaceCheck.cpp:46 @@ +45,3 @@ +// extern "C" globals need to be in the global namespace. +if (VDecl->isExternC()) + return; I think it would be better to check it in matcher. I see that there is isExternC, but it only works for FunctionDecl, but I think that adding isExternC for VarDecl would be good Comment at: clang-tidy/misc/GlobalNamespaceCheck.cpp:67 @@ +66,3 @@ + } + + // FIXME: If we want to be really fancy we could move declaration-less What about macros? I think you should check isMacroId on location here (don't do fixit when it is in macro, but print warning) Also please add tests for it. Repository: rL LLVM https://reviews.llvm.org/D23130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22766: Handle -mlong-calls on Hexagon
echristo added a comment. You haven't removed the custom handling? -eric Repository: rL LLVM https://reviews.llvm.org/D22766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy
Alexander_Droste marked 3 inline comments as done. Alexander_Droste added a comment. https://reviews.llvm.org/D22729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277696 - After PR28761 use -Wall with -Werror in builtins tests to identify
Author: echristo Date: Thu Aug 4 01:02:50 2016 New Revision: 277696 URL: http://llvm.org/viewvc/llvm-project?rev=277696&view=rev Log: After PR28761 use -Wall with -Werror in builtins tests to identify possible problems in headers. Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c cfe/trunk/test/CodeGen/avx-builtins.c cfe/trunk/test/CodeGen/avx2-builtins.c cfe/trunk/test/CodeGen/avx512bw-builtins.c cfe/trunk/test/CodeGen/avx512cdintrin.c cfe/trunk/test/CodeGen/avx512dq-builtins.c cfe/trunk/test/CodeGen/avx512er-builtins.c cfe/trunk/test/CodeGen/avx512f-builtins.c cfe/trunk/test/CodeGen/avx512ifma-builtins.c cfe/trunk/test/CodeGen/avx512ifmavl-builtins.c cfe/trunk/test/CodeGen/avx512pf-builtins.c cfe/trunk/test/CodeGen/avx512vbmi-builtins.c cfe/trunk/test/CodeGen/avx512vbmivl-builtin.c cfe/trunk/test/CodeGen/avx512vl-builtins.c cfe/trunk/test/CodeGen/avx512vlbw-builtins.c cfe/trunk/test/CodeGen/avx512vlcd-builtins.c cfe/trunk/test/CodeGen/avx512vldq-builtins.c cfe/trunk/test/CodeGen/bmi-builtins.c cfe/trunk/test/CodeGen/builtin-clflushopt.c cfe/trunk/test/CodeGen/f16c-builtins.c cfe/trunk/test/CodeGen/fma4-builtins.c cfe/trunk/test/CodeGen/mmx-builtins.c cfe/trunk/test/CodeGen/pku.c cfe/trunk/test/CodeGen/sse-builtins.c cfe/trunk/test/CodeGen/sse2-builtins.c cfe/trunk/test/CodeGen/sse3-builtins.c cfe/trunk/test/CodeGen/sse41-builtins.c cfe/trunk/test/CodeGen/sse42-builtins.c cfe/trunk/test/CodeGen/sse4a-builtins.c cfe/trunk/test/CodeGen/ssse3-builtins.c cfe/trunk/test/CodeGen/x86_32-xsave.c cfe/trunk/test/CodeGen/x86_64-xsave.c cfe/trunk/test/CodeGen/xop-builtins.c Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-builtins.c?rev=277696&r1=277695&r2=277696&view=diff == --- cfe/trunk/test/CodeGen/3dnow-builtins.c (original) +++ cfe/trunk/test/CodeGen/3dnow-builtins.c Thu Aug 4 01:02:50 2016 @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa -emit-llvm -o - -Werror | FileCheck %s -check-prefix=GCC -check-prefix=CHECK -// RUN: %clang_cc1 %s -triple=x86_64-scei-ps4 -target-feature +3dnowa -emit-llvm -o - -Werror | FileCheck %s -check-prefix=PS4 -check-prefix=CHECK +// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa -emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=GCC -check-prefix=CHECK +// RUN: %clang_cc1 %s -triple=x86_64-scei-ps4 -target-feature +3dnowa -emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=PS4 -check-prefix=CHECK // Don't include mm_malloc.h, it's system specific. #define __MM_MALLOC_H Modified: cfe/trunk/test/CodeGen/avx-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx-builtins.c?rev=277696&r1=277695&r2=277696&view=diff == --- cfe/trunk/test/CodeGen/avx-builtins.c (original) +++ cfe/trunk/test/CodeGen/avx-builtins.c Thu Aug 4 01:02:50 2016 @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx -emit-llvm -o - -Werror | FileCheck %s -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx -emit-llvm -o - -Wall -Werror | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx -fno-signed-char -emit-llvm -o - -Wall -Werror | FileCheck %s // Don't include mm_malloc.h, it's system specific. #define __MM_MALLOC_H Modified: cfe/trunk/test/CodeGen/avx2-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx2-builtins.c?rev=277696&r1=277695&r2=277696&view=diff == --- cfe/trunk/test/CodeGen/avx2-builtins.c (original) +++ cfe/trunk/test/CodeGen/avx2-builtins.c Thu Aug 4 01:02:50 2016 @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 -emit-llvm -o - -Werror | FileCheck %s -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 -emit-llvm -o - -Wall -Werror | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 -fno-signed-char -emit-llvm -o - -Wall -Werror | FileCheck %s // Don't include mm_malloc.h, it's system specific. #define __MM_MALLOC_H Modified: cfe/trunk/test/CodeGen/avx512bw-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512bw-builtins.c?rev=277696&r1=277695&r2=277696&view=diff =
Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy
Alexander_Droste updated this revision to Diff 66760.
Alexander_Droste added a comment.
- clarify lambda argument with comment
- explicit cast to `int` for the number of indirections in for loop
https://reviews.llvm.org/D22729
Files:
clang-tidy/mpi/BufferDerefCheck.cpp
clang-tidy/mpi/BufferDerefCheck.h
clang-tidy/mpi/CMakeLists.txt
clang-tidy/mpi/MPITidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/mpi-buffer-deref.rst
test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
test/clang-tidy/mpi-buffer-deref.cpp
Index: test/clang-tidy/mpi-buffer-deref.cpp
===
--- /dev/null
+++ test/clang-tidy/mpi-buffer-deref.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s mpi-buffer-deref %t -- -- -I %S/Inputs/mpi-type-mismatch
+
+#include "mpimock.h"
+
+void negativeTests() {
+ char *buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer [mpi-buffer-deref]
+
+ unsigned **buf2;
+ MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer
+
+ short buf3[1][1];
+ MPI_Send(buf3, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: array->array
+
+ long double _Complex *buf4[1];
+ MPI_Send(buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array
+
+ std::complex *buf5[1][1];
+ MPI_Send(&buf5, 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array->array->pointer
+}
+
+void positiveTests() {
+ char buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+ unsigned *buf2;
+ MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+
+ short buf3[1][1];
+ MPI_Send(buf3[0], 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+ long double _Complex *buf4[1];
+ MPI_Send(*buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ long double _Complex buf5[1];
+ MPI_Send(buf5, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ std::complex *buf6[1][1];
+ MPI_Send(*buf6[0], 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ // Referencing an array with '&' is valid, as this also points to the
+ // beginning of the array.
+ long double _Complex buf7[1];
+ MPI_Send(&buf7, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+}
Index: test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
===
--- test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
+++ test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
@@ -24,13 +24,15 @@
#define MPI_DATATYPE_NULL 0
#define MPI_CHAR 0
#define MPI_BYTE 0
+#define MPI_SHORT 0
#define MPI_INT 0
#define MPI_LONG 0
#define MPI_LONG_DOUBLE 0
#define MPI_UNSIGNED 0
#define MPI_INT8_T 0
#define MPI_UINT8_T 0
#define MPI_UINT16_T 0
+#define MPI_C_FLOAT_COMPLEX 0
#define MPI_C_LONG_DOUBLE_COMPLEX 0
#define MPI_FLOAT 0
#define MPI_DOUBLE 0
Index: docs/clang-tidy/checks/mpi-buffer-deref.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/mpi-buffer-deref.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - mpi-buffer-deref
+
+mpi-buffer-deref
+
+
+This check verifies if a buffer passed to an MPI (Message Passing Interface)
+function is sufficiently dereferenced. Buffers should be passed as a single
+pointer or array. As MPI function signatures specify ``void *`` for their buffer
+types, insufficiently dereferenced buffers can be passed, like for example as
+double pointers or multidimensional arrays, without a compiler warning emitted.
+
+Examples:
+.. code:: c++
+ // A double pointer is passed to the MPI function.
+ char *buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+ // A multidimensional array is passed to the MPI function.
+ short buf[1][1];
+ MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+ // A pointer to an array is passed to the MPI function.
+ short *buf[1];
+ MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -110,6 +110,7 @@
modernize-use-nullptr
modernize-use-override
modernize-use-using
+ mpi-buffer-deref
mpi-type-mismatch
performance-faster-string-find
performance-for-range-copy
Index: clang-tidy/mpi/MPITidyModule.cpp
===
--- clang-tidy/mpi/MPITidyModule.cpp
+++ clang-tidy/mpi/MPITidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#i
Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy
Alexander_Droste updated this revision to Diff 66761.
Alexander_Droste added a comment.
- add needed extra line for docs
https://reviews.llvm.org/D22729
Files:
clang-tidy/mpi/BufferDerefCheck.cpp
clang-tidy/mpi/BufferDerefCheck.h
clang-tidy/mpi/CMakeLists.txt
clang-tidy/mpi/MPITidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/mpi-buffer-deref.rst
test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
test/clang-tidy/mpi-buffer-deref.cpp
Index: test/clang-tidy/mpi-buffer-deref.cpp
===
--- /dev/null
+++ test/clang-tidy/mpi-buffer-deref.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s mpi-buffer-deref %t -- -- -I %S/Inputs/mpi-type-mismatch
+
+#include "mpimock.h"
+
+void negativeTests() {
+ char *buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer [mpi-buffer-deref]
+
+ unsigned **buf2;
+ MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer
+
+ short buf3[1][1];
+ MPI_Send(buf3, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: array->array
+
+ long double _Complex *buf4[1];
+ MPI_Send(buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array
+
+ std::complex *buf5[1][1];
+ MPI_Send(&buf5, 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array->array->pointer
+}
+
+void positiveTests() {
+ char buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+ unsigned *buf2;
+ MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+
+ short buf3[1][1];
+ MPI_Send(buf3[0], 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+ long double _Complex *buf4[1];
+ MPI_Send(*buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ long double _Complex buf5[1];
+ MPI_Send(buf5, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ std::complex *buf6[1][1];
+ MPI_Send(*buf6[0], 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+ // Referencing an array with '&' is valid, as this also points to the
+ // beginning of the array.
+ long double _Complex buf7[1];
+ MPI_Send(&buf7, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+}
Index: test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
===
--- test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
+++ test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
@@ -24,13 +24,15 @@
#define MPI_DATATYPE_NULL 0
#define MPI_CHAR 0
#define MPI_BYTE 0
+#define MPI_SHORT 0
#define MPI_INT 0
#define MPI_LONG 0
#define MPI_LONG_DOUBLE 0
#define MPI_UNSIGNED 0
#define MPI_INT8_T 0
#define MPI_UINT8_T 0
#define MPI_UINT16_T 0
+#define MPI_C_FLOAT_COMPLEX 0
#define MPI_C_LONG_DOUBLE_COMPLEX 0
#define MPI_FLOAT 0
#define MPI_DOUBLE 0
Index: docs/clang-tidy/checks/mpi-buffer-deref.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/mpi-buffer-deref.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - mpi-buffer-deref
+
+mpi-buffer-deref
+
+
+This check verifies if a buffer passed to an MPI (Message Passing Interface)
+function is sufficiently dereferenced. Buffers should be passed as a single
+pointer or array. As MPI function signatures specify ``void *`` for their buffer
+types, insufficiently dereferenced buffers can be passed, like for example as
+double pointers or multidimensional arrays, without a compiler warning emitted.
+
+Examples:
+.. code:: c++
+
+ // A double pointer is passed to the MPI function.
+ char *buf;
+ MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+ // A multidimensional array is passed to the MPI function.
+ short buf[1][1];
+ MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+ // A pointer to an array is passed to the MPI function.
+ short *buf[1];
+ MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -110,6 +110,7 @@
modernize-use-nullptr
modernize-use-override
modernize-use-using
+ mpi-buffer-deref
mpi-type-mismatch
performance-faster-string-find
performance-for-range-copy
Index: clang-tidy/mpi/MPITidyModule.cpp
===
--- clang-tidy/mpi/MPITidyModule.cpp
+++ clang-tidy/mpi/MPITidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "BufferDerefCheck.h"
#i
Re: [PATCH] D22943: [Driver] Add FIXME's where we can't use effective triples (NFC)
echristo added a comment. Seems reasonable to fix the tests? https://reviews.llvm.org/D22943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22944: [Driver] Replace one-off uses of default triples with effective triples (NFCI)
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. OK, please go back and document in ToolChain.h the difference between the Triple and the EffectiveTriple - at length, as a follow on patch. -eric https://reviews.llvm.org/D22944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22945: [Driver] Replace more uses of default triples with effective triples (NFCI)
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. OK. -eric https://reviews.llvm.org/D22945 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
