Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added inline comments.
Comment at: lib/Headers/intrin.h:504
@@ +503,3 @@
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
compnerd wrote:
> mstorsjo wrote:
> > compnerd wrote:
> > > Perhaps we should add static asserts that _BitPos is within limits for
> > > the signed shift.
> > Sure, although I guess that also goes for the existing inline functions as
> > well?
> >
> > Which kind of assert would be suitable for that here? As far as I see,
> > static_assert is C++ only, while this header also can be used from C.
> >
> > If I try to add _Static_assert, which is usable in C, I get the following
> > error when compiling:
> >
> > intrin.h:499:18: error:
> > static_assert expression is not an integral constant expression
> > _Static_assert(_BitPos < 32, "_BitPos out of range");
> >
> > This even when I don't actually use the inline function anywhere, just
> > including intrin.h.
> Yeah, we would have to use `_Static_assert`, but that requires C11. It is
> possible to emulate it, but probably not worth the effort. I am worried
> though that we could introduce undefined behavior with an incorrect parameter.
Sure, there's probably such a risk. But this issue already exists - an almost
identical _interlockedbittestandset function already exists in the header - I'm
just adding new copies of it with different atomic semantics (__ATOMIC_ACQUIRE
etc).
So I'd ask if we can deal with that issue separately from this patch.
https://reviews.llvm.org/D24609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24663: When replacements have the same offset, make replacements with smaller length order first in the set.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: lib/Tooling/Core/Replacement.cpp:407 @@ -409,3 +406,3 @@ bool Result = true; - for (Replacements::const_iterator I = Replaces.begin(), -E = Replaces.end(); + for (Replacements::const_reverse_iterator I = Replaces.rbegin(), +E = Replaces.rend(); Maybe use auto? https://reviews.llvm.org/D24663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24663: When replacements have the same offset, make replacements with smaller length order first in the set.
ioeric updated this revision to Diff 71729.
ioeric marked an inline comment as done.
ioeric added a comment.
- Use auto.
https://reviews.llvm.org/D24663
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,7 +431,8 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""),
SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
+ for (auto I = Replaces.rbegin(),
+E = Replaces.rend();
I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
Index: include/clang/Tooling/Core/Replacement.h
===
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &RHS) const {
return Replaces == RHS.Replaces;
}
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,7 +431,8 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""), SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
+ for (auto I = Replaces.rbegin(),
+E = Replaces.rend();
I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
Index: include/clang/Tooling/Core/Replacement.h
===
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &R
Re: [PATCH] D24663: When replacements have the same offset, make replacements with smaller length order first in the set.
ioeric updated this revision to Diff 71730.
ioeric added a comment.
- Format code properly.
https://reviews.llvm.org/D24663
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,8 +431,7 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""),
SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
Index: include/clang/Tooling/Core/Replacement.h
===
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &RHS) const {
return Replaces == RHS.Replaces;
}
Index: lib/Tooling/Core/Replacement.cpp
===
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,8 +431,7 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""), SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
Index: include/clang/Tooling/Core/Replacement.h
===
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &RHS) const {
retu
r281819 - When replacements have the same offset, make replacements with smaller length order first in the set.
Author: ioeric
Date: Sat Sep 17 07:26:42 2016
New Revision: 281819
URL: http://llvm.org/viewvc/llvm-project?rev=281819&view=rev
Log:
When replacements have the same offset, make replacements with smaller length
order first in the set.
Summary:
No behavioral change intended. The change makes iterating the replacements set
more intuitive in Replacements class implementation. Previously, insertion is
ordered before an deletion/replacement with the same offset, which is
counter-intuitive for implementation, especially for a followup patch to
support adding insertions around replacements.
With the current ordering, we only need to make `applyAllReplacements` iterate
the replacements set reversely when applying them so that deletion/replacement
is still applied before insertion with the same offset.
Reviewers: klimek, djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D24663
Modified:
cfe/trunk/include/clang/Tooling/Core/Replacement.h
cfe/trunk/lib/Tooling/Core/Replacement.cpp
Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=281819&r1=281818&r2=281819&view=diff
==
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Sat Sep 17 07:26:42 2016
@@ -151,6 +151,7 @@ class Replacements {
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@ class Replacements {
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &RHS) const {
return Replaces == RHS.Replaces;
}
Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=281819&r1=281818&r2=281819&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Sat Sep 17 07:26:42 2016
@@ -83,11 +83,8 @@ bool operator<(const Replacement &LHS, c
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@ unsigned Replacements::getShiftedCodePos
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,8 +431,7 @@ llvm::Expected applyAllRepl
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""),
SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24663: When replacements have the same offset, make replacements with smaller length order first in the set.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281819: When replacements have the same offset, make
replacements with smaller length… (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D24663?vs=71730&id=71731#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24663
Files:
cfe/trunk/include/clang/Tooling/Core/Replacement.h
cfe/trunk/lib/Tooling/Core/Replacement.cpp
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,8 +431,7 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""),
SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
Index: cfe/trunk/include/clang/Tooling/Core/Replacement.h
===
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_reverse_iterator const_reverse_iterator;
Replacements() = default;
@@ -193,6 +194,10 @@
const_iterator end() const { return Replaces.end(); }
+ const_reverse_iterator rbegin() const { return Replaces.rbegin(); }
+
+ const_reverse_iterator rend() const { return Replaces.rend(); }
+
bool operator==(const Replacements &RHS) const {
return Replaces == RHS.Replaces;
}
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -83,11 +83,8 @@
if (LHS.getOffset() != RHS.getOffset())
return LHS.getOffset() < RHS.getOffset();
- // Apply longer replacements first, specifically so that deletions are
- // executed before insertions. It is (hopefully) never the intention to
- // delete parts of newly inserted code.
if (LHS.getLength() != RHS.getLength())
-return LHS.getLength() > RHS.getLength();
+return LHS.getLength() < RHS.getLength();
if (LHS.getFilePath() != RHS.getFilePath())
return LHS.getFilePath() < RHS.getFilePath();
@@ -407,9 +404,7 @@
bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) {
bool Result = true;
- for (Replacements::const_iterator I = Replaces.begin(),
-E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
if (I->isApplicable()) {
Result = I->apply(Rewrite) && Result;
} else {
@@ -436,8 +431,7 @@
"", 0, llvm::MemoryBuffer::getMemBuffer(Code, ""));
FileID ID = SourceMgr.createFileID(Files.getFile(""), SourceLocation(),
clang::SrcMgr::C_User);
- for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end();
- I != E; ++I) {
+ for (auto I = Replaces.rbegin(), E = Replaces.rend(); I != E; ++I) {
Replacement Replace("", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
Index: cfe/trunk/include/clang/Tooling/Core/Replacement.h
===
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h
@@ -151,6 +151,7 @@
public:
typedef ReplacementsImpl::const_iterator const_iterator;
+ typedef ReplacementsImpl::const_revers
[PATCH] clang-format: Adding two new format options.
**SpacesAroundConditions** (``bool``)
If ``true``, spaces will be inserted around if/for/while conditions.
**SpacesAfterNot** (``bool``)
If ``true``, spaces will be inserted after ``!``.
---
docs/ClangFormatStyleOptions.rst | 6 ++
include/clang/Format/Format.h| 8
lib/Format/Format.cpp| 6 ++
lib/Format/TokenAnnotator.cpp| 14 +-
unittests/Format/FormatTest.cpp | 20
5 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/docs/ClangFormatStyleOptions.rst b/docs/ClangFormatStyleOptions.rst
index a548e83..fac5369 100644
--- a/docs/ClangFormatStyleOptions.rst
+++ b/docs/ClangFormatStyleOptions.rst
@@ -721,6 +721,12 @@ the configuration (without a prefix: ``Auto``).
**SpacesInSquareBrackets** (``bool``)
If ``true``, spaces will be inserted after ``[`` and before ``]``.
+**SpacesAroundConditions** (``bool``)
+ If ``true``, spaces will be inserted around if/for/while conditions.
+
+**SpacesAfterNot** (``bool``)
+ If ``true``, spaces will be inserted after ``!``.
+
**Standard** (``LanguageStandard``)
Format compatible with this standard, e.g. use ``A >``
instead of ``A>`` for ``LS_Cpp03``.
diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index 1ff305d..72bff2a 100644
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -597,6 +597,12 @@ struct FormatStyle {
/// \brief If ``true``, spaces will be inserted after ``[`` and before ``]``.
bool SpacesInSquareBrackets;
+ /// \brief If ``true``, spaces will be inserted around if/for/while
conditions.
+ bool SpacesAroundConditions;
+
+ /// \brief If ``true``, spaces will be inserted after ``!``.
+ bool SpacesAfterNot;
+
/// \brief Supported language standards.
enum LanguageStandard {
/// Use C++03-compatible syntax.
@@ -707,6 +713,8 @@ struct FormatStyle {
SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
SpacesInParentheses == R.SpacesInParentheses &&
SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
+ SpacesAfterNot == R.SpacesAfterNot &&
+ SpacesAroundConditions == R.SpacesAroundConditions &&
Standard == R.Standard && TabWidth == R.TabWidth &&
UseTab == R.UseTab;
}
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 32d6bb8..2e04e41 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -352,6 +352,10 @@ template <> struct MappingTraits {
Style.SpacesInCStyleCastParentheses);
IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
+IO.mapOptional("SpacesAfterNot",
+ Style.SpacesAfterNot);
+IO.mapOptional("SpacesAroundConditions",
+ Style.SpacesAroundConditions);
IO.mapOptional("Standard", Style.Standard);
IO.mapOptional("TabWidth", Style.TabWidth);
IO.mapOptional("UseTab", Style.UseTab);
@@ -556,6 +560,8 @@ FormatStyle getLLVMStyle() {
LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
LLVMStyle.SpaceBeforeAssignmentOperators = true;
LLVMStyle.SpacesInAngles = false;
+ LLVMStyle.SpacesAfterNot = false;
+ LLVMStyle.SpacesAroundConditions = false;
LLVMStyle.PenaltyBreakComment = 300;
LLVMStyle.PenaltyBreakFirstLessLess = 120;
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 4a90522..5861930 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -1976,6 +1976,16 @@ bool TokenAnnotator::spaceRequiredBetween(const
AnnotatedLine &Line,
return Right.is(tok::hash);
if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
return Style.SpaceInEmptyParentheses;
+ if (Style.SpacesAroundConditions) {
+if (Left.is(tok::l_paren) && Left.Previous &&
+ Left.Previous->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for,
tok::kw_while,
+ tok::kw_switch, TT_ForEachMacro))
+return true;
+if (Right.is(tok::r_paren) && Right.MatchingParen &&
Right.MatchingParen->Previous &&
+ Right.MatchingParen->Previous->isOneOf(tok::kw_if, tok::pp_elif,
tok::kw_for, tok::kw_while,
+ tok::kw_switch, TT_ForEachMacro))
+return true;
+ }
if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
return (Right.is(TT_CastRParen) ||
(Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
@@ -2196,6 +2206,8 @@ bool TokenAnnotator::spaceRequiredBefore(const
AnnotatedLine &Line,
return Style.SpacesInContainerLiterals;
return true;
}
+ if (Style.SpacesAfterNot && Left.is(tok::exclaim))
+return true;
if (Left.is(TT_UnaryOperator))
return Right.is(TT_BinaryOperator);
@@ -2214,7 +2226,7 @@ bool TokenAnnotator::spaceRequiredBefore(const
AnnotatedLine &Line,
if (!Style.SpaceBeforeAssignmentO
Re: [PATCH] D24361: hasDeclaration(qualType(...)) matcher should unwrap ElaboratedType and TemplateSpecializationType
lukasza added a comment.
Richard, could you please take a look?
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:750
@@ +749,3 @@
+else if (auto *ET = Node->getAs())
+ return matchesSpecialized(ET->getNamedType(), Finder, Builder);
+else if (auto *TST = Node->getAs())
There is also a question whether I should not only start supporting not only
qualType(hasDeclaration(...)) when qualType wraps an ElaboratedType node, but
also start supporting elaboratedType(hasDeclaration(...)).
Snippets of a patch that could achieve this (but then again - I am not sure if
this is desirable and/or necessary):
+++ include/clang/ASTMatchers/ASTMatchersInternal.h (working copy)
...
+ /// \brief Gets the QualType from ElaboratedType
+ /// and returns whether the inner matches on it.
+ bool matchesSpecialized(const ElaboratedType &Node,
+ ASTMatchFinder *Finder,
+ BoundNodesTreeBuilder *Builder) const {
+return matchesSpecialized(Node.getNamedType(), Finder, Builder);
+ }
+
...
/// \brief All types that are supported by HasDeclarationMatcher above.
-typedef TypeList HasDeclarationSupportedTypes;
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:753
@@ -749,2 +752,3 @@
+ return matchesSpecialized(*TST, Finder, Builder);
else if (auto *TT = Node->getAs())
return matchesDecl(TT->getDecl(), Finder, Builder);
I do notice that there are quite a few similar Node->getAs if statements
here. They could be replaced with something "clever" (and probably unreadable):
+++ include/clang/ASTMatchers/ASTMatchersInternal.h (working copy)
...
+ /// \brief Terminating case for recursion over template parameters
+ /// performed by matchesQualTypeAsAnyOf template.
+ template
+ bool matchesQualTypeAsAnyOf(const QualType &Node, ASTMatchFinder *Finder,
+ BoundNodesTreeBuilder *Builder) const {
+return false;
+ }
+
+ /// \brief Returns true if Node->getAs is not null and matches inner
+ /// matcher for any type U listed in template parameters.
+ template ::value, int>::type = 0>
+ bool matchesQualTypeAsAnyOf(const QualType &Node, ASTMatchFinder *Finder,
+ BoundNodesTreeBuilder *Builder) const {
+if (const U *u = Node->getAs())
+ return matchesSpecialized(*u, Finder, Builder);
+
+return matchesQualTypeAsAnyOf(Node, Finder, Builder);
+ }
+
...
-else if (auto *OCIT = Node->getAs())
- return matchesDecl(OCIT->getDecl(), Finder, Builder);
-else if (auto *UUT = Node->getAs())
- return matchesDecl(UUT->getDecl(), Finder, Builder);
-else if (auto *ICNT = Node->getAs())
- return matchesDecl(ICNT->getDecl(), Finder, Builder);
-return false;
+else if (auto *TD = Node->getAsTagDecl())
+ return matchesDecl(TD, Finder, Builder);
+
+return matchesQualTypeAsAnyOf<
+ArrayType, InjectedClassNameType, ObjCInterfaceType,
TemplateSpecializationType,
+TypedefType, UnresolvedUsingType, void>(
+Node, Finder, Builder);
Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2124
@@ +2123,3 @@
+ // hasDeclaration should see through:
+ // 1. from ElaboratedType (Namespace::MyTemplate) to
+ //TemplateSpecializationType (MyTemplate)
FWIW, I couldn't repro a similar issue (i.e. VarDecl or ParmVarDecl of a type
with user-provided declaration, but where hasDeclaration doesn't match
*anything*) for other types (i.e. AutoType or ArrayType) - only for
ElaboratedType and TemplateSpecializationType.
https://reviews.llvm.org/D24361
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r281826 - [clang-rename] Fix handling of unchanged files
Author: alexshap
Date: Sat Sep 17 12:08:47 2016
New Revision: 281826
URL: http://llvm.org/viewvc/llvm-project?rev=281826&view=rev
Log:
[clang-rename] Fix handling of unchanged files
Fix the output of clang-rename for the files without modifications.
Update the code in clang-reorder-fields/tool/ClangReorderFields.cpp
to avoid inconsistency.
Example:
a.h:
struct A {};
a.cpp:
#include "a.h"
int main() { return 0; }
Before the changes the output looks like this:
clang-rename -qualified-name=A -new-name=B a.cpp
Test plan: make -j8 check-clang-tools
Differential revision: https://reviews.llvm.org/D24634
Added:
clang-tools-extra/trunk/test/clang-rename/IncludeHeaderWithSymbol.cpp
clang-tools-extra/trunk/test/clang-rename/Inputs/HeaderWithSymbol.h
Modified:
clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp
Modified: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp?rev=281826&r1=281825&r2=281826&view=diff
==
--- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp Sat Sep 17
12:08:47 2016
@@ -222,7 +222,7 @@ int main(int argc, const char **argv) {
Tool.applyAllReplacements(Rewrite);
for (const auto &File : Files) {
const auto *Entry = FileMgr.getFile(File);
- auto ID = Sources.translateFile(Entry);
+ const auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
Rewrite.getEditBuffer(ID).write(outs());
}
}
Modified:
clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp?rev=281826&r1=281825&r2=281826&view=diff
==
--- clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp
(original)
+++ clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp
Sat Sep 17 12:08:47 2016
@@ -80,13 +80,8 @@ int main(int argc, const char **argv) {
for (const auto &File : Files) {
const auto *Entry = FileMgr.getFile(File);
-const auto ID = Sources.translateFile(Entry);
-// The method Rewriter::getRewriteBufferFor returns nullptr if
-// the file has not been changed.
-if (const auto *RB = Rewrite.getRewriteBufferFor(ID))
- RB->write(outs());
-else
- outs() << Sources.getMemoryBufferForFile(Entry)->getBuffer();
+const auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
+Rewrite.getEditBuffer(ID).write(outs());
}
return ExitCode;
Added: clang-tools-extra/trunk/test/clang-rename/IncludeHeaderWithSymbol.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/IncludeHeaderWithSymbol.cpp?rev=281826&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/IncludeHeaderWithSymbol.cpp
(added)
+++ clang-tools-extra/trunk/test/clang-rename/IncludeHeaderWithSymbol.cpp Sat
Sep 17 12:08:47 2016
@@ -0,0 +1,10 @@
+#include "Inputs/HeaderWithSymbol.h"
+
+int main() {
+ return 0; // CHECK: {{^ return 0;}}
+}
+
+// Test 1.
+// The file IncludeHeaderWithSymbol.cpp doesn't contain the symbol Foo
+// and is expected to be written to stdout without modifications
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | FileCheck %s
Added: clang-tools-extra/trunk/test/clang-rename/Inputs/HeaderWithSymbol.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/Inputs/HeaderWithSymbol.h?rev=281826&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/Inputs/HeaderWithSymbol.h (added)
+++ clang-tools-extra/trunk/test/clang-rename/Inputs/HeaderWithSymbol.h Sat Sep
17 12:08:47 2016
@@ -0,0 +1 @@
+struct Foo {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24626: [OpenCL] Diagnose assignment to dereference of half type pointer
Anastasia accepted this revision. Anastasia added a comment. LGTM! https://reviews.llvm.org/D24626 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24666: [OpenCL] Allow half type kernel argument when cl_khr_fp16 is enabled
Anastasia added inline comments. Comment at: lib/Sema/SemaDecl.cpp:7600 @@ +7599,3 @@ +// Do not diagnose half type since it is diagnosed as invalid argument +// type for any function eleswhere. +if (!PT->isHalfType()) -> elsewhere https://reviews.llvm.org/D24666 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24690: Replace __ANDROID__ with __BIONIC__.
EricWF added inline comments. Comment at: include/__config:340 @@ -339,3 +344,1 @@ #if !defined(_LIBCPP_HAS_MUSL_LIBC) -# include -#if __GLIBC_PREREQ(2, 15) What happened to this include? I believe it's needed to get `__GLIBC_PREREQ`. Repository: rL LLVM https://reviews.llvm.org/D24690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. In https://reviews.llvm.org/D18639#514991, @hfinkel wrote: > In https://reviews.llvm.org/D18639#491232, @mclow.lists wrote: > > > And is there any reason why `__libcpp_isinf` can't just return `false` for > > non-fp types? > > > For custom numeric types that have an isinf, etc. found by ADL, they should > continue to work. Do we already support custom numeric types? If so could you add a test for this under `test/libcxx`? Just a simple test case that instantiates the functions and shows it compiles. Comment at: include/cmath:593 @@ +592,3 @@ +_LIBCPP_ALWAYS_INLINE +typename std::enable_if::value, bool>::type +__libcpp_isnan(_A1 __lcpp_x) _NOEXCEPT nit: the `std::` qualifier on types is unnecessary. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r281845 - [libFuzzer] use 'if guard' instead of 'if guard >= 0' with trace-pc; change the guard type to intptr_t; use separate array for 8-bit counters
Author: kcc Date: Sat Sep 17 23:52:23 2016 New Revision: 281845 URL: http://llvm.org/viewvc/llvm-project?rev=281845&view=rev Log: [libFuzzer] use 'if guard' instead of 'if guard >= 0' with trace-pc; change the guard type to intptr_t; use separate array for 8-bit counters Modified: cfe/trunk/docs/SanitizerCoverage.rst Modified: cfe/trunk/docs/SanitizerCoverage.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SanitizerCoverage.rst?rev=281845&r1=281844&r2=281845&view=diff == --- cfe/trunk/docs/SanitizerCoverage.rst (original) +++ cfe/trunk/docs/SanitizerCoverage.rst Sat Sep 17 23:52:23 2016 @@ -331,10 +331,10 @@ on every edge: .. code-block:: none - if (guard_variable >= 0) + if (guard_variable) __sanitizer_cov_trace_pc_guard(&guard_variable) -Every edge will have its own 8-byte `guard_variable`. +Every edge will have its own `guard_variable` (uintptr_t). The compler will also insert a module constructor that will call @@ -342,7 +342,7 @@ The compler will also insert a module co // The guards are [start, stop). // This function may be called multiple times with the same values of start/stop. - __sanitizer_cov_trace_pc_guard_init(uint64_t *start, uint64_t *stop); + __sanitizer_cov_trace_pc_guard_init(uintptr_t *start, uintptr_t *stop); Similarly to `trace-pc,indirect-calls`, with `trace-pc-guards,indirect-calls` ``__sanitizer_cov_trace_pc_indirect(void *callee)`` will be inserted on every indirect call. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24690: Replace __ANDROID__ with __BIONIC__.
danalbert added a comment. In https://reviews.llvm.org/D24690#545523, @compnerd wrote: > So, the only thing that Im confused about is where does `__BIONIC__` get > defined? It's in Bionic's ``, which gets pulled in via ``. Comment at: include/__config:340 @@ -339,3 +344,1 @@ #if !defined(_LIBCPP_HAS_MUSL_LIBC) -# include -#if __GLIBC_PREREQ(2, 15) EricWF wrote: > What happened to this include? I believe it's needed to get `__GLIBC_PREREQ`. Included much earlier now (L90). We always needed this for Linux, and having it at the top of the file means we won't accidentally forget to include it for an earlier check. Comment at: include/__config:766 @@ -761,3 +765,3 @@ // Most unix variants have catopen. These are the specific ones that don't. -#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION) +#if !defined(_WIN32) && !defined(__BIONIC__) && !defined(_NEWLIB_VERSION) #define _LIBCPP_HAS_CATOPEN 1 compnerd wrote: > Not your fault, but `_WIN32` and `__unix__`? Im not sure if MinGW or cygwin > define both. That is odd. I would assume that `_WIN32` implies `!__unix__`. Repository: rL LLVM https://reviews.llvm.org/D24690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304
djasper added a comment. I think, this is the wrong fix. Instead, a || (Left.is(TT_TemplateOpener) && !Right.is(TT_TemplateCloser)) should be added to the last return statement of this function. Also, could you please add a test in unittests/Format/FormatTest.cpp. https://reviews.llvm.org/D24703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r281816 - clang-format: [JS] Fix a crash in handledTemplateStrings.
Author: djasper
Date: Sat Sep 17 02:20:36 2016
New Revision: 281816
URL: http://llvm.org/viewvc/llvm-project?rev=281816&view=rev
Log:
clang-format: [JS] Fix a crash in handledTemplateStrings.
Modified:
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=281816&r1=281815&r2=281816&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Sat Sep 17 02:20:36 2016
@@ -235,6 +235,8 @@ void FormatTokenLexer::handleTemplateStr
return;
}
if (BacktickToken->is(tok::r_brace)) {
+if (StateStack.size() == 1)
+ return;
StateStack.pop();
if (StateStack.top() != LexerState::TEMPLATE_STRING)
return;
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=281816&r1=281815&r2=281816&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Sat Sep 17 02:20:36 2016
@@ -1253,6 +1253,9 @@ TEST_F(FormatTestJS, NestedTemplateStrin
verifyFormat(
"var x = `${xs.map(x => `${x}`).join('\\n')}`;");
verifyFormat("var x = `he${({text: 'll'}.text)}o`;");
+
+ // Crashed at some point.
+ verifyFormat("}");
}
TEST_F(FormatTestJS, TaggedTemplateStrings) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks good! Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:291 @@ +290,3 @@ + static ProgramStateRef addExtentSize(CheckerContext &C, + const CXXNewExpr *NE, + ProgramStateRef State Whitespace a bit strange. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:997 @@ +996,3 @@ +// +ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C, + const CXXNewExpr *NE, >> Perhaps ExprEngine would be the proper place for that code as well. > Interesting point. Can you clarify the last sentence? I'm thinking that the standard operator new() should be properly modeled by the analyzer core; we are already doing this with respect to memory space of the region it returns, why not do that for extent as well, somewhere at the same place. We could probably make a refactoring pass over `MallocChecker` to move things around and return it to a readable state. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1011 @@ +1010,3 @@ +// containing the elements. +Region = (State->getSVal(NE, LCtx)) + .getAsRegion() dkrupp wrote: > MemRegion has now method called castAs<>, only getAs<>, so I stayed with it. Ouch, i meant, `cast(State->getSVal(NE, LCtx).getAsRegion())` etc. https://reviews.llvm.org/D24307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
