[PATCH] D40439: [Tooling] Remove file/command enumeration from CompilationDatabase.
sammccall added a comment. In https://reviews.llvm.org/D40439#935678, @bkramer wrote: > There are a few users of the C++ API out there, do we have migration path for > them? Do we have any idea what these look like? One option would be to keep the functionality on the JSONCompilationDatabase implementation (which properly supports it), and remove it from the interface. https://reviews.llvm.org/D40439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client
nik added a comment. Could you elaborate on "the client will be able to use it to generate a reproducer for the crash"? Having the json file, what would I need to do in order to reproduce the crash? Repository: rC Clang https://reviews.llvm.org/D40527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule
Quolyk created this revision.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D40543
Files:
include/clang/Tooling/Refactoring/Rename/RenamingAction.h
Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
private:
QualifiedRenameRule(const NamedDecl *ND,
- std::string NewQualifiedName)
+ const std::string &NewQualifiedName)
: ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
Expected
Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
private:
QualifiedRenameRule(const NamedDecl *ND,
- std::string NewQualifiedName)
+ const std::string &NewQualifiedName)
: ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
Expected
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule
hokein added inline comments.
Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
QualifiedRenameRule(const NamedDecl *ND,
- std::string NewQualifiedName)
+ const std::string &NewQualifiedName)
: ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
Passing `std::string` object is fine here -- because we use std::move below to
avoid the extra copy.
Is the warning caught by the clang-tidy check?
https://reviews.llvm.org/D40543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40072: [libclang] Support querying whether a declaration is invalid
nik added a comment. Ping https://reviews.llvm.org/D40072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
rjmccall accepted this revision. rjmccall added a comment. In https://reviews.llvm.org/D40275#937010, @tra wrote: > In https://reviews.llvm.org/D40275#933253, @tra wrote: > > > @rjmccall : are you OK with this approach? If VLA is not supported by the > > target, CUDA is handled as a special case so it can emit deferred diag, > > OpenMP reports an error only if shouldDiagnoseTargetSupportFromOpenMP() > > allows it, and everything else does so unconditionally. > > > @rjmccall : ping. Sorry for the delay; I took Thanksgiving week off. Yes, I think this patch is fine now, thanks. https://reviews.llvm.org/D40275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
tra wrote:
> tra wrote:
> > rjmccall wrote:
> > > Please write this check so that it trips in an "ordinary" build on a
> > > target that just happens to not support VLAs, something like:
> > >
> > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > shouldDiagnoseTargetSupportFromOpenMP())
> > >
> > > If you want to include the explicit OpenMP check there, it would need to
> > > be:
> > >
> > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > (!getLangOpts().OpenMP || shouldDiagnoseTargetSupportFromOpenMP()))
> > >
> > > but I think the first looks better.
> > >
> > > The CUDA and OpenMP paths here seem to be trying to achieve analogous
> > > things; it's unfortunate that we can't find a way to unify their
> > > approaches, even if we'd eventually want to use different diagnostic
> > > text. I imagine that the target-environment language restrictions are
> > > basically the same, since they arise for the same fundamental reasons, so
> > > all the places using CUDADiagIfDeviceCode are likely to have a check for
> > > shouldDiagnoseTargetSupportFromOpenMP() and vice-versa.
> > The problem is that in CUDA we can't just do
> > ```
> > if (!Context.getTargetInfo().isVLASupported() &&
> > shouldDiagnoseTargetSupportFromOpenMP())
> >Diag(Loc, diag::err_vla_unsupported);
> > ```
> >
> > In some situations diag messages will only be emitted if we attempt to
> > generate unsupported code on device side.
> > Consider
> > ```
> > __host__ __device__ void foo(int n) {
> > int vla[n];
> > }
> > ```
> >
> > When Sema sees this code during compilation, it can not tell whether there
> > is an error. Calling foo from the host code is perfectly valid. Calling it
> > from device code is not. `CUDADiagIfDeviceCode` creates 'postponed'
> > diagnostics which only gets emitted if we ever need to generate code for
> > the function on device.
> >
> > So, while CUDA and OpenMP do similar things, they are not quite the same.
> > If your goal to generalize CUDA and OpenMP handling, then it would have to
> > be folded into diagnostic-emitting itself and we'll need an analog of
> > CUDADiagIfDeviceCode which can handle both OpenMP and CUDA.
> > E.g. something like this:
> > ```
> > ??? DiagIfDeviceCode(???) {
> >if (OpenCL || (OpenMP && shouldDiagnoseTargetSupportFromOpenMP()))
> >Diag(...);
> >else if (CUDA)
> >CUDADiagIfDeviceCode()
> > }
> >
> > ...
> >
> > if (!Context.getTargetInfo().isVLASupported())
> >DiagIfDeviceCode();
> >
> > ```
> >
> > Would that work for you?
> There's another issue with this approach -- diagnostics itself. Each dialect
> has its own. Specifically CUDA diags have details that are relevant only to
> CUDA. I suspect OpenMP has something specific as well. If we insist emitting
> only one kind of error for particular case across all dialects, we'll have to
> stick to bare bones "feature X is not supported" which will not have
> sufficient details to explain why the error was triggered in CUDA.
>
> IMO dialect-specific handling of cuda errors in this case is the lesser evil.
>
> I'll update the patch to handle non-cuda cases the way you suggested.
If there really is interesting language-specific information to provide in a
diagnostic, I agree that it's hard to avoid having different code for different
targets. On the other hand, the CUDA-specific information in this specific
diagnostic seems unnecessary — does the user really care about whether the
function was '__device__' vs. '__host__ __device__'? in fact, isn't the latter
a bit misleading? — and I feel like a generic "cannot use variable-length
arrays when compiling for device 'nvptx64'" would be perfectly satisfactory in
both CUDA and OpenMP, and that's probably true for almost all of these
diagnostics.
On a completely different note, I do want to point out that the only thing you
actually *need* to ban here is declaring a local variable of VLA type. There's
no reason at all to ban VLA types in general; they just compile to extra
arithmetic.
https://reviews.llvm.org/D40275
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39903: [libclang] Allow pretty printing declarations
nik added a comment. Ping III - is there anything I can do to get this reviewed faster? 3 weeks passed. https://reviews.llvm.org/D39903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm Thanks for the rename! https://reviews.llvm.org/D40399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D40406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
sammccall added a comment.
Thanks for doing this!
Most of my comments are of the form "can we make this conceptually simpler, and
somewhat less awesome".
This is because it's a somewhat unusual/scary pattern, so I'd like the
implementation to be as small and transparent as possible.
Comment at: clangd/Context.cpp:16
+
+static Context *GlobalCtx = nullptr;
+static Context EmptyContext(nullptr, {});
Seems like it would simplify things if:
- GlobalCtx was always set (static local trick)
- GlobalSession swapped its context with Global, and then swapped back in its
destructor
Comment at: clangd/Context.h:64
+private:
+ Context *Parent;
+ TypedValueMap Data;
nit: const
Comment at: clangd/Context.h:65
+ Context *Parent;
+ TypedValueMap Data;
+};
We add complexity here (implementation and conceptual) to allow multiple
properties to be set at the same level (vs having a key and an AnyStorage and
making Context a linked list).
Is this for performance? I'm not convinced it'll actually be faster for our
workloads, or that it matters.
Comment at: clangd/Context.h:71
+/// before any clangd functions are called.
+class GlobalSession {
+public:
Maybe `WithGlobalContext` is a good name for this scoped object?
This comment doesn't give a clear idea why someone would want to call this.
Maybe `All contexts used by clangd inherit from this global context (including
contexts created internally)`
Comment at: clangd/Context.h:79
+/// Otherwise returns an empty Context.
+Context &globalCtx();
+
ilya-biryukov wrote:
> bkramer wrote:
> > This is a giant code smell. If we want the context route, please pass
> > contexts everywhere. I really don't want this kind of technical debt in
> > clangd now.
> I'm with you on this one, but I think @sammccall was keen on having the
> global context. The main reason was to always have access to **some** loggers
> and tracers, even when it's hard to pass the Context into the function.
> It's perfectly easy to remove all usages of `globalCtx()`, currently only 8
> usages to get rid of. However, I'd wait for Sam's comment before doing that.
It's important to be able to call functions that require a context if you don't
have one - adding a log statement/trace for debugging shouldn't require
changing plumbing/interfaces. (It's fine if we want to avoid checking in such
code...)
Having an "empty" global context allows this.
At the same time, we want the ability to set the sink for logs/traces etc
globally.
A couple of options:
- the sink (e.g. Logger) is part of the context, we need to allow embedders to
set/augment the global context
- the sink is not stored in the context, instead it is some other singleton
the embedder can set up
I don't have a strong opinion which is better. It's nice to reuse mechanisms,
on the other hand loggers vs request IDs are pretty different types of data.
Comment at: clangd/Context.h:116
+/// Creates a new ContextBuilder, using globalCtx() as a parent.
+ContextBuilder buildCtx();
+/// Creates a new ContextBuilder with explicit \p Parent.
This seems more naturally a method on Context, e.g.
Context C = globalCtx().derive(key, value);
The relationship between global and C is clear.
(You can allow chaining+mapping by having Context::derive and
ContextBuilder::derive both return ContextBuilder&&, but as noted below I'm not
sure it's worth the complexity over Context::derive ->Context)
Comment at: clangd/TypedValueMap.h:1
+//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===//
+//
This might be doing a little more than it needs to.
Do we need the ability to have multiple values of the same type?
If request ID is an int, needing to do `struct RequestID { ID int };` doesn't
seem like much of a burden, and simplifies the semantics here.
And for cases like Logger where the type carries the semantics, keying by type
is clearer.
Comment at: clangd/TypedValueMap.h:76
+
+ template bool emplace(PtrKey &PtrKey, Arg *A) {
+return emplace(PtrKey.UnderlyingKey, A);
Shouldn't this just be Type *A?
I think the extra convenience of PtrKey probably isn't worth the complexity,
though.
https://reviews.llvm.org/D40485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r319157 - [clangd] Add missing (but documented!) JSONExpr typed accessors
Author: sammccall
Date: Tue Nov 28 01:25:09 2017
New Revision: 319157
URL: http://llvm.org/viewvc/llvm-project?rev=319157&view=rev
Log:
[clangd] Add missing (but documented!) JSONExpr typed accessors
Summary:
Noticed this when I tried to port the Protocol.h parsers.
And tests for the inspect API, which caught a small bug.
Reviewers: ioeric
Subscribers: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D40399
Modified:
clang-tools-extra/trunk/clangd/JSONExpr.cpp
clang-tools-extra/trunk/clangd/JSONExpr.h
clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp
Modified: clang-tools-extra/trunk/clangd/JSONExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONExpr.cpp?rev=319157&r1=319156&r2=319157&view=diff
==
--- clang-tools-extra/trunk/clangd/JSONExpr.cpp (original)
+++ clang-tools-extra/trunk/clangd/JSONExpr.cpp Tue Nov 28 01:25:09 2017
@@ -161,7 +161,7 @@ bool Parser::parseExpr(Expr &Out) {
}
case '[': {
Out = json::ary{};
-json::ary &A = *Out.array();
+json::ary &A = *Out.asArray();
eatWhitespace();
if (peek() == ']') {
++P;
@@ -185,7 +185,7 @@ bool Parser::parseExpr(Expr &Out) {
}
case '{': {
Out = json::obj{};
-json::obj &O = *Out.object();
+json::obj &O = *Out.asObject();
eatWhitespace();
if (peek() == '}') {
++P;
@@ -507,17 +507,17 @@ bool operator==(const Expr &L, const Exp
return false;
switch (L.kind()) {
case Expr::Null:
-return L.null() == R.null();
+return *L.asNull() == *R.asNull();
case Expr::Boolean:
-return L.boolean() == R.boolean();
+return *L.asBoolean() == *R.asBoolean();
case Expr::Number:
-return L.boolean() == R.boolean();
+return *L.asNumber() == *R.asNumber();
case Expr::String:
-return L.string() == R.string();
+return *L.asString() == *R.asString();
case Expr::Array:
-return *L.array() == *R.array();
+return *L.asArray() == *R.asArray();
case Expr::Object:
-return *L.object() == *R.object();
+return *L.asObject() == *R.asObject();
}
llvm_unreachable("Unknown expression kind");
}
Modified: clang-tools-extra/trunk/clangd/JSONExpr.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONExpr.h?rev=319157&r1=319156&r2=319157&view=diff
==
--- clang-tools-extra/trunk/clangd/JSONExpr.h (original)
+++ clang-tools-extra/trunk/clangd/JSONExpr.h Tue Nov 28 01:25:09 2017
@@ -55,14 +55,14 @@ namespace json {
// object (json::obj)
//
// The kind can be queried directly, or implicitly via the typed accessors:
-// if (Optional S = E.string())
+// if (Optional S = E.asString()
// assert(E.kind() == Expr::String);
//
// Array and Object also have typed indexing accessors for easy traversal:
// Expected E = parse(R"( {"options": {"font": "sans-serif"}} )");
-// if (json::obj* O = E->object())
-// if (json::obj* Opts = O->object("options"))
-// if (Optional Font = Opts->string("font"))
+// if (json::obj* O = E->asObject())
+// if (json::obj* Opts = O->getObject("options"))
+// if (Optional Font = Opts->getString("font"))
// assert(Opts->at("font").kind() == Expr::String);
//
// === Serialization ===
@@ -166,38 +166,38 @@ public:
}
// Typed accessors return None/nullptr if the Expr is not of this type.
- llvm::Optional null() const {
+ llvm::Optional asNull() const {
if (LLVM_LIKELY(Type == T_Null))
return nullptr;
return llvm::None;
}
- llvm::Optional boolean() const {
-if (LLVM_LIKELY(Type == T_Null))
+ llvm::Optional asBoolean() const {
+if (LLVM_LIKELY(Type == T_Boolean))
return as();
return llvm::None;
}
- llvm::Optional number() const {
+ llvm::Optional asNumber() const {
if (LLVM_LIKELY(Type == T_Number))
return as();
return llvm::None;
}
- llvm::Optional string() const {
+ llvm::Optional asString() const {
if (Type == T_String)
return llvm::StringRef(as());
if (LLVM_LIKELY(Type == T_StringRef))
return as();
return llvm::None;
}
- const ObjectExpr *object() const {
+ const ObjectExpr *asObject() const {
return LLVM_LIKELY(Type == T_Object) ? &as() : nullptr;
}
- ObjectExpr *object() {
+ ObjectExpr *asObject() {
return LLVM_LIKELY(Type == T_Object) ? &as() : nullptr;
}
- const ArrayExpr *array() const {
+ const ArrayExpr *asArray() const {
return LLVM_LIKELY(Type == T_Array) ? &as() : nullptr;
}
- ArrayExpr *array() {
+ ArrayExpr *asArray() {
return LLVM_LIKELY(Type == T_Array) ? &as() : nullptr;
}
@@ -292,6 +292,63 @@ public:
Expr &operator[](ObjectKey &&K) {
return emplace(std::move(K), Expr(nullptr)).first->second;
}
+
+// Look up a property, returning nullptr if it doesn't exist.
[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE319157: [clangd] Add missing (but documented!) JSONExpr
typed accessors (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D40399?vs=124204&id=124528#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40399
Files:
clangd/JSONExpr.cpp
clangd/JSONExpr.h
unittests/clangd/JSONExprTests.cpp
Index: clangd/JSONExpr.cpp
===
--- clangd/JSONExpr.cpp
+++ clangd/JSONExpr.cpp
@@ -161,7 +161,7 @@
}
case '[': {
Out = json::ary{};
-json::ary &A = *Out.array();
+json::ary &A = *Out.asArray();
eatWhitespace();
if (peek() == ']') {
++P;
@@ -185,7 +185,7 @@
}
case '{': {
Out = json::obj{};
-json::obj &O = *Out.object();
+json::obj &O = *Out.asObject();
eatWhitespace();
if (peek() == '}') {
++P;
@@ -507,17 +507,17 @@
return false;
switch (L.kind()) {
case Expr::Null:
-return L.null() == R.null();
+return *L.asNull() == *R.asNull();
case Expr::Boolean:
-return L.boolean() == R.boolean();
+return *L.asBoolean() == *R.asBoolean();
case Expr::Number:
-return L.boolean() == R.boolean();
+return *L.asNumber() == *R.asNumber();
case Expr::String:
-return L.string() == R.string();
+return *L.asString() == *R.asString();
case Expr::Array:
-return *L.array() == *R.array();
+return *L.asArray() == *R.asArray();
case Expr::Object:
-return *L.object() == *R.object();
+return *L.asObject() == *R.asObject();
}
llvm_unreachable("Unknown expression kind");
}
Index: clangd/JSONExpr.h
===
--- clangd/JSONExpr.h
+++ clangd/JSONExpr.h
@@ -55,14 +55,14 @@
// object (json::obj)
//
// The kind can be queried directly, or implicitly via the typed accessors:
-// if (Optional S = E.string())
+// if (Optional S = E.asString()
// assert(E.kind() == Expr::String);
//
// Array and Object also have typed indexing accessors for easy traversal:
// Expected E = parse(R"( {"options": {"font": "sans-serif"}} )");
-// if (json::obj* O = E->object())
-// if (json::obj* Opts = O->object("options"))
-// if (Optional Font = Opts->string("font"))
+// if (json::obj* O = E->asObject())
+// if (json::obj* Opts = O->getObject("options"))
+// if (Optional Font = Opts->getString("font"))
// assert(Opts->at("font").kind() == Expr::String);
//
// === Serialization ===
@@ -166,38 +166,38 @@
}
// Typed accessors return None/nullptr if the Expr is not of this type.
- llvm::Optional null() const {
+ llvm::Optional asNull() const {
if (LLVM_LIKELY(Type == T_Null))
return nullptr;
return llvm::None;
}
- llvm::Optional boolean() const {
-if (LLVM_LIKELY(Type == T_Null))
+ llvm::Optional asBoolean() const {
+if (LLVM_LIKELY(Type == T_Boolean))
return as();
return llvm::None;
}
- llvm::Optional number() const {
+ llvm::Optional asNumber() const {
if (LLVM_LIKELY(Type == T_Number))
return as();
return llvm::None;
}
- llvm::Optional string() const {
+ llvm::Optional asString() const {
if (Type == T_String)
return llvm::StringRef(as());
if (LLVM_LIKELY(Type == T_StringRef))
return as();
return llvm::None;
}
- const ObjectExpr *object() const {
+ const ObjectExpr *asObject() const {
return LLVM_LIKELY(Type == T_Object) ? &as() : nullptr;
}
- ObjectExpr *object() {
+ ObjectExpr *asObject() {
return LLVM_LIKELY(Type == T_Object) ? &as() : nullptr;
}
- const ArrayExpr *array() const {
+ const ArrayExpr *asArray() const {
return LLVM_LIKELY(Type == T_Array) ? &as() : nullptr;
}
- ArrayExpr *array() {
+ ArrayExpr *asArray() {
return LLVM_LIKELY(Type == T_Array) ? &as() : nullptr;
}
@@ -292,6 +292,63 @@
Expr &operator[](ObjectKey &&K) {
return emplace(std::move(K), Expr(nullptr)).first->second;
}
+
+// Look up a property, returning nullptr if it doesn't exist.
+json::Expr *get(const ObjectKey &K) {
+ auto I = find(K);
+ if (I == end())
+return nullptr;
+ return &I->second;
+}
+const json::Expr *get(const ObjectKey &K) const {
+ auto I = find(K);
+ if (I == end())
+return nullptr;
+ return &I->second;
+}
+// Typed accessors return None/nullptr if
+// - the property doesn't exist
+// - or it has the wrong type
+llvm::Optional getNull(const ObjectKey &K) const {
+ if (auto *V = get(K))
+return V->asNull();
+ return llvm::None;
+}
+llvm::Optional getBoolean(const ObjectKey &K) const {
+ if (auto *V = get(K))
+return V->asBoolean();
+ return llvm::None;
+}
+llvm::Optional getNumber(const ObjectKey &K) const {
+ if (aut
[PATCH] D40528: add new check to find NSError init invocation
hokein added inline comments. Comment at: docs/clang-tidy/checks/objc-avoid-nserror-init.rst:10 +``errorWithDomain:code:userInfo:`` to create new NSError objects instead +of ``[NSError alloc] init]``. Otherwise it will lead to a warning message +during compilation in Xcode. What's the warning message in Xcode? I suspect whether there is a diagnostic flag in clang already. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE319159: [clangd] Switch from YAMLParser to JSONExpr
(authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D40406?vs=124163&id=124529#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40406
Files:
clangd/ClangdLSPServer.cpp
clangd/JSONExpr.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/Trace.cpp
test/clangd/authority-less-uri.test
test/clangd/definitions.test
test/clangd/diagnostics.test
test/clangd/did-change-watch-files.test
test/clangd/protocol.test
unittests/clangd/JSONExprTests.cpp
Index: unittests/clangd/JSONExprTests.cpp
===
--- unittests/clangd/JSONExprTests.cpp
+++ unittests/clangd/JSONExprTests.cpp
@@ -205,6 +205,7 @@
EXPECT_TRUE(O->getNull("null"));
EXPECT_EQ(O->getNumber("number"), llvm::Optional(2.78));
+ EXPECT_FALSE(O->getInteger("number"));
EXPECT_EQ(O->getString("string"), llvm::Optional("json"));
ASSERT_FALSE(O->getObject("missing"));
ASSERT_FALSE(O->getObject("array"));
@@ -216,6 +217,7 @@
EXPECT_EQ(A->getBoolean(1), llvm::Optional(true));
ASSERT_TRUE(A->getArray(4));
EXPECT_EQ(*A->getArray(4), (ary{1, 2, 3}));
+ EXPECT_EQ(A->getArray(4)->getInteger(1), llvm::Optional(2));
int I = 0;
for (Expr &E : *A) {
if (I++ == 5) {
Index: clangd/JSONRPCDispatcher.h
===
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -17,7 +17,6 @@
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
-#include "llvm/Support/YAMLParser.h"
#include
#include
@@ -30,7 +29,7 @@
public:
JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs,
llvm::raw_ostream *InputMirror = nullptr, bool Pretty = false)
- : Outs(Outs), Logs(Logs), InputMirror(InputMirror), Pretty(Pretty) {}
+ : Pretty(Pretty), Outs(Outs), Logs(Logs), InputMirror(InputMirror) {}
/// Emit a JSONRPC message.
void writeMessage(const json::Expr &Result);
@@ -44,11 +43,13 @@
/// Unlike other methods of JSONOutput, mirrorInput is not thread-safe.
void mirrorInput(const Twine &Message);
+ // Whether output should be pretty-printed.
+ const bool Pretty;
+
private:
llvm::raw_ostream &Outs;
llvm::raw_ostream &Logs;
llvm::raw_ostream *InputMirror;
- bool Pretty;
std::mutex StreamMutex;
};
@@ -84,8 +85,7 @@
class JSONRPCDispatcher {
public:
// A handler responds to requests for a particular method name.
- using Handler =
- std::function;
+ using Handler = std::function;
/// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
/// method is received.
@@ -96,7 +96,7 @@
void registerHandler(StringRef Method, Handler H);
/// Parses a JSONRPC message and calls the Handler for it.
- bool call(StringRef Content, JSONOutput &Out) const;
+ bool call(const json::Expr &Message, JSONOutput &Out) const;
private:
llvm::StringMap Handlers;
Index: clangd/JSONRPCDispatcher.cpp
===
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -14,7 +14,6 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/SourceMgr.h"
-#include "llvm/Support/YAMLParser.h"
#include
using namespace clang;
@@ -101,88 +100,29 @@
Handlers[Method] = std::move(H);
}
-static void
-callHandler(const llvm::StringMap &Handlers,
-llvm::yaml::ScalarNode *Method, llvm::Optional ID,
-llvm::yaml::MappingNode *Params,
-const JSONRPCDispatcher::Handler &UnknownHandler, JSONOutput &Out) {
- llvm::SmallString<64> MethodStorage;
- llvm::StringRef MethodStr = Method->getValue(MethodStorage);
- auto I = Handlers.find(MethodStr);
- auto &Handler = I != Handlers.end() ? I->second : UnknownHandler;
- Handler(RequestContext(Out, MethodStr, std::move(ID)), Params);
-}
-
-bool JSONRPCDispatcher::call(StringRef Content, JSONOutput &Out) const {
- llvm::SourceMgr SM;
- llvm::yaml::Stream YAMLStream(Content, SM);
-
- auto Doc = YAMLStream.begin();
- if (Doc == YAMLStream.end())
+bool JSONRPCDispatcher::call(const json::Expr &Message, JSONOutput &Out) const {
+ // Message must be an object with "jsonrpc":"2.0".
+ auto *Object = Message.asObject();
+ if (!Object || Object->getString("jsonrpc") != Optional("2.0"))
return false;
-
- auto *Object = dyn_cast_or_null(Doc->getRoot());
- if (!Object)
-return false;
-
- llvm::yaml::ScalarNode *Version = nullptr;
- llvm::yaml::ScalarNode *Method = nullptr;
- llvm::yaml::MappingNode *Params = nullptr;
+ // ID may be any JSON value. If absent, this is a notification.
llvm::Optional ID;
- for (auto &NextKey
[clang-tools-extra] r319159 - [clangd] Switch from YAMLParser to JSONExpr
Author: sammccall
Date: Tue Nov 28 01:37:43 2017
New Revision: 319159
URL: http://llvm.org/viewvc/llvm-project?rev=319159&view=rev
Log:
[clangd] Switch from YAMLParser to JSONExpr
Summary:
- Converted Protocol.h parse() functions to take JSON::Expr.
These no longer detect and log unknown fields, as this is not that
useful and no longer free.
I haven't changed the error handling too much: fields that were
treated as optional before are still optional, even when it's wrong.
Exception: object properties with the wrong type are now ignored.
- Made JSONRPCDispatcher parse using json::parse
- The bug where 'method' must come before 'params' in the stream is
fixed as a side-effect. (And the same bug in executeCommand).
- Some parser crashers fixed as a side effect.
e.g. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3890
- The debug stream now prettyprints the input messages with --pretty.
- Request params are attached to traces when tracing is enabled.
- Fixed some bugs in tests (errors tolerated by YAMLParser, and
off-by-ones in Content-Length that our null-termination was masking)
- Fixed a random double-escape bug in ClangdLSPServer (it was our last
use of YAMLParser!)
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D40406
Removed:
clang-tools-extra/trunk/test/clangd/did-change-watch-files.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/JSONExpr.h
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/Trace.cpp
clang-tools-extra/trunk/test/clangd/authority-less-uri.test
clang-tools-extra/trunk/test/clangd/definitions.test
clang-tools-extra/trunk/test/clangd/diagnostics.test
clang-tools-extra/trunk/test/clangd/protocol.test
clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=319159&r1=319158&r2=319159&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Nov 28 01:37:43 2017
@@ -139,7 +139,7 @@ void ClangdLSPServer::onRename(Ctx C, Re
std::string Code = Server.getDocument(File);
std::vector Edits = replacementsToEdits(Code, *Replacements);
WorkspaceEdit WE;
- WE.changes = {{llvm::yaml::escape(Params.textDocument.uri.uri), Edits}};
+ WE.changes = {{Params.textDocument.uri.uri, Edits}};
C.reply(WorkspaceEdit::unparse(WE));
}
@@ -249,7 +249,7 @@ bool ClangdLSPServer::run(std::istream &
// Set up JSONRPCDispatcher.
JSONRPCDispatcher Dispatcher(
- [](RequestContext Ctx, llvm::yaml::MappingNode *Params) {
+ [](RequestContext Ctx, const json::Expr &Params) {
Ctx.replyError(ErrorCode::MethodNotFound, "method not found");
});
registerCallbackHandlers(Dispatcher, Out, /*Callbacks=*/*this);
Modified: clang-tools-extra/trunk/clangd/JSONExpr.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONExpr.h?rev=319159&r1=319158&r2=319159&view=diff
==
--- clang-tools-extra/trunk/clangd/JSONExpr.h (original)
+++ clang-tools-extra/trunk/clangd/JSONExpr.h Tue Nov 28 01:37:43 2017
@@ -181,6 +181,16 @@ public:
return as();
return llvm::None;
}
+ llvm::Optional asInteger() const {
+if (LLVM_LIKELY(Type == T_Number)) {
+ double D = as();
+ if (LLVM_LIKELY(std::modf(D, &D) == 0 &&
+ D >= std::numeric_limits::min() &&
+ D <= std::numeric_limits::max()))
+return D;
+}
+return llvm::None;
+ }
llvm::Optional asString() const {
if (Type == T_String)
return llvm::StringRef(as());
@@ -324,6 +334,11 @@ public:
return V->asNumber();
return llvm::None;
}
+llvm::Optional getInteger(const ObjectKey &K) const {
+ if (auto *V = get(K))
+return V->asInteger();
+ return llvm::None;
+}
llvm::Optional getString(const ObjectKey &K) const {
if (auto *V = get(K))
return V->asString();
@@ -374,6 +389,9 @@ public:
llvm::Optional getNumber(size_t I) const {
return (*this)[I].asNumber();
}
+llvm::Optional getInteger(size_t I) const {
+ return (*this)[I].asInteger();
+}
llvm::Optional getString(size_t I) const {
return (*this)[I].asString();
}
Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/tru
[PATCH] D40072: [libclang] Support querying whether a declaration is invalid
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: include/clang-c/Index.h:2622 + * + * A declaration is invalid if it could not be parsed successfully. + */ Perhaps add that we need to have identified it as a declaration, and that it'll return 0 when we didn't indeed identify something as a declaration :) https://reviews.llvm.org/D40072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support
dberris added a comment.
In https://reviews.llvm.org/D39114#930461, @kubamracek wrote:
> Hi, can you split the patch and send individual changes as separate patches?
> It's getting hard to follow. And I think we should land the parts that are
> "safe to land", e.g. the ASM changes or some of the NFC whitespace changes.
> The Darwin-specific changes in tests can also be landed (but don't enable the
> tests on Darwin, yet).
Yes, I'm going to be breaking this up into smaller parts -- just trying to see
where it's going at this point.
I've just finally gotten to the point where I can get back to this again. :)
In https://reviews.llvm.org/D39114#930477, @kubamracek wrote:
> And we should probably introduce files named `xray_linux.cc` and
> `xray_mac.cc` to contain platform-specific functions.
I fully agree. That should happen in smaller steps.
Expect smaller reviews coming soon, from breaking this patch up. :D
Comment at: compiler-rt/lib/xray/xray_x86_64.cc:25
+ size_t Len = 0;
+ if (sysctlbyname("hw.cpufrequency_max", &CPUFreq, &Len, NULL, 0) == -1) {
+Report("Unable to determine CPU frequency for TSC accounting; errno =
%d\n",
krytarowski wrote:
> Is it OK for you to call here internally malloc(3)? At least the NetBSD
> version calls it.
Good point. We should be caching this information earlier on in the process
(i.e. before we start tracing) to avoid deadlocks in case we have an
instrumented malloc implementation. Let me put a `TODO` or at least make sure
we're calling this at a point that is safe (probably at initialisation time).
https://reviews.llvm.org/D39114
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov added inline comments.
Comment at: clangd/Context.cpp:16
+
+static Context *GlobalCtx = nullptr;
+static Context EmptyContext(nullptr, {});
sammccall wrote:
> Seems like it would simplify things if:
> - GlobalCtx was always set (static local trick)
> - GlobalSession swapped its context with Global, and then swapped back in
> its destructor
Will do
Comment at: clangd/Context.h:65
+ Context *Parent;
+ TypedValueMap Data;
+};
sammccall wrote:
> We add complexity here (implementation and conceptual) to allow multiple
> properties to be set at the same level (vs having a key and an AnyStorage and
> making Context a linked list).
> Is this for performance? I'm not convinced it'll actually be faster for our
> workloads, or that it matters.
Conceptually, a `Context` is more convenient to use when it stores multiple
values. This allows to put a bunch of things and assign meaning to `Context`
(i.e., a `Context` for processing a single LSP request, global context). If
`Context`s were a linked list, the intermediate `Context`s would be hard to
assign the meaning to.
That being said, storage strategy for `Context`s is an implementation detail
and could be changed anytime. I don't have big preferences here, but I think
that storing a linked list of maps has, in general, a better performance than
storing a linked list.
And given that it's already there, I'd leave it this way.
Comment at: clangd/Context.h:79
+/// Otherwise returns an empty Context.
+Context &globalCtx();
+
sammccall wrote:
> ilya-biryukov wrote:
> > bkramer wrote:
> > > This is a giant code smell. If we want the context route, please pass
> > > contexts everywhere. I really don't want this kind of technical debt in
> > > clangd now.
> > I'm with you on this one, but I think @sammccall was keen on having the
> > global context. The main reason was to always have access to **some**
> > loggers and tracers, even when it's hard to pass the Context into the
> > function.
> > It's perfectly easy to remove all usages of `globalCtx()`, currently only 8
> > usages to get rid of. However, I'd wait for Sam's comment before doing that.
> It's important to be able to call functions that require a context if you
> don't have one - adding a log statement/trace for debugging shouldn't require
> changing plumbing/interfaces. (It's fine if we want to avoid checking in such
> code...)
> Having an "empty" global context allows this.
>
> At the same time, we want the ability to set the sink for logs/traces etc
> globally.
>
> A couple of options:
> - the sink (e.g. Logger) is part of the context, we need to allow embedders
> to set/augment the global context
> - the sink is not stored in the context, instead it is some other singleton
> the embedder can set up
>
> I don't have a strong opinion which is better. It's nice to reuse mechanisms,
> on the other hand loggers vs request IDs are pretty different types of data.
I'd still get rid of it. The less implicit behavior we have, the better.
It does not seem hard to plumb the `Context` all the way through clangd
currently. And it should not be too hard in the future.
I was asking myself multiple times whether we needed the global context in the
first place while implementing it.
I think we should aim for avoiding global state altogether. That includes
singletons, etc.
Comment at: clangd/Context.h:116
+/// Creates a new ContextBuilder, using globalCtx() as a parent.
+ContextBuilder buildCtx();
+/// Creates a new ContextBuilder with explicit \p Parent.
sammccall wrote:
> This seems more naturally a method on Context, e.g.
>
> Context C = globalCtx().derive(key, value);
>
> The relationship between global and C is clear.
>
> (You can allow chaining+mapping by having Context::derive and
> ContextBuilder::derive both return ContextBuilder&&, but as noted below I'm
> not sure it's worth the complexity over Context::derive ->Context)
I think the current interface is simple enough and allows for both storage
implementations. I'd really avoid providing an interface that ties us into a
single storage implementation.
Comment at: clangd/TypedValueMap.h:1
+//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===//
+//
sammccall wrote:
> This might be doing a little more than it needs to.
>
> Do we need the ability to have multiple values of the same type?
> If request ID is an int, needing to do `struct RequestID { ID int };` doesn't
> seem like much of a burden, and simplifies the semantics here.
>
> And for cases like Logger where the type carries the semantics, keying by
> type is clearer.
Are `Key<>` types confusing? I really like the fact that I don't have to
specify type information and it is carried in the `Key` to the
`get`/`emplace` methods, i.e. I don't have to spe
[PATCH] D40528: add new check to find NSError init invocation
Wizard added inline comments. Comment at: docs/clang-tidy/checks/objc-avoid-nserror-init.rst:10 +``errorWithDomain:code:userInfo:`` to create new NSError objects instead +of ``[NSError alloc] init]``. Otherwise it will lead to a warning message +during compilation in Xcode. hokein wrote: > What's the warning message in Xcode? I suspect whether there is a diagnostic > flag in clang already. It was discussed originally here https://buganizer.corp.google.com/issues/62445078 I think Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges
erikjv added a comment.
Maybe something like this works:
--- a/tools/libclang/CIndex.cpp
+++ b/tools/libclang/CIndex.cpp
@@ -8090,6 +8090,7 @@ CXSourceRangeList
*clang_getSkippedRanges(CXTranslationUnit TU, CXFile file) {
SourceManager &sm = Ctx.getSourceManager();
FileEntry *fileEntry = static_cast(file);
FileID wantedFileID = sm.translateFile(fileEntry);
+ bool isMainFile = wantedFileID == sm.getMainFileID();
const std::vector &SkippedRanges = ppRec->getSkippedRanges();
std::vector wantedRanges;
@@ -8097,6 +8098,8 @@ CXSourceRangeList
*clang_getSkippedRanges(CXTranslationUnit TU, CXFile file) {
i != ei; ++i) {
if (sm.getFileID(i->getBegin()) == wantedFileID ||
sm.getFileID(i->getEnd()) == wantedFileID)
wantedRanges.push_back(*i);
+else if (isMainFile && (astUnit->isInPreambleFileID(i->getBegin()) ||
astUnit->isInPreambleFileID(i->getEnd(
+ wantedRanges.push_back(*i);
}
skipped->count = wantedRanges.size();
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40310: Restructure how we break tokens.
krasimir added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1525
+ if (!DryRun)
+Token->adaptStartOfLine(0, Whitespaces);
+
If we indent here, shouldn't that also change ContentStartColumn?
Comment at: lib/Format/ContinuationIndenter.cpp:1557
if (LineIndex < EndIndex - 1)
+ // The last line's penalty is handled in addNextStateToQueue().
Penalty += Style.PenaltyExcessCharacter *
How does the last line's penalty get handled in addNextStateToQueue()?
Comment at: lib/Format/ContinuationIndenter.cpp:1565
- // Check if compressing the whitespace range will bring the line length
- // under the limit. If that is the case, we perform whitespace
compression
- // instead of inserting a line break.
- unsigned RemainingTokenColumnsAfterCompression =
- Token->getLineLengthAfterCompression(RemainingTokenColumns, Split);
- if (RemainingTokenColumnsAfterCompression <= RemainingSpace) {
-RemainingTokenColumns = RemainingTokenColumnsAfterCompression;
-ReflowInProgress = true;
-if (!DryRun)
- Token->compressWhitespace(LineIndex, TailOffset, Split, Whitespaces);
-DEBUG(llvm::dbgs() << "Compressing below limit.\n");
-break;
- }
-
- // Compute both the penalties for:
- // - not breaking, and leaving excess characters
- // - adding a new line break
- assert(RemainingTokenColumnsAfterCompression > RemainingSpace);
- unsigned ExcessCharactersPenalty =
- (RemainingTokenColumnsAfterCompression - RemainingSpace) *
- Style.PenaltyExcessCharacter;
-
- unsigned BreakPenalty = NewBreakPenalty;
- unsigned ColumnsUsed =
- Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first);
- if (ColumnsUsed > ColumnLimit)
-BreakPenalty +=
-Style.PenaltyExcessCharacter * (ColumnsUsed - ColumnLimit);
-
- DEBUG(llvm::dbgs() << "Penalty excess: " << ExcessCharactersPenalty
- << "\nbreak : " << BreakPenalty << "\n");
- // Only continue to add the line break if the penalty of the excess
- // characters is larger than the penalty of the line break.
- // FIXME: This does not take into account when we can later remove the
- // line break again due to a reflow.
- if (ExcessCharactersPenalty < BreakPenalty) {
-if (!DryRun)
- Token->compressWhitespace(LineIndex, TailOffset, Split, Whitespaces);
-// Do not set ReflowInProgress: we do not have any space left to
-// reflow into.
-Penalty += ExcessCharactersPenalty;
-break;
+ if (!Current.isStringLiteral()) {
+// Check whether the next natural split point after the current one
I really dislike this: we shouldn't have the reflow control flow depend on the
specific type of the token. Better introduce a new virtual method that enables
this branch and override it appropriately.
Comment at: lib/Format/ContinuationIndenter.cpp:1578
+LineIndex, TailOffset + Split.first + Split.second, ColumnLimit,
+ContentStartColumn + ToSplitColumns, CommentPragmasRegex);
+// Compute the comlumns necessary to fit the next non-breakable
sequence
nit: `BreakableToken::Split NextSplit = Token->getSplit(...)`
Comment at: lib/Format/ContinuationIndenter.cpp:1578
+LineIndex, TailOffset + Split.first + Split.second, ColumnLimit,
+ContentStartColumn + ToSplitColumns, CommentPragmasRegex);
+// Compute the comlumns necessary to fit the next non-breakable
sequence
krasimir wrote:
> nit: `BreakableToken::Split NextSplit = Token->getSplit(...)`
Hm, right now `ContentStartColumn + ToSplitColumns` points to the column where
the character at offset `TailOffset + Split.first` is supposed to be. Then
`NextSplit` is relative to the offset `TailOffset + Split.first +
Split.second`, so IMO it shouldn't use `ContentStartColumn + ToSplitColumns` as
a start column. I think that `ToSplitColumns` needs to be computed as follows:
```
unsigned ToSplitColumns = Token->getRangeLength(LineIndex, TailOffset,
Split.first + Split.second, ContentStartColumn);
```
Also, a comment of the intended meaning of `ToSplitColumns` would be helpful.
Comment at: lib/Format/ContinuationIndenter.cpp:1579
+ContentStartColumn + ToSplitColumns, CommentPragmasRegex);
+// Compute the comlumns necessary to fit the next non-breakable
sequence
+// into the current line.
nit: `comlumns`
https://reviews.llvm.org/D40310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor
erikjv closed this revision. erikjv added a comment. Submitted as r317308. https://reviews.llvm.org/D38578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin
dberris updated this revision to Diff 124538. dberris retitled this revision from "[XRay][darwin] Initial XRay in Darwin Support" to "[XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin". dberris edited the summary of this revision. dberris added a comment. This revision is now accepted and ready to land. Retitle and reduce patch to smaller bits. https://reviews.llvm.org/D39114 Files: compiler-rt/cmake/config-ix.cmake compiler-rt/lib/xray/CMakeLists.txt compiler-rt/lib/xray/weak_symbols.txt compiler-rt/lib/xray/xray_fdr_logging.cc compiler-rt/lib/xray/xray_flags.inc compiler-rt/lib/xray/xray_init.cc compiler-rt/lib/xray/xray_trampoline_x86_64.S Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S === --- compiler-rt/lib/xray/xray_trampoline_x86_64.S +++ compiler-rt/lib/xray/xray_trampoline_x86_64.S @@ -14,10 +14,13 @@ //===--===// #include "../builtins/assembly.h" +#include "../sanitizer_common/sanitizer_asm.h" + + .macro SAVE_REGISTERS subq $192, %rsp - .cfi_def_cfa_offset 200 + CFI_DEF_CFA_OFFSET(200) // At this point, the stack pointer should be aligned to an 8-byte boundary, // because any call instructions that come after this will add another 8 // bytes and therefore align it to 16-bytes. @@ -57,7 +60,7 @@ movq 8(%rsp), %r8 movq 0(%rsp), %r9 addq $192, %rsp - .cfi_def_cfa_offset 8 + CFI_DEF_CFA_OFFSET(8) .endm .macro ALIGNED_CALL_RAX @@ -75,21 +78,25 @@ .endm .text +#if !defined(__APPLE__) + .section .text +#else + .section __TEXT,__text +#endif .file "xray_trampoline_x86.S" //===--===// - .globl __xray_FunctionEntry + .globl ASM_TSAN_SYMBOL(__xray_FunctionEntry) .align 16, 0x90 - .type __xray_FunctionEntry,@function - -__xray_FunctionEntry: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionEntry) +ASM_TSAN_SYMBOL(__xray_FunctionEntry): + CFI_STARTPROC SAVE_REGISTERS // This load has to be atomic, it's concurrent with __xray_patch(). // On x86/amd64, a simple (type-aligned) MOV instruction is enough. - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax, %rax je .Ltmp0 @@ -101,28 +108,27 @@ .Ltmp0: RESTORE_REGISTERS retq -.Ltmp1: - .size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry - .cfi_endproc + ASM_SIZE(__xray_FunctionEntry) + CFI_ENDPROC //===--===// - .globl __xray_FunctionExit + .globl ASM_TSAN_SYMBOL(__xray_FunctionExit) .align 16, 0x90 - .type __xray_FunctionExit,@function -__xray_FunctionExit: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionExit) +ASM_TSAN_SYMBOL(__xray_FunctionExit): + CFI_STARTPROC // Save the important registers first. Since we're assuming that this // function is only jumped into, we only preserve the registers for // returning. subq $56, %rsp - .cfi_def_cfa_offset 64 + CFI_DEF_CFA_OFFSET(64) movq %rbp, 48(%rsp) movupd %xmm0, 32(%rsp) movupd %xmm1, 16(%rsp) movq %rax, 8(%rsp) movq %rdx, 0(%rsp) - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax,%rax je .Ltmp2 @@ -138,22 +144,21 @@ movq 8(%rsp), %rax movq 0(%rsp), %rdx addq $56, %rsp - .cfi_def_cfa_offset 8 + CFI_DEF_CFA_OFFSET(8) retq -.Ltmp3: - .size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit - .cfi_endproc + ASM_SIZE(__xray_FunctionExit) + CFI_ENDPROC //===--===// - .global __xray_FunctionTailExit + .globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit) .align 16, 0x90 - .type __xray_FunctionTailExit,@function -__xray_FunctionTailExit: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionTailExit) +ASM_TSAN_SYMBOL(__xray_FunctionTailExit): + CFI_STARTPROC SAVE_REGISTERS - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax,%rax je .Ltmp4 @@ -165,26 +170,25 @@ .Ltmp4: RESTORE_REGISTERS retq -.Ltmp5: - .size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit - .cfi_endproc + ASM_SIZE(__xray_FunctionTailExit) + CFI_ENDPROC //===--===// - .globl __xray_ArgLoggerEntry + .globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry) .align 16, 0x90 - .type __xray_ArgLoggerEntry,@function -__xray_ArgLoggerEntry: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry) +ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry): + CFI_STARTPROC SAVE_REGISTERS // Again, these function pointer loads must be atomic; MOV is fine. - movq _ZN6__xray13XRayArgLoggerE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax testq %rax, %rax j
[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin
dberris updated this revision to Diff 124539. dberris edited the summary of this revision. dberris added a comment. Update description to not use tabs. https://reviews.llvm.org/D39114 Files: compiler-rt/cmake/config-ix.cmake compiler-rt/lib/xray/CMakeLists.txt compiler-rt/lib/xray/weak_symbols.txt compiler-rt/lib/xray/xray_fdr_logging.cc compiler-rt/lib/xray/xray_flags.inc compiler-rt/lib/xray/xray_init.cc compiler-rt/lib/xray/xray_trampoline_x86_64.S Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S === --- compiler-rt/lib/xray/xray_trampoline_x86_64.S +++ compiler-rt/lib/xray/xray_trampoline_x86_64.S @@ -14,10 +14,13 @@ //===--===// #include "../builtins/assembly.h" +#include "../sanitizer_common/sanitizer_asm.h" + + .macro SAVE_REGISTERS subq $192, %rsp - .cfi_def_cfa_offset 200 + CFI_DEF_CFA_OFFSET(200) // At this point, the stack pointer should be aligned to an 8-byte boundary, // because any call instructions that come after this will add another 8 // bytes and therefore align it to 16-bytes. @@ -57,7 +60,7 @@ movq 8(%rsp), %r8 movq 0(%rsp), %r9 addq $192, %rsp - .cfi_def_cfa_offset 8 + CFI_DEF_CFA_OFFSET(8) .endm .macro ALIGNED_CALL_RAX @@ -75,21 +78,25 @@ .endm .text +#if !defined(__APPLE__) + .section .text +#else + .section __TEXT,__text +#endif .file "xray_trampoline_x86.S" //===--===// - .globl __xray_FunctionEntry + .globl ASM_TSAN_SYMBOL(__xray_FunctionEntry) .align 16, 0x90 - .type __xray_FunctionEntry,@function - -__xray_FunctionEntry: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionEntry) +ASM_TSAN_SYMBOL(__xray_FunctionEntry): + CFI_STARTPROC SAVE_REGISTERS // This load has to be atomic, it's concurrent with __xray_patch(). // On x86/amd64, a simple (type-aligned) MOV instruction is enough. - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax, %rax je .Ltmp0 @@ -101,28 +108,27 @@ .Ltmp0: RESTORE_REGISTERS retq -.Ltmp1: - .size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry - .cfi_endproc + ASM_SIZE(__xray_FunctionEntry) + CFI_ENDPROC //===--===// - .globl __xray_FunctionExit + .globl ASM_TSAN_SYMBOL(__xray_FunctionExit) .align 16, 0x90 - .type __xray_FunctionExit,@function -__xray_FunctionExit: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionExit) +ASM_TSAN_SYMBOL(__xray_FunctionExit): + CFI_STARTPROC // Save the important registers first. Since we're assuming that this // function is only jumped into, we only preserve the registers for // returning. subq $56, %rsp - .cfi_def_cfa_offset 64 + CFI_DEF_CFA_OFFSET(64) movq %rbp, 48(%rsp) movupd %xmm0, 32(%rsp) movupd %xmm1, 16(%rsp) movq %rax, 8(%rsp) movq %rdx, 0(%rsp) - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax,%rax je .Ltmp2 @@ -138,22 +144,21 @@ movq 8(%rsp), %rax movq 0(%rsp), %rdx addq $56, %rsp - .cfi_def_cfa_offset 8 + CFI_DEF_CFA_OFFSET(8) retq -.Ltmp3: - .size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit - .cfi_endproc + ASM_SIZE(__xray_FunctionExit) + CFI_ENDPROC //===--===// - .global __xray_FunctionTailExit + .globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit) .align 16, 0x90 - .type __xray_FunctionTailExit,@function -__xray_FunctionTailExit: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_FunctionTailExit) +ASM_TSAN_SYMBOL(__xray_FunctionTailExit): + CFI_STARTPROC SAVE_REGISTERS - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax testq %rax,%rax je .Ltmp4 @@ -165,26 +170,25 @@ .Ltmp4: RESTORE_REGISTERS retq -.Ltmp5: - .size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit - .cfi_endproc + ASM_SIZE(__xray_FunctionTailExit) + CFI_ENDPROC //===--===// - .globl __xray_ArgLoggerEntry + .globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry) .align 16, 0x90 - .type __xray_ArgLoggerEntry,@function -__xray_ArgLoggerEntry: - .cfi_startproc + ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry) +ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry): + CFI_STARTPROC SAVE_REGISTERS // Again, these function pointer loads must be atomic; MOV is fine. - movq _ZN6__xray13XRayArgLoggerE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax testq %rax, %rax jne .Larg1entryLog // If [arg1 logging handler] not set, defer to no-arg logging. - movq _ZN6__xray19XRayPatchedFunctionE(%rip), %rax + movq ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax tes
[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin
dberris added a comment. Landing this now and seeing whether there's fallout from the build bots. https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes
xazax.hun accepted this revision.
xazax.hun added a comment.
I found some nit, otherwise LG!
I think you should includ the context in the patches, that makes them reviewing
much easier. See:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Comment at: include/clang/Sema/TemplateInstCallback.h:47
+ for (auto &C : Callbacks) {
+if (C) {
+ C->initialize(TheSema);
We usually omit the braces for one line long single statements guarded by ifs.
Comment at: lib/Frontend/FrontendActions.cpp:326
+private:
+ static std::string ToString(CodeSynthesisContext::SynthesisKind Kind) {
+switch (Kind) {
The methods should start with a lowercase letter even if they are static.
https://reviews.llvm.org/D5767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule
Quolyk added inline comments.
Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
QualifiedRenameRule(const NamedDecl *ND,
- std::string NewQualifiedName)
+ const std::string &NewQualifiedName)
: ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
hokein wrote:
> Passing `std::string` object is fine here -- because we use std::move below
> to avoid the extra copy.
>
> Is the warning caught by the clang-tidy check?
Unfortunatelly, I haven't tested, but I believe it's checkcpp warning.
https://reviews.llvm.org/D40543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP
ioeric created this revision.
Herald added subscribers: ilya-biryukov, mgorny.
o Experimental interfaces to support use multiple index sources (e.g. AST index,
global index) for code completion, cross-reference finding etc.
o Replace sema code completion for qualified-id with index-based completion.
https://reviews.llvm.org/D40548
Files:
clangd/ASTIndex.cpp
clangd/ASTIndex.h
clangd/CMakeLists.txt
clangd/ClangdIndex.cpp
clangd/ClangdIndex.h
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.cpp
clangd/ClangdUnitStore.h
clangd/SymbolIndex.h
clangd/tool/ClangdMain.cpp
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -160,7 +160,7 @@
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
EnableSnippets, ResourceDirRef,
-CompileCommandsDirPath);
+CompileCommandsDirPath, {});
constexpr int NoShutdownRequestErrorCode = 1;
llvm::set_thread_name("clangd.main");
return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode;
Index: clangd/SymbolIndex.h
===
--- /dev/null
+++ clangd/SymbolIndex.h
@@ -0,0 +1,57 @@
+//===--- CompletionIndex.h - Index for code completion ---*- 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_CLANGD_COMPLETIONINDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPLETIONINDEX_H
+
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+struct CompletionRequest {
+ std::string Query;
+ std::vector FixedPrefixes;
+};
+
+struct ScoreSignals {
+ float fuzzy_score;
+};
+
+struct CompletionSymbol {
+ ScoreSignals Signals;
+
+ std::string UID;
+ std::string QualifiedName;
+};
+
+struct CompletionResult {
+ //std::vector Symbol;
+ std::vector Symbols;
+ bool all_matched;
+};
+
+class SymbolIndex {
+public:
+ virtual ~SymbolIndex() = default;
+
+ virtual llvm::Expected
+ complete(const CompletionRequest &Req) const = 0;
+
+ virtual llvm::Expected
+ getSymbolInfo(llvm::StringRef UID) const = 0;
+
+ virtual llvm::Expected>
+ getAllOccurrences(llvm::StringRef UID) const = 0;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPLETIONINDEX_H
Index: clangd/ClangdUnitStore.h
===
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -12,6 +12,7 @@
#include
+#include "ASTIndex.h"
#include "ClangdUnit.h"
#include "GlobalCompilationDatabase.h"
#include "Path.h"
@@ -25,11 +26,14 @@
/// Thread-safe mapping from FileNames to CppFile.
class CppFileCollection {
public:
+ explicit CppFileCollection(ASTIndexSourcer *IndexSourcer)
+ : IndexSourcer(IndexSourcer) {}
+
std::shared_ptr
getOrCreateFile(PathRef File, PathRef ResourceDir,
GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory,
std::shared_ptr PCHs,
- clangd::Logger &Logger) {
+ clangd::Logger &Logger, ASTIndexSourcer *IndexSourcer) {
std::lock_guard Lock(Mutex);
auto It = OpenedFiles.find(File);
@@ -39,7 +43,8 @@
It = OpenedFiles
.try_emplace(File, CppFile::Create(File, std::move(Command),
StorePreamblesInMemory,
- std::move(PCHs), Logger))
+ std::move(PCHs), Logger,
+ IndexSourcer))
.first;
}
return It->second;
@@ -86,6 +91,7 @@
std::mutex Mutex;
llvm::StringMap> OpenedFiles;
+ ASTIndexSourcer *IndexSourcer;
};
} // namespace clangd
} // namespace clang
Index: clangd/ClangdUnitStore.cpp
===
--- clangd/ClangdUnitStore.cpp
+++ clangd/ClangdUnitStore.cpp
@@ -23,6 +23,7 @@
std::shared_ptr Result = It->second;
OpenedFiles.erase(It);
+ IndexSourcer->remove(File);
return Result;
}
@@ -42,14 +43,15 @@
It = OpenedFiles
.try_emplace(File, CppFile::Create(File, std::move(NewCommand),
StorePreamblesInMemory,
-std::move(PCHs), Logger))
+std::move(PCHs), Logger,
+
[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases
xazax.hun added a comment.
In https://reviews.llvm.org/D38171#927046, @alexfh wrote:
> And, btw, sorry for the long delay. I've been on travelling / on vacation for
> the last few weeks.
No problem. Will you have some time to think about the overall concept?
Implementation and test wise it looks good to me.
I think this patch is a step in a good direction but of course, it is important
for the community to agree on the approach.
Comment at: test/clang-tidy/warning-check-aliases.cpp:10
+ try {
+
+ } catch (B &X) {
There are some redundant empty lines.
https://reviews.llvm.org/D38171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases
xazax.hun added inline comments.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:262
+std::vector ClangTidyContext::getEnabledClangDiagnostics() {
+ llvm::SmallVector Diags;
+ DiagnosticIDs::getAllDiagnostics(diag::Flavor::WarningOrError, Diags);
I am wondering if this is still a good candidate to be a small vector with this
size. Maybe a regular vector with a reserve might be better?
https://reviews.llvm.org/D38171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40072: [libclang] Support querying whether a declaration is invalid
nik updated this revision to Diff 124550.
nik marked an inline comment as done.
nik added a comment.
Rebaed and clarified the documentation only.
Please submit as I don't have the permissions.
https://reviews.llvm.org/D40072
Files:
include/clang-c/Index.h
test/Index/print-type-size.cpp
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
tools/libclang/libclang.exports
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -288,6 +288,7 @@
clang_isConstQualifiedType
clang_isCursorDefinition
clang_isDeclaration
+clang_isInvalidDeclaration
clang_isExpression
clang_isFileMultipleIncludeGuarded
clang_isFunctionTypeVariadic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5379,6 +5379,15 @@
(K >= CXCursor_FirstExtraDecl && K <= CXCursor_LastExtraDecl);
}
+unsigned clang_isInvalidDeclaration(CXCursor C) {
+ if (clang_isDeclaration(C.kind)) {
+if (const Decl *D = getCursorDecl(C))
+ return D->isInvalidDecl();
+ }
+
+ return 0;
+}
+
unsigned clang_isReference(enum CXCursorKind K) {
return K >= CXCursor_FirstRef && K <= CXCursor_LastRef;
}
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -810,6 +810,8 @@
printf(" (variadic)");
if (clang_Cursor_isObjCOptional(Cursor))
printf(" (@optional)");
+if (clang_isInvalidDeclaration(Cursor))
+ printf(" (invalid)");
switch (clang_getCursorExceptionSpecificationType(Cursor))
{
Index: test/Index/print-type-size.cpp
===
--- test/Index/print-type-size.cpp
+++ test/Index/print-type-size.cpp
@@ -4,8 +4,8 @@
namespace basic {
-// CHECK64: VarDecl=v:[[@LINE+2]]:6 (Definition) [type=void] [typekind=Void]
-// CHECK32: VarDecl=v:[[@LINE+1]]:6 (Definition) [type=void] [typekind=Void]
+// CHECK64: VarDecl=v:[[@LINE+2]]:6 (Definition) (invalid) [type=void]
[typekind=Void]
+// CHECK32: VarDecl=v:[[@LINE+1]]:6 (Definition) (invalid) [type=void]
[typekind=Void]
void v;
// CHECK64: VarDecl=v1:[[@LINE+2]]:7 (Definition) [type=void *]
[typekind=Pointer] [sizeof=8] [alignof=8]
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
* compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
*/
#define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 43
+#define CINDEX_VERSION_MINOR 44
#define CINDEX_VERSION_ENCODE(major, minor) ( \
((major) * 1) \
@@ -2616,6 +2616,16 @@
*/
CINDEX_LINKAGE unsigned clang_isDeclaration(enum CXCursorKind);
+/**
+ * \brief Determine whether the given declaration is invalid.
+ *
+ * A declaration is invalid if it could not be parsed successfully.
+ *
+ * \returns non-zero if the cursor represents a declaration and it is
+ * invalid, otherwise NULL.
+ */
+CINDEX_LINKAGE unsigned clang_isInvalidDeclaration(CXCursor);
+
/**
* \brief Determine whether the given cursor kind represents a simple
* reference.
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -288,6 +288,7 @@
clang_isConstQualifiedType
clang_isCursorDefinition
clang_isDeclaration
+clang_isInvalidDeclaration
clang_isExpression
clang_isFileMultipleIncludeGuarded
clang_isFunctionTypeVariadic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5379,6 +5379,15 @@
(K >= CXCursor_FirstExtraDecl && K <= CXCursor_LastExtraDecl);
}
+unsigned clang_isInvalidDeclaration(CXCursor C) {
+ if (clang_isDeclaration(C.kind)) {
+if (const Decl *D = getCursorDecl(C))
+ return D->isInvalidDecl();
+ }
+
+ return 0;
+}
+
unsigned clang_isReference(enum CXCursorKind K) {
return K >= CXCursor_FirstRef && K <= CXCursor_LastRef;
}
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -810,6 +810,8 @@
printf(" (variadic)");
if (clang_Cursor_isObjCOptional(Cursor))
printf(" (@optional)");
+if (clang_isInvalidDeclaration(Cursor))
+ printf(" (invalid)");
switch (clang_getCursorExceptionSpecificationType(Cursor))
{
Index: test/Index/print-type-size.cpp
===
--- test/Index/print-type-size.cpp
+++ test/Index/pr
[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true
MTC updated this revision to Diff 124549.
MTC added a comment.
Update the llvm_unreachable's description of the BlockEntrance-branch from
"Unexpected ProgramPoint" to "Unexpected CFG element at front of block".
https://reviews.llvm.org/D37187
Files:
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/loop-widening-notes.cpp
Index: test/Analysis/loop-widening-notes.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+ if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}}
+// expected-note@-1 {{Taking true branch}}
+bar();
+ for (int i = 0; // expected-note {{Loop condition is true. Entering loop
body}}
+ // expected-note@-1 {{Loop condition is false. Execution
continues on line 16}}
+ ++i,// expected-note {{Value assigned to 'p_a'}}
+ i < flag_a;
+ ++i) {}
+
+ *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from
variable 'p_a')}}
+ return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+ flag_b = 100;
+ int num = 10;
+ while (flag_b-- > 0) { // expected-note {{Loop condition is true. Entering
loop body}}
+ // expected-note@-1 {{Value assigned to 'num'}}
+ // expected-note@-2 {{Loop condition is false.
Execution continues on line 30}}
+num = flag_b;
+ }
+ if (num < 0) // expected-note {{Assuming 'num' is >= 0}}
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+ else if (num >= 1) // expected-note {{Assuming 'num' is < 1}}
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+ else
+flag_b = 100;
+ return flag_b / num; // no-crash expected-warning {{Division by zero}}
+ // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+ int num = 10;
+ do { // expected-note {{Loop condition is true. Execution continues on
line 47}}
+ // expected-note@-1 {{Loop condition is false. Exiting loop}}
+num--;
+ } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+ int local = 0;
+ if (num == 0) // expected-note {{Assuming 'num' is equal to 0}}
+ // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+ // expected-note@-1 {{Division by zero}}
+ return local;
+}
+
+int flag_d;
+int test_for_loop() {
+ int num = 10;
+ for (int i = 0;// expected-note {{Loop condition is true. Entering loop
body}}
+ // expected-note@-1 {{Loop condition is false. Execution
continues on line 67}}
+ new int(10), // expected-note {{Value assigned to 'num'}}
+ i < flag_d;
+ ++i) {
+++num;
+ }
+ if (num == 0) // expected-note {{Assuming 'num' is equal to 0}}
+// expected-note@-1 {{Taking true branch}}
+flag_d += 10;
+ return flag_d / num; // no-crash expected-warning {{Division by zero}}
+ // expected-note@-1 {{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -690,6 +690,15 @@
return getLocationForCaller(CEE->getCalleeContext(),
CEE->getLocationContext(),
SMng);
+ } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+ return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+ return PathDiagnosticLocation(
+ NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
} else {
llvm_unreachable("Unexpected ProgramPoint");
}
Index: test/Analysis/loop-widening-notes.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 -analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+ if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}}
+// expected-no
[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true
MTC marked an inline comment as done. MTC added a comment. Hi dcoughlin, Thank you very much for your help. I have no commit access and hope someone can commit it on my behalf. Thanks a lot! https://reviews.llvm.org/D37187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
alexfh updated this revision to Diff 124553.
alexfh added a comment.
Herald added a subscriber: klimek.
- clang-format -style=llvm
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40507
Files:
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstantArgumentCheck.cpp
clang-tidy/misc/MoveConstantArgumentCheck.h
clang-tidy/misc/NoexceptMoveConstructorCheck.cpp
clang-tidy/misc/NoexceptMoveConstructorCheck.h
clang-tidy/modernize/PassByValueCheck.cpp
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/MoveConstArgCheck.cpp
clang-tidy/performance/MoveConstArgCheck.h
clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tidy/performance/NoexceptMoveConstructorCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/hicpp-move-const-arg.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-move-const-arg.rst
docs/clang-tidy/checks/misc-noexcept-move-constructor.rst
docs/clang-tidy/checks/performance-move-const-arg.rst
docs/clang-tidy/checks/performance-noexcept-move-constructor.rst
test/clang-tidy/misc-move-const-arg.cpp
test/clang-tidy/misc-noexcept-move-constructor.cpp
test/clang-tidy/modernize-pass-by-value.cpp
test/clang-tidy/performance-move-const-arg.cpp
test/clang-tidy/performance-noexcept-move-constructor.cpp
Index: test/clang-tidy/performance-noexcept-move-constructor.cpp
===
--- test/clang-tidy/performance-noexcept-move-constructor.cpp
+++ test/clang-tidy/performance-noexcept-move-constructor.cpp
@@ -1,16 +1,18 @@
-// RUN: %check_clang_tidy %s misc-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
class A {
A(A &&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked
+ // noexcept [performance-noexcept-move-constructor]
A &operator=(A &&);
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
};
struct B {
static constexpr bool kFalse = false;
B(B &&) noexcept(kFalse);
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move
+ // constructor evaluates to 'false' [performance-noexcept-move-constructor]
};
class OK {};
@@ -24,16 +26,16 @@
public:
OK1();
OK1(const OK1 &);
- OK1(OK1&&) noexcept;
+ OK1(OK1 &&) noexcept;
OK1 &operator=(OK1 &&) noexcept;
void f();
void g() noexcept;
};
class OK2 {
static constexpr bool kTrue = true;
-public:
+ public:
OK2(OK2 &&) noexcept(true) {}
OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; }
};
Index: test/clang-tidy/performance-move-const-arg.cpp
===
--- test/clang-tidy/performance-move-const-arg.cpp
+++ test/clang-tidy/performance-move-const-arg.cpp
@@ -1,73 +1,91 @@
-// RUN: %check_clang_tidy %s misc-move-const-arg %t
+// RUN: %check_clang_tidy %s performance-move-const-arg %t
namespace std {
-template struct remove_reference;
+template
+struct remove_reference;
-template struct remove_reference { typedef _Tp type; };
+template
+struct remove_reference {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &&> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
template
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
return static_cast::type &&>(__t);
}
-} // namespace std
+} // namespace std
class A {
-public:
+ public:
A() {}
A(const A &rhs) {}
A(A &&rhs) {}
};
int f1() {
return std::move(42);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [misc-move-const-arg]
- // CHECK-FIXES: return 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of
+ // the trivially-copyable type 'int' has no effect; remove std::move()
+ // [performance-move-const-arg] CHECK-FIXES: return 42;
}
int f2(int x2) {
return std::move(x2);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x2' of the trivially-copyable type 'int'
- // CHECK-FIXES: return x2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x2' of
+ // the trivially-copyable type 'int' CHECK-FIXES: return x2;
}
int *f3(int *x3) {
return std::move(x3);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warni
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Found one possible problem. Once fixed, LG!
Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:33
+#include "../performance/MoveConstArgCheck.h"
+#include "../performance/NoexceptMoveConstructorCheck.h"
#include "../readability/BracesAroundStatementsCheck.h"
Don't you need to add performance module to link to the HICPP module to avoid
link errors?
Comment at: clang-tidy/performance/MoveConstArgCheck.h:19
-class MoveConstantArgumentCheck : public ClangTidyCheck {
+class MoveConstArgCheck : public ClangTidyCheck {
public:
Is there any specific reason to rename this class? I am ok with the change,
just wondering.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40507
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
alexfh updated this revision to Diff 124554.
alexfh added a comment.
- Reverted formatting in tests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40507
Files:
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstantArgumentCheck.cpp
clang-tidy/misc/MoveConstantArgumentCheck.h
clang-tidy/misc/NoexceptMoveConstructorCheck.cpp
clang-tidy/misc/NoexceptMoveConstructorCheck.h
clang-tidy/modernize/PassByValueCheck.cpp
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/MoveConstArgCheck.cpp
clang-tidy/performance/MoveConstArgCheck.h
clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tidy/performance/NoexceptMoveConstructorCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/hicpp-move-const-arg.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-move-const-arg.rst
docs/clang-tidy/checks/misc-noexcept-move-constructor.rst
docs/clang-tidy/checks/performance-move-const-arg.rst
docs/clang-tidy/checks/performance-noexcept-move-constructor.rst
test/clang-tidy/misc-move-const-arg.cpp
test/clang-tidy/misc-noexcept-move-constructor.cpp
test/clang-tidy/modernize-pass-by-value.cpp
test/clang-tidy/performance-move-const-arg.cpp
test/clang-tidy/performance-noexcept-move-constructor.cpp
Index: test/clang-tidy/performance-noexcept-move-constructor.cpp
===
--- test/clang-tidy/performance-noexcept-move-constructor.cpp
+++ test/clang-tidy/performance-noexcept-move-constructor.cpp
@@ -1,16 +1,16 @@
-// RUN: %check_clang_tidy %s misc-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
class A {
A(A &&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
A &operator=(A &&);
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
};
struct B {
static constexpr bool kFalse = false;
B(B &&) noexcept(kFalse);
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
};
class OK {};
@@ -24,16 +24,16 @@
public:
OK1();
OK1(const OK1 &);
- OK1(OK1&&) noexcept;
+ OK1(OK1 &&) noexcept;
OK1 &operator=(OK1 &&) noexcept;
void f();
void g() noexcept;
};
class OK2 {
static constexpr bool kTrue = true;
-public:
+ public:
OK2(OK2 &&) noexcept(true) {}
OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; }
};
Index: test/clang-tidy/performance-move-const-arg.cpp
===
--- test/clang-tidy/performance-move-const-arg.cpp
+++ test/clang-tidy/performance-move-const-arg.cpp
@@ -1,31 +1,41 @@
-// RUN: %check_clang_tidy %s misc-move-const-arg %t
+// RUN: %check_clang_tidy %s performance-move-const-arg %t
namespace std {
-template struct remove_reference;
+template
+struct remove_reference;
-template struct remove_reference { typedef _Tp type; };
+template
+struct remove_reference {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &&> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
template
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
return static_cast::type &&>(__t);
}
-} // namespace std
+} // namespace std
class A {
-public:
+ public:
A() {}
A(const A &rhs) {}
A(A &&rhs) {}
};
int f1() {
return std::move(42);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [misc-move-const-arg]
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [performance-move-const-arg]
// CHECK-FIXES: return 42;
}
@@ -45,11 +55,14 @@
A f5(const A x5) {
return std::move(x5);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [misc-move-const-arg]
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
// CHECK-FIXES: return x5;
}
-template T f6(const T x6) { return std::move(x6); }
[clang-tools-extra] r319170 - [clang-tidy] Fix tests for ReplaceRandomShuffleCheck
Author: xazax
Date: Tue Nov 28 05:54:52 2017
New Revision: 319170
URL: http://llvm.org/viewvc/llvm-project?rev=319170&view=rev
Log:
[clang-tidy] Fix tests for ReplaceRandomShuffleCheck
Patch by: Daniel Kolozsvari!
Differential Revision: https://reviews.llvm.org/D40516
Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp
Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp?rev=319170&r1=319169&r2=319170&view=diff
==
---
clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp
(original)
+++
clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp
Tue Nov 28 05:54:52 2017
@@ -34,21 +34,21 @@ int main() {
std::vector vec;
std::random_shuffle(vec.begin(), vec.end());
- // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' instead
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' instead
// CHECK-FIXES: std::shuffle(vec.begin(), vec.end(),
std::mt19937(std::random_device()()));
std::shuffle(vec.begin(), vec.end());
random_shuffle(vec.begin(), vec.end());
- // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' instead
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' instead
// CHECK-FIXES: shuffle(vec.begin(), vec.end(),
std::mt19937(std::random_device()()));
std::random_shuffle(vec.begin(), vec.end(), myrandom);
- // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
// CHECK-FIXES: std::shuffle(vec.begin(), vec.end(),
std::mt19937(std::random_device()()));
random_shuffle(vec.begin(), vec.end(), myrandom);
- // CHECK-MESSAGE: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::random_shuffle' has been
removed in C++17; use 'std::shuffle' and an alternative random mechanism instead
// CHECK-FIXES: shuffle(vec.begin(), vec.end(),
std::mt19937(std::random_device()()));
shuffle(vec.begin(), vec.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from one small nit with the test file missing a trailing newline. Comment at: test/clang-tidy/fuchsia-default-arguments.cpp:82 +} \ No newline at end of file Please add a newline at the end of the file. https://reviews.llvm.org/D40108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a minor formatting nit. Comment at: test/clang-tidy/readability-else-after-return.cpp:7 + ~string(); +}; +} Indentation is incorrect here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: tools/scan-build-py/libscanbuild/analyze.py:44 +CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt' +CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps' What would happen when multiple analyzer runs are launched concurrently in the same directory? Would they race on this file? Comment at: tools/scan-build-py/libscanbuild/analyze.py:64 if need_analyzer(args.build): -run_analyzer_parallel(args) +run_analyzer_with_ctu(args) else: `run_analyzer_with_ctu` is an unfortunate function name, since it may or may not launch CTU depending on the passed arguments. Could we just call it `run_analyzer`? Comment at: tools/scan-build-py/libscanbuild/analyze.py:103 +def prefix_with(constant, pieces): +""" From a sequence create another sequence where every second element Actually I would prefer a separate NFC PR just moving this function. This PR is already too complicated as it is. Comment at: tools/scan-build-py/libscanbuild/analyze.py:120 + func_map_cmd=args.func_map_cmd) +if hasattr(args, 'ctu_phases') and hasattr(args.ctu_phases, 'dir') +else CtuConfig(collect=False, analyze=False, dir='', func_map_cmd='')) Extensive `hasattr` usage is often a codesmell, and it hinders understanding. Firstly, shouldn't `args.ctu_phases.dir` be always available, judging from the code in `arguments.py`? Secondly, in what cases `args.ctu_phases` is not available when we are already going down the CTU code path? Shouldn't we throw an error at that point instead of creating a default configuration? (default-configurations-instead-of-crashing is also another way to introduce very subtle and hard-to-catch error modes) Comment at: tools/scan-build-py/libscanbuild/analyze.py:128 +A function map contains the id of a function (mangled name) and the +originating source (the corresponding AST file) name.""" + Could you also specify what `func_map_lines` is (file? list? of strings?) in the docstring? There's specific Sphinx syntax for that. Otherwise it is hard to follow. Comment at: tools/scan-build-py/libscanbuild/analyze.py:138 +else: +mangled_to_asts[mangled_name].add(ast_file) + Could be improved with `defaultdict`: ``` mangled_to_asts = defaultdict(set) ... mangled_to_asts[mangled_name].add(ast_file) ``` Comment at: tools/scan-build-py/libscanbuild/analyze.py:144 +if len(ast_files) == 1: +mangled_ast_pairs.append((mangled_name, ast_files.pop())) + Firstly, no need to modify the set in order to get the first element, just use `next(iter(ast_files))`. Secondly, when exactly do conflicting names happen? Is it a bug? Shouldn't the tool log such cases? Comment at: tools/scan-build-py/libscanbuild/analyze.py:145 +mangled_ast_pairs.append((mangled_name, ast_files.pop())) + +return mangled_ast_pairs Overall, instead of creating a dictionary with multiple elements, and then converting to a list, it's much simper to only add an element to `mangled_to_asts` when it is not already mapped to something (probably logging a message otherwise), and then just return `mangled_to_asts.items()` Comment at: tools/scan-build-py/libscanbuild/analyze.py:160 +def generate_func_map_lines(fnmap_dir): +""" Iterate over all lines of input files in random order. """ + Firstly, is `glob.glob` actually random, as the docstring is saying? If yes, can we make it not to be random (e.g. with `sorted`), as dealing with randomized input makes tracking down bugs so much harder? Comment at: tools/scan-build-py/libscanbuild/analyze.py:168 + +def write_global_map(ctudir, arch, mangled_ast_pairs): +""" Write (mangled function name, ast file) pairs into final file. """ If `write_global_map` is a closure, please use the fact that it would capture `ctudir` from the outer scope, and remove it from the argument list. That would make it more obvious that it does not change between invocations. Comment at: tools/scan-build-py/libscanbuild/analyze.py:189 +# Remove all temporary files +shutil.rmtree(fnmap_dir, ignore_errors=True) + Having an analysis tool remove files is scary, what if (maybe not in this revision, but in a future iteration) a bug is introduced, and the tool removes user code instead? Why not just create a temporary directory with `tempfile.mkdtemp`, put all temporary files there, and then simply iterate through them? Then yo
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
Hahnfeld accepted this revision. Hahnfeld added a comment. This revision is now accepted and ready to land. I'm fine if @rjmccall is fine - but looks like I need to accept this revision because I requested changes in the past? https://reviews.llvm.org/D40275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP
malaperle added a comment. Hi Eric! As you might know I'm working on persisted indexing. I was wondering which cases needed the index for code completion? Could you give a small example? I thought the AST alone would be sufficient for that. I'll look at this patch more closely a bit later but I can look at what needs to be added in the persisted index interface/data. If you'd like to see the work in progress, it's here: https://github.com/MarkZ3/clang-tools-extra/blob/IndexFunctionsPrototype https://github.com/MarkZ3/clang-tools-extra/blob/IndexFunctionsPrototype/clangd/index/README https://reviews.llvm.org/D40548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw
malcolm.parsons updated this revision to Diff 124566.
malcolm.parsons added a comment.
clang-format
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40505
Files:
clang-tidy/readability/ElseAfterReturnCheck.cpp
test/clang-tidy/readability-else-after-return.cpp
Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -1,5 +1,16 @@
// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11
-fexceptions
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namespace std
+
+struct my_exception {
+ my_exception(const std::string &s);
+};
+
void f(int a) {
if (a > 0)
return;
@@ -85,5 +96,12 @@
// CHECK-FIXES: {{^}}} // comment-9
x++;
}
+if (x) {
+ throw my_exception("foo");
+} else { // comment-10
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-10
+ x++;
+}
}
}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -21,7 +21,8 @@
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
const auto ControlFlowInterruptorMatcher =
stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
Finder->addMatcher(
compoundStmt(forEach(
ifStmt(hasThen(stmt(
Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -1,5 +1,16 @@
// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11 -fexceptions
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namespace std
+
+struct my_exception {
+ my_exception(const std::string &s);
+};
+
void f(int a) {
if (a > 0)
return;
@@ -85,5 +96,12 @@
// CHECK-FIXES: {{^}}} // comment-9
x++;
}
+if (x) {
+ throw my_exception("foo");
+} else { // comment-10
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-10
+ x++;
+}
}
}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -21,7 +21,8 @@
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
const auto ControlFlowInterruptorMatcher =
stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
Finder->addMatcher(
compoundStmt(forEach(
ifStmt(hasThen(stmt(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r319174 - [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw
Author: malcolm.parsons
Date: Tue Nov 28 06:57:47 2017
New Revision: 319174
URL: http://llvm.org/viewvc/llvm-project?rev=319174&view=rev
Log:
[clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw
Summary:
The readability-else-after-return check was not warning about
an else after a throw of an exception that had arguments that needed
to be cleaned up.
Reviewers: aaron.ballman, alexfh, djasper
Reviewed By: aaron.ballman
Subscribers: lebedev.ri, klimek, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D40505
Modified:
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=319174&r1=319173&r2=319174&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Tue
Nov 28 06:57:47 2017
@@ -21,7 +21,8 @@ namespace readability {
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
const auto ControlFlowInterruptorMatcher =
stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
Finder->addMatcher(
compoundStmt(forEach(
ifStmt(hasThen(stmt(
Modified:
clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp?rev=319174&r1=319173&r2=319174&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return.cpp
Tue Nov 28 06:57:47 2017
@@ -1,5 +1,16 @@
// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11
-fexceptions
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namespace std
+
+struct my_exception {
+ my_exception(const std::string &s);
+};
+
void f(int a) {
if (a > 0)
return;
@@ -85,5 +96,12 @@ void foo() {
// CHECK-FIXES: {{^}}} // comment-9
x++;
}
+if (x) {
+ throw my_exception("foo");
+} else { // comment-10
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-10
+ x++;
+}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE319174: [clang-tidy] Ignore ExprWithCleanups when looking
for else-after-throw (authored by malcolm.parsons).
Changed prior to commit:
https://reviews.llvm.org/D40505?vs=124566&id=124567#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40505
Files:
clang-tidy/readability/ElseAfterReturnCheck.cpp
test/clang-tidy/readability-else-after-return.cpp
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -21,7 +21,8 @@
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
const auto ControlFlowInterruptorMatcher =
stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
Finder->addMatcher(
compoundStmt(forEach(
ifStmt(hasThen(stmt(
Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -1,5 +1,16 @@
// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11
-fexceptions
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namespace std
+
+struct my_exception {
+ my_exception(const std::string &s);
+};
+
void f(int a) {
if (a > 0)
return;
@@ -85,5 +96,12 @@
// CHECK-FIXES: {{^}}} // comment-9
x++;
}
+if (x) {
+ throw my_exception("foo");
+} else { // comment-10
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-10
+ x++;
+}
}
}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -21,7 +21,8 @@
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
const auto ControlFlowInterruptorMatcher =
stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
- breakStmt().bind("break"), cxxThrowExpr().bind("throw")));
+ breakStmt().bind("break"),
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw");
Finder->addMatcher(
compoundStmt(forEach(
ifStmt(hasThen(stmt(
Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -1,5 +1,16 @@
// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++11 -fexceptions
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namespace std
+
+struct my_exception {
+ my_exception(const std::string &s);
+};
+
void f(int a) {
if (a > 0)
return;
@@ -85,5 +96,12 @@
// CHECK-FIXES: {{^}}} // comment-9
x++;
}
+if (x) {
+ throw my_exception("foo");
+} else { // comment-10
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
+// CHECK-FIXES: {{^}}} // comment-10
+ x++;
+}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP
ilya-biryukov added a comment. In https://reviews.llvm.org/D40548#937626, @malaperle wrote: > Hi Eric! As you might know I'm working on persisted indexing. I was wondering > which cases needed the index for code completion? Could you give a small > example? I thought the AST alone would be sufficient for that. I'll look at > this patch more closely a bit later but I can look at what needs to be added > in the persisted index interface/data. Just wanted to chime in to give my perspective on some of the work that's being done here. The index would allow to implement a project-wide completion (e.g., even if you don't `#include "llvm/ADT/DenseMap.h"`, you'll get completion for `DenseMap` and the #include will be added automatically upon completing an item). This is not aiming trying to have persistent indexes, instead trying to figure out how the right interfaces and plug things into `ClangdServer` properly, so that we can later play around with different implementations of the indexes. (I.e., we'll probably have our internal implementation at Google at some point). I guess the main thing right now would be to align on how the `SymbolIndex` interface should look like. Comment at: clangd/ClangdLSPServer.h:37 + llvm::Optional CompileCommandsDir, + std::vector< + std::pair> Maybe pass a parameter of type `SymbolIndex&` instead of a vector, which is used to create `CombinedSymbolIndex` later? It seems that `ClangdServer` does not really care about the specific index implementation that's used (nor should it!) We could have helper methods to conveniently create `CombinedSymbolIndex` from a list of indexes, or even create the default index for clangd. https://reviews.llvm.org/D40548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP
ioeric added a comment. In https://reviews.llvm.org/D40548#937626, @malaperle wrote: > Hi Eric! As you might know I'm working on persisted indexing. I was wondering > which cases needed the index for code completion? Could you give a small > example? I thought the AST alone would be sufficient for that. I'll look at > this patch more closely a bit later but I can look at what needs to be added > in the persisted index interface/data. > > If you'd like to see the work in progress, it's here: > https://github.com/MarkZ3/clang-tools-extra/blob/IndexFunctionsPrototype > > https://github.com/MarkZ3/clang-tools-extra/blob/IndexFunctionsPrototype/clangd/index/README Hi Marc! Yes, I was aware of your work. I wanted to upload the prototype early on to get discussions started, especially to get alignment with your work on work space index. As Ilya said, we would like clangd to support global code completion and other LSP features based on global index (i.e. all symbols in the code base). For example, for code completion on scope specifiers "nx::ny::", we want to be able to complete "nx::ny::X" even if the symbol is not in the AST yet. However, it might not always be feasible to rely on clangd to generate index for a huge code base, so we want to be able to support plugging in additional index sources. The index can be from opened files in clangd, all files in the project (e.g. clangd/ directory), or all files in a huge code base. IIUC, the persistent index you are working on would fall into the project index bucket? This patch tries to define interfaces for all indexes and how they would be used/merged in clangd, and we would really like your feedback on it. The code completion and AST index changes in the patch are mostly for experimenting with the interfaces. https://reviews.llvm.org/D40548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Clang-format: add finer-grained options for putting all arguments on one line.
Another bump on this - again, this is my first time trying to submit a patch, so please let me know if there's a better way to go about this. On Mon, Nov 20, 2017 at 10:22 AM, Russell McClellan wrote: > Hi - > > Just pinging this after a week; let me know if there's a better way to > submit patches, I'm a first time contributor. > > On Mon, Nov 13, 2017 at 6:55 PM, Russell McClellan > wrote: >> Attached is a patch that adds two new options, >> AllowAllArgumentsOnNextLine and >> AllowAllConstructorInitializersOnNextLine. These mirror the existing >> AllowAllParametersOfDeclarationOnNextLine and allow me to support an >> internal style guide where I work. I think this would be generally >> useful, some have asked for it on stackoverflow: >> >> https://stackoverflow.com/questions/30057534/clang-format-binpackarguments-not-working-as-expected >> >> https://stackoverflow.com/questions/38635106/clang-format-how-to-prevent-all-function-arguments-on-next-line >> >> Thanks for your consideration, and let me know if there's anything I >> can do to improve the patch; I'm a first-time contributor. >> >> Thanks, >> -Russell ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases.
NoQ created this revision.
Herald added subscribers: rnkovacs, eraman.
Under the assumption of `-analyzer-config c++-allocator-inlining=true`, which
enables evaluation of `operator new` as a regular function call, this patch
shows what it takes to actually inline the constructor into the return value of
such operator call.
The CFG for a `new C()` expression looks like:
- `CXXNewAllocator` `new C()` (a special kind of `CFGElement` on which operator
new is being evaluated)
- `CXXConstructExpr` `C()` (a regular `CFGStmt` element on which we call the
constructor)
- `CXXNewExpr` `new C()` (a regular `CFGStmt` element on which we should
ideally have nothing to do)
What i did here is:
1. First, i relax the requirements for constructor regions, as discussed on the
mailing list
(http://lists.llvm.org/pipermail/cfe-dev/2017-November/056095.html). We can now
construct into `ElementRegion` unless it actually represents an array element
(we're not dealing with `operator new[]` yet). Also we remove the explicit
bailout for constructions that correspond to operator new parent expressions,
as long as `-analyzer-config c++-allocator-inlining` is enabled. See changes in
`mayInlineCallKind()`.
2. Then, when the constructor is being modeled, we're trying to get back the
value returned by `operator new`. This is done similarly to other construction
cases - by finding the next (!!) element in the CFG, figuring out that it's a
`CXXNewExpr`, and then taking the respective region to construct into from the
Environment. The `CXXNewAllocator` element is not relied upon on this phase -
for now it only triggers the evaluation of `operator new` at the right moment,
so that we had the return value. See changes in
`getRegionForConstructedObject()` and `canHaveDirectConstructor()`.
3. When we reach the actual `CXXNewExpr` CFG element (the third one), we need
to preserve the value we already have for the `CXXNewExpr` in the environment.
The old code that's conjuring a (heap) pointer here would probably still remain
to handle the case when an inlined `operator new` has actually returned an
`UnknownVal`. Still, this needs polishing, as the FIXME at the top of
`VisitCXXNewExpr()` suggests. See the newly added hack in `VisitCXXNewExpr()`.
4. Finally, the `CXXNewExpr` value keeps dying in the Environment. From the
point of view of the current liveness analysis, the new-expression is dead (or
rather not yet born) until the `CXXNewExpr` statement element is reached, so it
immediately gets purged on every step. The really dirty code that prevents
this, //that should never be committed//, is in `shouldRemoveDeadBindings()`:
for the sake of this proof-of-concept, i've crudely disabled garbage collection
on the respective moments of time. I believe that the proper fix would be on
the liveness analysis side: mark the new-expression as live between the
`CXXNewAllocator` element and `CXXNewExpr` statement element.
My todo list before committing this would be:
1. Fix the live expressions hack.
2. See how reliable is `findElementDirectlyInitializedByCurrentConstructor()`
in this case.
3. Understand how my code works for non-object constructors (eg. `new int`).
4. See how much `VisitCXXNewExpr` can be refactored.
Once this lands, i think we should be looking into enabling `-analyzer-config
c++-allocator-inlining` by default.
https://reviews.llvm.org/D40560
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/inline.cpp
test/Analysis/new-ctor.cpp
Index: test/Analysis/new-ctor.cpp
===
--- /dev/null
+++ test/Analysis/new-ctor.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct S {
+ int x;
+ S() : x(1) {}
+ ~S() {}
+};
+
+void foo() {
+ S *s = new S;
+ clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
+}
Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -315,17 +315,13 @@
int value;
IntWrapper(int input) : value(input) {
- // We don't want this constructor to be inlined unless we can actually
- // use the proper region for operator new.
- // See PR12014 and .
- clang_analyzer_checkInlined(false); // no-warning
+ clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
}
};
void test() {
IntWrapper *obj = new IntWrapper(42);
-// should be TRUE
-clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(obj->value == 42); // expected-warning{{TRUE}}
delete obj;
}
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
[PATCH] D40561: [libclang] Fix cursors for functions with trailing return type
nik created this revision.
For the function declaration
auto foo5(Foo) -> Foo;
the parameter tokens were mapped to cursors representing the
FunctionDecl:
Keyword: "auto" [1:1 - 1:5] FunctionDecl=test5:1:6
Identifier: "test5" [1:6 - 1:11] FunctionDecl=test5:1:6
Punctuation: "(" [1:11 - 1:12] FunctionDecl=test5:1:6
Identifier: "X" [1:12 - 1:13] FunctionDecl=test5:1:6 // Ops, not a TypeRef
Punctuation: ")" [1:13 - 1:14] FunctionDecl=test5:1:6
Punctuation: "->" [1:15 - 1:17] FunctionDecl=test5:1:6
Identifier: "X" [1:18 - 1:19] TypeRef=struct X:7:8
Punctuation: ";" [1:19 - 1:20]
Fix this by ensuring that the trailing return type is not visited as
first.
https://reviews.llvm.org/D40561
Files:
test/Index/annotate-tokens.cpp
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -783,6 +783,16 @@
return false;
}
+static bool HasTrailingReturnType(FunctionDecl *ND) {
+ const QualType Ty = ND->getType();
+ if (const FunctionType *AFT = Ty->getAs()) {
+if (const FunctionProtoType *FT = dyn_cast(AFT))
+ return FT->hasTrailingReturn();
+ }
+
+ return false;
+}
+
/// \brief Compare two base or member initializers based on their source order.
static int CompareCXXCtorInitializers(CXXCtorInitializer *const *X,
CXXCtorInitializer *const *Y) {
@@ -802,14 +812,16 @@
// written. This requires a bit of work.
TypeLoc TL = TSInfo->getTypeLoc().IgnoreParens();
FunctionTypeLoc FTL = TL.getAs();
+const bool HasTrailingRT = HasTrailingReturnType(ND);
// If we have a function declared directly (without the use of a typedef),
// visit just the return type. Otherwise, just visit the function's type
// now.
-if ((FTL && !isa(ND) && Visit(FTL.getReturnLoc())) ||
+if ((FTL && !isa(ND) && !HasTrailingRT &&
+ Visit(FTL.getReturnLoc())) ||
(!FTL && Visit(TL)))
return true;
-
+
// Visit the nested-name-specifier, if present.
if (NestedNameSpecifierLoc QualifierLoc = ND->getQualifierLoc())
if (VisitNestedNameSpecifierLoc(QualifierLoc))
@@ -825,7 +837,11 @@
// Visit the function parameters, if we have a function type.
if (FTL && VisitFunctionTypeLoc(FTL, true))
return true;
-
+
+// Visit the function's trailing return type.
+if (FTL && HasTrailingRT && Visit(FTL.getReturnLoc()))
+ return true;
+
// FIXME: Attributes?
}
Index: test/Index/annotate-tokens.cpp
===
--- test/Index/annotate-tokens.cpp
+++ test/Index/annotate-tokens.cpp
@@ -37,7 +37,9 @@
~C();
};
-// RUN: c-index-test -test-annotate-tokens=%s:1:1:38:1 %s
-fno-delayed-template-parsing | FileCheck %s
+auto test5(X) -> X;
+
+// RUN: c-index-test -test-annotate-tokens=%s:1:1:41:1 %s -std=c++14
-fno-delayed-template-parsing | FileCheck %s
// CHECK: Keyword: "struct" [1:1 - 1:7] StructDecl=bonk:1:8 (Definition)
// CHECK: Identifier: "bonk" [1:8 - 1:12] StructDecl=bonk:1:8 (Definition)
// CHECK: Punctuation: "{" [1:13 - 1:14] StructDecl=bonk:1:8 (Definition)
@@ -184,6 +186,14 @@
// CHECK: Punctuation: "}" [29:22 - 29:23] CompoundStmt=
// CHECK: Punctuation: "~" [37:3 - 37:4] CXXDestructor=~C:37:3
// CHECK: Identifier: "C" [37:4 - 37:5] CXXDestructor=~C:37:3
+// CHECK: Keyword: "auto" [40:1 - 40:5] FunctionDecl=test5:40:6
+// CHECK: Identifier: "test5" [40:6 - 40:11] FunctionDecl=test5:40:6
+// CHECK: Punctuation: "(" [40:11 - 40:12] FunctionDecl=test5:40:6
+// CHECK: Identifier: "X" [40:12 - 40:13] TypeRef=struct X:7:8
+// CHECK: Punctuation: ")" [40:13 - 40:14] FunctionDecl=test5:40:6
+// CHECK: Punctuation: "->" [40:15 - 40:17] FunctionDecl=test5:40:6
+// CHECK: Identifier: "X" [40:18 - 40:19] TypeRef=struct X:7:8
+// CHECK: Punctuation: ";" [40:19 - 40:20]
// RUN: env LIBCLANG_DISABLE_CRASH_RECOVERY=1 c-index-test
-test-annotate-tokens=%s:32:1:32:13 %s | FileCheck %s -check-prefix=CHECK2
// CHECK2: Keyword: "if" [32:3 - 32:5] IfStmt=
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -783,6 +783,16 @@
return false;
}
+static bool HasTrailingReturnType(FunctionDecl *ND) {
+ const QualType Ty = ND->getType();
+ if (const FunctionType *AFT = Ty->getAs()) {
+if (const FunctionProtoType *FT = dyn_cast(AFT))
+ return FT->hasTrailingReturn();
+ }
+
+ return false;
+}
+
/// \brief Compare two base or member initializers based on their source order.
static int CompareCXXCtorInitializers(CXXCtorInitializer *const *X,
CXXCtorInitializer *const *Y) {
@@ -802,14 +812,16 @@
// written. This requires a bit of work.
TypeLoc TL = TSInfo->getTypeLoc().IgnoreParens();
FunctionTypeLoc FTL =
[PATCH] D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases.
NoQ added a comment. > for the sake of this proof-of-concept, i've crudely disabled garbage > collection on the respective moments of time Forgot to mention that this breaks tests in `NewDeleteLeaks-PR19102.cpp`, which are still failing in the present revision. Leak warnings get delayed to much later lines of code here. This example shows that this liveness hack may delay garbage collection indefinitely. https://reviews.llvm.org/D40560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r319178 - Refactor functions PrintTemplateArgumentList
Author: sepavloff
Date: Tue Nov 28 08:14:14 2017
New Revision: 319178
URL: http://llvm.org/viewvc/llvm-project?rev=319178&view=rev
Log:
Refactor functions PrintTemplateArgumentList
These functions were defined as static members of TemplateSpecializationType.
Now they are moved to namespace level. Previously there were different
implementations for lists containing TemplateArgument and TemplateArgumentLoc,
now these implementations share the same code.
This change is a result of refactoring patch D40508. NFC.
Modified:
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclTemplate.cpp
cfe/trunk/lib/AST/NestedNameSpecifier.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/tools/libclang/CIndex.cpp
Modified: cfe/trunk/include/clang/AST/Type.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=319178&r1=319177&r2=319178&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Nov 28 08:14:14 2017
@@ -4533,21 +4533,6 @@ public:
static bool anyDependentTemplateArguments(const TemplateArgumentListInfo &,
bool &InstantiationDependent);
- /// \brief Print a template argument list, including the '<' and '>'
- /// enclosing the template arguments.
- static void PrintTemplateArgumentList(raw_ostream &OS,
-ArrayRef Args,
-const PrintingPolicy &Policy,
-bool SkipBrackets = false);
-
- static void PrintTemplateArgumentList(raw_ostream &OS,
-ArrayRef Args,
-const PrintingPolicy &Policy);
-
- static void PrintTemplateArgumentList(raw_ostream &OS,
-const TemplateArgumentListInfo &,
-const PrintingPolicy &Policy);
-
/// True if this template specialization type matches a current
/// instantiation in the context in which it is found.
bool isCurrentInstantiation() const {
@@ -4623,6 +4608,20 @@ public:
}
};
+/// \brief Print a template argument list, including the '<' and '>'
+/// enclosing the template arguments.
+void printTemplateArgumentList(raw_ostream &OS,
+ ArrayRef Args,
+ const PrintingPolicy &Policy);
+
+void printTemplateArgumentList(raw_ostream &OS,
+ ArrayRef Args,
+ const PrintingPolicy &Policy);
+
+void printTemplateArgumentList(raw_ostream &OS,
+ const TemplateArgumentListInfo &Args,
+ const PrintingPolicy &Policy);
+
/// The injected class name of a C++ class template or class
/// template partial specialization. Used to record that a type was
/// spelled with a bare identifier rather than as a template-id; the
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=319178&r1=319177&r2=319178&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Nov 28 08:14:14 2017
@@ -6274,9 +6274,8 @@ void ASTContext::getObjCEncodingForTypeI
= dyn_cast(RDecl)) {
const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
llvm::raw_string_ostream OS(S);
-TemplateSpecializationType::PrintTemplateArgumentList(OS,
-TemplateArgs.asArray(),
-(*this).getPrintingPolicy());
+printTemplateArgumentList(OS, TemplateArgs.asArray(),
+ getPrintingPolicy());
}
} else {
S += '?';
Modified: cfe/trunk/lib/AST/Decl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=319178&r1=319177&r2=319178&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Tue Nov 28 08:14:14 2017
@@ -1506,8 +1506,7 @@ void NamedDecl::printQualifiedName(raw_o
if (const auto *Spec = dyn_cast(DC)) {
OS << Spec->getName();
const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
- TemplateSpecializationType::PrintTemplateArgumentList(
- OS, TemplateArgs.asArray(), P);
+ printTemplateArgumentList(OS, TemplateArgs.asArray(), P);
} else if (const auto *ND = dyn_cast(DC)) {
if (P.SuppressUnwrittenScope &&
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
alexfh updated this revision to Diff 124580.
alexfh added a comment.
Herald added a subscriber: rnkovacs.
- Add dependencies to prevent a link error
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40507
Files:
clang-tidy/hicpp/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstantArgumentCheck.cpp
clang-tidy/misc/MoveConstantArgumentCheck.h
clang-tidy/misc/NoexceptMoveConstructorCheck.cpp
clang-tidy/misc/NoexceptMoveConstructorCheck.h
clang-tidy/modernize/PassByValueCheck.cpp
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/MoveConstArgCheck.cpp
clang-tidy/performance/MoveConstArgCheck.h
clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tidy/performance/NoexceptMoveConstructorCheck.h
clang-tidy/performance/PerformanceTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/hicpp-move-const-arg.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-move-const-arg.rst
docs/clang-tidy/checks/misc-noexcept-move-constructor.rst
docs/clang-tidy/checks/performance-move-const-arg.rst
docs/clang-tidy/checks/performance-noexcept-move-constructor.rst
test/clang-tidy/misc-move-const-arg.cpp
test/clang-tidy/misc-noexcept-move-constructor.cpp
test/clang-tidy/modernize-pass-by-value.cpp
test/clang-tidy/performance-move-const-arg.cpp
test/clang-tidy/performance-noexcept-move-constructor.cpp
Index: test/clang-tidy/performance-noexcept-move-constructor.cpp
===
--- test/clang-tidy/performance-noexcept-move-constructor.cpp
+++ test/clang-tidy/performance-noexcept-move-constructor.cpp
@@ -1,16 +1,16 @@
-// RUN: %check_clang_tidy %s misc-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
class A {
A(A &&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
A &operator=(A &&);
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
};
struct B {
static constexpr bool kFalse = false;
B(B &&) noexcept(kFalse);
- // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [misc-noexcept-move-constructor]
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
};
class OK {};
@@ -24,16 +24,16 @@
public:
OK1();
OK1(const OK1 &);
- OK1(OK1&&) noexcept;
+ OK1(OK1 &&) noexcept;
OK1 &operator=(OK1 &&) noexcept;
void f();
void g() noexcept;
};
class OK2 {
static constexpr bool kTrue = true;
-public:
+ public:
OK2(OK2 &&) noexcept(true) {}
OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; }
};
Index: test/clang-tidy/performance-move-const-arg.cpp
===
--- test/clang-tidy/performance-move-const-arg.cpp
+++ test/clang-tidy/performance-move-const-arg.cpp
@@ -1,31 +1,41 @@
-// RUN: %check_clang_tidy %s misc-move-const-arg %t
+// RUN: %check_clang_tidy %s performance-move-const-arg %t
namespace std {
-template struct remove_reference;
+template
+struct remove_reference;
-template struct remove_reference { typedef _Tp type; };
+template
+struct remove_reference {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
-template struct remove_reference<_Tp &&> { typedef _Tp type; };
+template
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
template
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
return static_cast::type &&>(__t);
}
-} // namespace std
+} // namespace std
class A {
-public:
+ public:
A() {}
A(const A &rhs) {}
A(A &&rhs) {}
};
int f1() {
return std::move(42);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [misc-move-const-arg]
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [performance-move-const-arg]
// CHECK-FIXES: return 42;
}
@@ -45,11 +55,14 @@
A f5(const A x5) {
return std::move(x5);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [misc-move-const-arg]
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg]
// C
[PATCH] D39730: Enabling constructor code completion
jkorous-apple added a comment. No luck. If I understand it correctly all the information that would be needed to distinguish between: foo:: and int i = foo:: in ResultBuilder::AddResult() is missing since the context is represented only by DeclContext. Something like a DirectASTParentNode would be necessary instead. https://reviews.llvm.org/D39730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
alexfh marked 2 inline comments as done.
alexfh added inline comments.
Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:33
+#include "../performance/MoveConstArgCheck.h"
+#include "../performance/NoexceptMoveConstructorCheck.h"
#include "../readability/BracesAroundStatementsCheck.h"
xazax.hun wrote:
> Don't you need to add performance module to link to the HICPP module to avoid
> link errors?
Indeed. I should probably teach the script to do this ;)
Comment at: clang-tidy/performance/MoveConstArgCheck.h:19
-class MoveConstantArgumentCheck : public ClangTidyCheck {
+class MoveConstArgCheck : public ClangTidyCheck {
public:
xazax.hun wrote:
> Is there any specific reason to rename this class? I am ok with the change,
> just wondering.
The reason is to make the class name consistent with the check name (which is
the case for all checks added by the add_new_check script, but not the case for
some checks that predate this script).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40507
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40256: [ARM] disable FPU features when using soft floating point.
keith.walker.arm updated this revision to Diff 124579.
keith.walker.arm added a comment.
> What are these disabled "R-UN" lines?
Oops! They shouldn't have been disabled in the patch.
> Do we really need to check every combination like this? I don't think the
> underlying logic actually varies.
I have reduced the tests in the part to do a check against each architectural
variant, and then just one check with the different -msoft-float/-mfpu=none
options.
https://reviews.llvm.org/D40256
Files:
lib/Driver/ToolChains/Arch/ARM.cpp
test/Driver/arm-cortex-cpus.c
test/Driver/arm-dotprod.c
test/Driver/arm-mfpu.c
test/Preprocessor/arm-target-features.c
Index: test/Driver/arm-dotprod.c
===
--- test/Driver/arm-dotprod.c
+++ test/Driver/arm-dotprod.c
@@ -4,8 +4,21 @@
// RUN: %clang -### -target arm -march=armv8.3a %s 2>&1 | FileCheck %s --check-prefix=CHECK-NONE
// CHECK-NONE-NOT: "-target-feature" "+dotprod"
-// RUN: %clang -### -target arm -march=armv8.2a+dotprod %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target arm -march=armv8.3a+dotprod %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target arm -mcpu=cortex-a75 %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target arm -mcpu=cortex-a55 %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target arm-linux-eabi -march=armv8.2a+dotprod %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target arm-linux-eabi -march=armv8.3a+dotprod %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target arm-linux-eabi -mcpu=cortex-a75 %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target arm-linux-eabi -mcpu=cortex-a55 %s 2>&1 | FileCheck %s
// CHECK: "+dotprod"
+
+// The following default to -msoft-float
+// RUN: %clang -### -target arm -march=armv8.2a+dotprod %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-NO-DOTPROD
+// RUN: %clang -### -target arm -march=armv8.3a+dotprod %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-NO-DOTPROD
+// RUN: %clang -### -target arm -mcpu=cortex-a75 %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-NO-DOTPROD
+// RUN: %clang -### -target arm -mcpu=cortex-a55 %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-NO-DOTPROD
+// We rely on the backend disabling dotprod as it depends on neon, so check that
+// neon is disabled after the dotprod was enabled.
+// CHECK-NO-DOTPROD-NOT: "+dotprod"
Index: test/Driver/arm-cortex-cpus.c
===
--- test/Driver/arm-cortex-cpus.c
+++ test/Driver/arm-cortex-cpus.c
@@ -284,13 +284,13 @@
// RUN: %clang -target arm -march=armebv8.2-a -mbig-endian -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V82A-THUMB %s
// CHECK-BE-V82A-THUMB: "-cc1"{{.*}} "-triple" "thumbebv8.2a-{{.*}}" "-target-cpu" "generic"
-// RUN: %clang -target armv8a -march=armv8.2-a+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-V82A-FP16 %s
+// RUN: %clang -target armv8a-linux-eabi -march=armv8.2-a+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-V82A-FP16 %s
// CHECK-V82A-FP16: "-cc1"{{.*}} "-triple" "armv8.2{{.*}}" "-target-cpu" "generic" {{.*}}"-target-feature" "+fullfp16"
// Once we have CPUs with optional v8.2-A FP16, we will need a way to turn it
// on and off. Cortex-A53 is a placeholder for now.
-// RUN: %clang -target armv8a -mcpu=cortex-a53+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-FP16 %s
-// RUN: %clang -target armv8a -mcpu=cortex-a53+nofp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-NOFP16 %s
+// RUN: %clang -target armv8a-linux-eabi -mcpu=cortex-a53+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-FP16 %s
+// RUN: %clang -target armv8a-linux-eabi -mcpu=cortex-a53+nofp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-NOFP16 %s
// CHECK-CORTEX-A53-FP16: "-cc1" {{.*}}"-target-cpu" "cortex-a53" {{.*}}"-target-feature" "+fullfp16"
// CHECK-CORTEX-A53-NOFP16: "-cc1" {{.*}}"-target-cpu" "cortex-a53" {{.*}}"-target-feature" "-fullfp16"
Index: test/Driver/arm-mfpu.c
===
--- test/Driver/arm-mfpu.c
+++ test/Driver/arm-mfpu.c
@@ -2,6 +2,8 @@
// RUN: %clang -target arm-linux-eabi %s -### -o %t.o 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-DEFAULT %s
+// CHECK-DEFAULT-NOT: "-target-feature" "+soft-float"
+// CHECK-DEFAULT: "-target-feature" "+soft-float-abi"
// CHECK-DEFAULT-NOT: "-target-feature" "+vfp2"
// CHECK-DEFAULT-NOT: "-target-feature" "+vfp3"
// CHECK-DEFAULT-NOT: "-target-feature" "+d16"
@@ -19,6 +21,10 @@
// RUN: %clang -target arm-linux-eabi -mfpu=vfp %s -### -o %t.o 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-VFP %s
+// RUN: %clang -target arm-linux-eabi -mfpu=vfp %s -mfloat-abi=soft -### -o %t.o 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-SOFT-ABI-FP %s
+// CHECK-VFP-NOT: "-target-feature" "+soft-float"
+// CHECK-VFP: "-target-feature" "+soft-float-abi"
// CHECK-VFP: "-target-feature" "+vfp2"
// CH
[PATCH] D40310: Restructure how we break tokens.
klimek updated this revision to Diff 124581.
klimek marked 3 inline comments as done.
klimek added a comment.
Address review comments.
https://reviews.llvm.org/D40310
Files:
lib/Format/BreakableToken.cpp
lib/Format/BreakableToken.h
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1096,11 +1096,12 @@
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
+ // FIXME: Do we need to fix up the " */" at the end?
+ // It doesn't look like any of our current logic triggers this.
EXPECT_EQ("/* This is a long\n"
" * comment that\n"
-" * doesn't\n"
-" * fit on one line.\n"
-" */",
+" * doesn't fit on\n"
+" * one line. */",
format("/* "
"This is a long "
"comment that "
@@ -2102,6 +2103,66 @@
EXPECT_EQ("///", format(" /// ", getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
+ // FIXME: This assumes we do not continue compressing whitespace once we are
+ // in reflow mode. Consider compressing whitespace.
+
+ // Test that we stop reflowing precisely at the column limit.
+ // After reflowing, "// reflows into foo" does not fit the column limit,
+ // so we compress the whitespace.
+ EXPECT_EQ("// some text that\n"
+"// reflows into foo\n",
+format("// some text that reflows\n"
+ "// into foo\n",
+ getLLVMStyleWithColumns(20)));
+ // Given one more column, "// reflows into foo" does fit the limit, so we
+ // do not compress the whitespace.
+ EXPECT_EQ("// some text that\n"
+"// reflows into foo\n",
+format("// some text that reflows\n"
+ "// into foo\n",
+ getLLVMStyleWithColumns(21)));
+}
+
+TEST_F(FormatTestComments, ReflowsCommentsWithExtraWhitespace) {
+ // Baseline.
+ EXPECT_EQ("// some text\n"
+"// that re flows\n",
+format("// some text that\n"
+ "// re flows\n",
+ getLLVMStyleWithColumns(16)));
+ EXPECT_EQ("// some text\n"
+"// that re flows\n",
+format("// some text that\n"
+ "// reflows\n",
+ getLLVMStyleWithColumns(16)));
+ EXPECT_EQ("/* some text\n"
+" * that re flows\n"
+" */\n",
+format("/* some text that\n"
+ "* re flows\n"
+ "*/\n",
+ getLLVMStyleWithColumns(16)));
+ // FIXME: We do not reflow if the indent of two subsequent lines differs;
+ // given that this is different behavior from block comments, do we want
+ // to keep this?
+ EXPECT_EQ("// some text\n"
+"// that\n"
+"// re flows\n",
+format("// some text that\n"
+ "// re flows\n",
+ getLLVMStyleWithColumns(16)));
+ // Space within parts of a line that fit.
+ // FIXME: Use the earliest possible split while reflowing to compress the
+ // whitespace within the line.
+ EXPECT_EQ("// some text that\n"
+"// does re flow\n"
+"// more here\n",
+format("// some text that does\n"
+ "// re flow more here\n",
+ getLLVMStyleWithColumns(21)));
+}
+
TEST_F(FormatTestComments, IgnoresIf0Contents) {
EXPECT_EQ("#if 0\n"
"}{)(&*(^%%#%@! fsadj f;ldjs ,:;| <<<>>>][)(][\n"
@@ -2484,6 +2545,7 @@
" long */\n"
" b);",
format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(16)));
+
EXPECT_EQ(
"a = f(\n"
"a,\n"
@@ -2888,16 +2950,15 @@
getLLVMStyleWithColumns(20)));
}
-TEST_F(FormatTestComments, NoCrush_Bug34236) {
+TEST_F(FormatTestComments, NoCrash_Bug34236) {
// This is a test case from a crasher reported in:
// https://bugs.llvm.org/show_bug.cgi?id=34236
// Temporarily disable formatting for readability.
// clang-format off
EXPECT_EQ(
"/**/ /*\n"
" * a\n"
-" * b c\n"
-" * d*/",
+" * b c d*/",
format(
"/**/ /*\n"
" * a b\n"
Index: unittests/Format/FormatTest.cpp
==
[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.
ioeric created this revision.
... in qualified code completion and decl lookup.
https://reviews.llvm.org/D40562
Files:
lib/Sema/SemaCodeComplete.cpp
lib/Sema/SemaLookup.cpp
test/CodeCompletion/ignore-global-decls.cpp
Index: test/CodeCompletion/ignore-global-decls.cpp
===
--- /dev/null
+++ test/CodeCompletion/ignore-global-decls.cpp
@@ -0,0 +1,20 @@
+namespace ns {
+ struct bar {
+ };
+
+ struct baz {
+ };
+
+ int func(int a, bar b, baz c);
+}
+
+void test() {
+ ns::
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 %s -o - |
FileCheck %s --check-prefix=CHECK-1
+// CHECK-1-DAG: COMPLETION: bar : bar
+// CHECK-1-DAG: COMPLETION: baz : baz
+// CHECK-1-DAG: COMPLETION: func : [#int#]func(<#int a#>, <#bar b#>, <#baz c#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7
-no-code-completion-globals %s -o - | FileCheck %s -allow-empty
--check-prefix=CHECK-EMPTY
+// CHECK-EMPTY: {{^}}{{$}}
+}
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3785,8 +3785,12 @@
LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind);
Result.setAllowHidden(Consumer.includeHiddenDecls());
VisibleDeclsRecord Visited;
- if (!IncludeGlobalScope)
+ if (!IncludeGlobalScope) {
Visited.visitedContext(Context.getTranslationUnitDecl());
+// Declarations in namespace are also considered global.
+if (Ctx->isNamespace() || Ctx->isTranslationUnit())
+ Visited.visitedContext(Ctx);
+ }
ShadowContextRAII Shadow(Visited);
::LookupVisibleDecls(Ctx, Result, /*QualifiedNameLookup=*/true,
/*InBaseClass=*/false, Consumer, Visited,
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4639,7 +4639,7 @@
CodeCompletionDeclConsumer Consumer(Results, CurContext);
LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
- /*IncludeGlobalScope=*/true,
+ CodeCompleter->includeGlobals(),
/*IncludeDependentBases=*/true);
HandleCodeCompleteResults(this, CodeCompleter,
Index: test/CodeCompletion/ignore-global-decls.cpp
===
--- /dev/null
+++ test/CodeCompletion/ignore-global-decls.cpp
@@ -0,0 +1,20 @@
+namespace ns {
+ struct bar {
+ };
+
+ struct baz {
+ };
+
+ int func(int a, bar b, baz c);
+}
+
+void test() {
+ ns::
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 %s -o - | FileCheck %s --check-prefix=CHECK-1
+// CHECK-1-DAG: COMPLETION: bar : bar
+// CHECK-1-DAG: COMPLETION: baz : baz
+// CHECK-1-DAG: COMPLETION: func : [#int#]func(<#int a#>, <#bar b#>, <#baz c#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:7 -no-code-completion-globals %s -o - | FileCheck %s -allow-empty --check-prefix=CHECK-EMPTY
+// CHECK-EMPTY: {{^}}{{$}}
+}
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3785,8 +3785,12 @@
LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind);
Result.setAllowHidden(Consumer.includeHiddenDecls());
VisibleDeclsRecord Visited;
- if (!IncludeGlobalScope)
+ if (!IncludeGlobalScope) {
Visited.visitedContext(Context.getTranslationUnitDecl());
+// Declarations in namespace are also considered global.
+if (Ctx->isNamespace() || Ctx->isTranslationUnit())
+ Visited.visitedContext(Ctx);
+ }
ShadowContextRAII Shadow(Visited);
::LookupVisibleDecls(Ctx, Result, /*QualifiedNameLookup=*/true,
/*InBaseClass=*/false, Consumer, Visited,
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4639,7 +4639,7 @@
CodeCompletionDeclConsumer Consumer(Results, CurContext);
LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
- /*IncludeGlobalScope=*/true,
+ CodeCompleter->includeGlobals(),
/*IncludeDependentBases=*/true);
HandleCodeCompleteResults(this, CodeCompleter,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40310: Restructure how we break tokens.
klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1525 + if (!DryRun) +Token->adaptStartOfLine(0, Whitespaces); + krasimir wrote: > If we indent here, shouldn't that also change ContentStartColumn? Nope, this will exactly adapt the start of the line to ContentStartColumn. ContentStartColumn is where the line thinks it wants to be, not where it is. Comment at: lib/Format/ContinuationIndenter.cpp:1557 if (LineIndex < EndIndex - 1) + // The last line's penalty is handled in addNextStateToQueue(). Penalty += Style.PenaltyExcessCharacter * krasimir wrote: > How does the last line's penalty get handled in addNextStateToQueue()? By the State's Column being above the column limit. Comment at: lib/Format/ContinuationIndenter.cpp:1578 +LineIndex, TailOffset + Split.first + Split.second, ColumnLimit, +ContentStartColumn + ToSplitColumns, CommentPragmasRegex); +// Compute the comlumns necessary to fit the next non-breakable sequence krasimir wrote: > krasimir wrote: > > nit: `BreakableToken::Split NextSplit = Token->getSplit(...)` > Hm, right now `ContentStartColumn + ToSplitColumns` points to the column > where the character at offset `TailOffset + Split.first` is supposed to be. > Then `NextSplit` is relative to the offset `TailOffset + Split.first + > Split.second`, so IMO it shouldn't use `ContentStartColumn + ToSplitColumns` > as a start column. I think that `ToSplitColumns` needs to be computed as > follows: > ``` > unsigned ToSplitColumns = Token->getRangeLength(LineIndex, TailOffset, > Split.first + Split.second, ContentStartColumn); > ``` > Also, a comment of the intended meaning of `ToSplitColumns` would be helpful. Good catch, but the solution is differnet. Using ContentStartColumn + ToSplitColumns is incorrect, we need ContentStartColumn + ToSplitColumns + 1 (as Split.second just contains the whitespace, but we want to consider that whitespace compressed). I tried to write a test for this, and convinced myself that while +1 is correct, it is currently impossible to change behavior based on the missing +1. https://reviews.llvm.org/D40310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r319183 - [clang-tidy] Move more checks from misc- to performance-
Author: alexfh Date: Tue Nov 28 08:41:03 2017 New Revision: 319183 URL: http://llvm.org/viewvc/llvm-project?rev=319183&view=rev Log: [clang-tidy] Move more checks from misc- to performance- Summary: rename_check.py misc-move-const-arg performance-move-const-arg rename_check.py misc-noexcept-move-constructor performance-noexcept-move-constructor Reviewers: hokein, xazax.hun Reviewed By: xazax.hun Subscribers: rnkovacs, klimek, mgorny, xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D40507 Added: clang-tools-extra/trunk/clang-tidy/performance/MoveConstArgCheck.cpp - copied, changed from r319174, clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp clang-tools-extra/trunk/clang-tidy/performance/MoveConstArgCheck.h - copied, changed from r319174, clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.h clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp - copied, changed from r319174, clang-tools-extra/trunk/clang-tidy/misc/NoexceptMoveConstructorCheck.cpp clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.h - copied, changed from r319174, clang-tools-extra/trunk/clang-tidy/misc/NoexceptMoveConstructorCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/performance-move-const-arg.rst - copied, changed from r319174, clang-tools-extra/trunk/docs/clang-tidy/checks/misc-move-const-arg.rst clang-tools-extra/trunk/docs/clang-tidy/checks/performance-noexcept-move-constructor.rst - copied, changed from r319174, clang-tools-extra/trunk/docs/clang-tidy/checks/misc-noexcept-move-constructor.rst clang-tools-extra/trunk/test/clang-tidy/performance-move-const-arg-trivially-copyable.cpp - copied, changed from r319174, clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp clang-tools-extra/trunk/test/clang-tidy/performance-move-const-arg.cpp - copied, changed from r319174, clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp clang-tools-extra/trunk/test/clang-tidy/performance-noexcept-move-constructor.cpp - copied, changed from r319174, clang-tools-extra/trunk/test/clang-tidy/misc-noexcept-move-constructor.cpp Removed: clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.h clang-tools-extra/trunk/clang-tidy/misc/NoexceptMoveConstructorCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/NoexceptMoveConstructorCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/misc-move-const-arg.rst clang-tools-extra/trunk/docs/clang-tidy/checks/misc-noexcept-move-constructor.rst clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp clang-tools-extra/trunk/test/clang-tidy/misc-noexcept-move-constructor.cpp Modified: clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp 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/hicpp-move-const-arg.rst clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Modified: clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt?rev=319183&r1=319182&r2=319183&view=diff == --- clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt Tue Nov 28 08:41:03 2017 @@ -17,6 +17,7 @@ add_clang_library(clangTidyHICPPModule clangTidyGoogleModule clangTidyMiscModule clangTidyModernizeModule + clangTidyPerformanceModule clangTidyReadabilityModule clangTidyUtils ) Modified: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp?rev=319183&r1=319182&r2=319183&view=diff == --- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp Tue Nov 28 08:41:03 2017 @@ -18,9 +18,7 @@ #include "../cppcoreguidelines/SpecialMemberFunctionsCheck.h" #include "../google/DefaultArgumentsCheck.h" #include "../google/ExplicitConstr
[PATCH] D40507: [clang-tidy] Move more checks from misc- to performance-
This revision was automatically updated to reflect the committed changes. alexfh marked 2 inline comments as done. Closed by commit rCTE319183: [clang-tidy] Move more checks from misc- to performance- (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D40507?vs=124580&id=124583#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40507 Files: clang-tidy/hicpp/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MoveConstantArgumentCheck.cpp clang-tidy/misc/MoveConstantArgumentCheck.h clang-tidy/misc/NoexceptMoveConstructorCheck.cpp clang-tidy/misc/NoexceptMoveConstructorCheck.h clang-tidy/modernize/PassByValueCheck.cpp clang-tidy/performance/CMakeLists.txt clang-tidy/performance/MoveConstArgCheck.cpp clang-tidy/performance/MoveConstArgCheck.h clang-tidy/performance/NoexceptMoveConstructorCheck.cpp clang-tidy/performance/NoexceptMoveConstructorCheck.h clang-tidy/performance/PerformanceTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/hicpp-move-const-arg.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/misc-move-const-arg.rst docs/clang-tidy/checks/misc-noexcept-move-constructor.rst docs/clang-tidy/checks/performance-move-const-arg.rst docs/clang-tidy/checks/performance-noexcept-move-constructor.rst test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp test/clang-tidy/misc-move-const-arg.cpp test/clang-tidy/misc-noexcept-move-constructor.cpp test/clang-tidy/modernize-pass-by-value.cpp test/clang-tidy/performance-move-const-arg-trivially-copyable.cpp test/clang-tidy/performance-move-const-arg.cpp test/clang-tidy/performance-noexcept-move-constructor.cpp Index: clang-tidy/hicpp/CMakeLists.txt === --- clang-tidy/hicpp/CMakeLists.txt +++ clang-tidy/hicpp/CMakeLists.txt @@ -17,6 +17,7 @@ clangTidyGoogleModule clangTidyMiscModule clangTidyModernizeModule + clangTidyPerformanceModule clangTidyReadabilityModule clangTidyUtils ) Index: clang-tidy/hicpp/HICPPTidyModule.cpp === --- clang-tidy/hicpp/HICPPTidyModule.cpp +++ clang-tidy/hicpp/HICPPTidyModule.cpp @@ -18,9 +18,7 @@ #include "../cppcoreguidelines/SpecialMemberFunctionsCheck.h" #include "../google/DefaultArgumentsCheck.h" #include "../google/ExplicitConstructorCheck.h" -#include "../misc/MoveConstantArgumentCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" -#include "../misc/NoexceptMoveConstructorCheck.h" #include "../misc/StaticAssertCheck.h" #include "../misc/UndelegatedConstructor.h" #include "../modernize/DeprecatedHeadersCheck.h" @@ -31,6 +29,8 @@ #include "../modernize/UseNoexceptCheck.h" #include "../modernize/UseNullptrCheck.h" #include "../modernize/UseOverrideCheck.h" +#include "../performance/MoveConstArgCheck.h" +#include "../performance/NoexceptMoveConstructorCheck.h" #include "../readability/BracesAroundStatementsCheck.h" #include "../readability/FunctionSizeCheck.h" #include "../readability/IdentifierNamingCheck.h" @@ -63,11 +63,11 @@ "hicpp-invalid-access-moved"); CheckFactories.registerCheck( "hicpp-member-init"); -CheckFactories.registerCheck( +CheckFactories.registerCheck( "hicpp-move-const-arg"); CheckFactories.registerCheck( "hicpp-new-delete-operators"); -CheckFactories.registerCheck( +CheckFactories.registerCheck( "hicpp-noexcept-move"); CheckFactories .registerCheck( Index: clang-tidy/performance/CMakeLists.txt === --- clang-tidy/performance/CMakeLists.txt +++ clang-tidy/performance/CMakeLists.txt @@ -7,7 +7,9 @@ InefficientAlgorithmCheck.cpp InefficientStringConcatenationCheck.cpp InefficientVectorOperationCheck.cpp + MoveConstArgCheck.cpp MoveConstructorInitCheck.cpp + NoexceptMoveConstructorCheck.cpp PerformanceTidyModule.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp Index: clang-tidy/performance/NoexceptMoveConstructorCheck.h === --- clang-tidy/performance/NoexceptMoveConstructorCheck.h +++ clang-tidy/performance/NoexceptMoveConstructorCheck.h @@ -0,0 +1,38 @@ +//===--- NoexceptMoveConstructorCheck.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_NOEXCEPTMOVECONSTRUCTORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H + +#include "../ClangTidy.h" + +namespac
[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.
ioeric created this revision.
https://reviews.llvm.org/D40563
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4603,9 +4603,15 @@
void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext) {
- if (!SS.getScopeRep() || !CodeCompleter)
+ if (!CodeCompleter)
return;
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+return;
+ }
// Always pretend to enter a context to ensure that a dependent type
// resolves to a dependent record.
DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
@@ -4621,7 +4627,7 @@
CodeCompleter->getCodeCompletionTUInfo(),
CodeCompletionContext::CCC_Name);
Results.EnterNewScope();
-
+
// The "template" keyword can follow "::" in the grammar, but only
// put it into the grammar if the nested-name-specifier is dependent.
NestedNameSpecifier *NNS = SS.getScopeRep();
@@ -4635,16 +4641,18 @@
// qualified-id completions.
if (!EnteringContext)
MaybeAddOverrideCalls(*this, Ctx, Results);
- Results.ExitScope();
-
+ Results.ExitScope();
+
CodeCompletionDeclConsumer Consumer(Results, CurContext);
LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
/*IncludeGlobalScope=*/true,
/*IncludeDependentBases=*/true);
- HandleCodeCompleteResults(this, CodeCompleter,
-Results.getCompletionContext(),
-Results.data(),Results.size());
+ auto CC = Results.getCompletionContext();
+ CC.setCXXScopeSpecifier(SS);
+
+ HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
}
void Sema::CodeCompleteUsing(Scope *S) {
Index: include/clang/Sema/CodeCompleteConsumer.h
===
--- include/clang/Sema/CodeCompleteConsumer.h
+++ include/clang/Sema/CodeCompleteConsumer.h
@@ -18,6 +18,7 @@
#include "clang/AST/DeclBase.h"
#include "clang/AST/Type.h"
#include "clang/Sema/CodeCompleteOptions.h"
+#include "clang/Sema/DeclSpec.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -280,6 +281,8 @@
/// \brief The identifiers for Objective-C selector parts.
ArrayRef SelIdents;
+ llvm::Optional ScopeSpecifier;
+
public:
/// \brief Construct a new code-completion context of the given kind.
CodeCompletionContext(enum Kind Kind) : Kind(Kind), SelIdents(None) { }
@@ -315,8 +318,20 @@
/// \brief Determines whether we want C++ constructors as results within this
/// context.
bool wantConstructorResults() const;
-};
+ /// \brief Sets the scope specifier that comes before the completion token.
+ /// This is expected to be set in code completions on qualfied specifiers
+ /// (e.g. "a::b::").
+ void setCXXScopeSpecifier(CXXScopeSpec SS) {
+this->ScopeSpecifier = std::move(SS);
+ }
+
+ llvm::Optional getCXXScopeSpecifier() {
+if (ScopeSpecifier)
+ return ScopeSpecifier.getPointer();
+return llvm::None;
+ }
+};
/// \brief A "string" used to describe how code completion can
/// be performed for an entity.
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4603,9 +4603,15 @@
void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext) {
- if (!SS.getScopeRep() || !CodeCompleter)
+ if (!CodeCompleter)
return;
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CC.setCXXScopeSpecifier(SS);
+HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
+return;
+ }
// Always pretend to enter a context to ensure that a dependent type
// resolves to a dependent record.
DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
@@ -4621,7 +4627,7 @@
CodeCompleter->getCodeCompletionTUInfo(),
CodeCompletionContext::CCC_Name);
Results.EnterNewScope();
-
+
// The "template" keyword can follow "::" in the grammar, but only
// put it into the grammar if the nested-name-specifier is dependent.
NestedNameSpecifier *NNS = SS.getScopeRep();
@@ -4635,16 +4641,18 @@
// qualified-id completions.
if (!EnteringContext)
MaybeAddOverrideCalls(*this, Ctx, Results);
- Results.ExitScope();
-
+ Results.ExitScope();
+
CodeCompletionDeclConsumer Consumer(Resul
[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.
sammccall created this revision.
Herald added subscribers: cfe-commits, klimek.
This makes the parse() functions about as short as they can be given the
current signature, and moves all array-traversal etc code to a
central location.
We keep the ability to distinguish between optional and required fields:
and we don't propagate parse errors for optional fields.
I've made most fields required per the LSP spec - the looseness we had
here was mostly a historical accident I think.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40564
Files:
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/initialize-params-invalid.test
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -45,4 +45,4 @@
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
Content-Length: 33
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -51,7 +51,7 @@
static URI fromUri(llvm::StringRef uri);
static URI fromFile(llvm::StringRef file);
- static URI parse(llvm::StringRef U) { return fromUri(U); }
+ static llvm::Optional parse(const json::Expr &U);
static json::Expr unparse(const URI &U);
friend bool operator==(const URI &LHS, const URI &RHS) {
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -24,6 +24,148 @@
using namespace clang;
using namespace clang::clangd;
+namespace {
+// Helper for mapping JSON objects onto our protocol structs. Intended use:
+// Optional parse(json::Expr E) {
+// ObjectParser O(E);
+// if (!O || !O.parse("mandatory_field", Result.MandatoryField))
+// return None;
+// O.parse("optional_field", Result.OptionalField);
+// return Result;
+// }
+// FIXME: the static methods here should probably become the public parse()
+// extension point. Overloading free functions allows us to uniformly handle
+// enums, vectors, etc.
+class ObjectParser {
+public:
+ ObjectParser(const json::Expr &E) : O(E.asObject()) {}
+
+ // True if the expression is an object.
+ operator bool() { return O; }
+
+ template bool parse(const char *Prop, T &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop))
+ return parse(*E, Out);
+return false;
+ }
+
+ // Optional requires special handling, because missing keys are OK.
+ template bool parse(const char *Prop, llvm::Optional &Out) {
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop)) {
+ return parse(*E, Out);
+}
+Out = None;
+return true;
+ }
+
+private:
+ // Primitives.
+ static bool parse(const json::Expr &E, std::string &Out) {
+if (auto S = E.asString()) {
+ Out = *S;
+ return true;
+}
+return false;
+ }
+
+ static bool parse(const json::Expr &E, int &Out) {
+if (auto S = E.asInteger()) {
+ Out = *S;
+ return true;
+}
+return false;
+ }
+
+ static bool parse(const json::Expr &E, bool &Out) {
+if (auto S = E.asBoolean()) {
+ Out = *S;
+ return true;
+}
+return false;
+ }
+
+ // Types with a parse() function.
+ template static bool parse(const json::Expr &E, T &Out) {
+if (auto Parsed = std::remove_reference::type::parse(E)) {
+ Out = std::move(*Parsed);
+ return true;
+}
+return false;
+ }
+
+ // Nullable values as Optional.
+ template
+ static bool parse(const json::Expr &E, llvm::Optional &Out) {
+if (E.asNull())
+ return true;
+T Result;
+if (!parse(E, Result))
+ return false;
+Out = std::move(Result);
+return true;
+ }
+
+ // Array values with std::vector type.
+ template
+ static bool parse(const json::Expr &E, std::vector &Out) {
+if (auto *A = E.asArray()) {
+ Out.clear();
+ Out.resize(A->size());
+ for (size_t I = 0; I < A->size(); ++I)
+if (!parse((*A)[I], Out[I]))
+ return false;
+ return true;
+}
+return false;
+ }
+
+ // Object values with std::map
+ template
+ static bool parse(const json::Expr &E, std::map &Out) {
+if (auto *O = E.asObject()) {
+ for (const auto &KV : *O)
+if (!parse(KV.second, Out[StringRef(KV.first)]))
+ return false;
+ return true;
+}
+return false;
+ }
+
+ // Special cased enums, which can't have T::parse() functions.
+ // FIXME: make everything free functions so there's no special casing.
+ static bool parse(const json::Expr &E, TraceLevel &Out) {
+if (auto S = E.asString()) {
+ if (*S == "off") {
+Out = TraceLevel::Off;
+return true;
+ } else if (*S == "me
[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy
juliehockett updated this revision to Diff 124586.
juliehockett marked an inline comment as done.
juliehockett added a comment.
Added missing newline
https://reviews.llvm.org/D40108
Files:
clang-tidy/CMakeLists.txt
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/DefaultArgumentsCheck.cpp
clang-tidy/fuchsia/DefaultArgumentsCheck.h
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-default-arguments.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/index.rst
test/clang-tidy/fuchsia-default-arguments.cpp
Index: test/clang-tidy/fuchsia-default-arguments.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-default-arguments.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s fuchsia-default-arguments %t
+
+int foo(int value = 5) { return value; }
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES: int foo(int value) { return value; }
+
+int f() {
+ foo();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NEXT: note: default parameter was declared here:
+ // CHECK-NEXT: int foo(int value = 5) { return value; }
+}
+
+int bar(int value) { return value; }
+
+int n() {
+ foo(0);
+ bar(0);
+}
+
+class Baz {
+public:
+ int a(int value = 5) { return value; }
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-FIXES: int a(int value) { return value; }
+
+ int b(int value) { return value; }
+};
+
+class Foo {
+ // Fix should be suggested in declaration
+ int a(int value = 53);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-FIXES: int a(int value);
+};
+
+// Fix shouldn't be suggested in implementation
+int Foo::a(int value) {
+ return value;
+}
+
+// Elided functions
+void f(int = 5) {};
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES: void f(int) {};
+
+void g(int) {};
+
+// Should not suggest fix for macro-defined parameters
+#define D(val) = val
+
+void h(int i D(5));
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES-NOT: void h(int i);
+
+void x(int i);
+void x(int i = 12);
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES: void x(int i);
+
+void x(int i) {}
+
+struct S {
+ void x(int i);
+};
+
+void S::x(int i = 12) {}
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES: void S::x(int i) {}
+
+int main() {
+ S s;
+ s.x();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NEXT: note: default parameter was declared here:
+ // CHECK-NEXT: void S::x(int i = 12) {}
+ x();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: calling a function that uses a default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NEXT: note: default parameter was declared here:
+ // CHECK-NEXT: void x(int i = 12);
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -61,6 +61,7 @@
``cert-`` Checks related to CERT Secure Coding Guidelines.
``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
``clang-analyzer-``Clang Static Analyzer checks.
+``fuchsia-`` Checks related to Fuchsia coding conventions.
``google-``Checks related to Google coding conventions.
``hicpp-`` Checks related to High Integrity C++ Coding Standard.
``llvm-`` Checks related to the LLVM coding conventions.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ fuchsia-default-arguments
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-default-arguments.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-default-arguments.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - fuchsia-default-arguments
+
+
[PATCH] D40508: Replace long type names in IR with hashes
sepavloff updated this revision to Diff 124587.
sepavloff added a comment.
Updated patch as part of it was committed in https://reviews.llvm.org/rL319178
https://reviews.llvm.org/D40508
Files:
include/clang/AST/PrettyPrinter.h
lib/AST/TypePrinter.cpp
lib/CodeGen/CodeGenTypes.cpp
test/CodeGenCXX/template-types.cpp
Index: test/CodeGenCXX/template-types.cpp
===
--- /dev/null
+++ test/CodeGenCXX/template-types.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
+
+// Taken from the test pr29160.cpp
+template
+struct Foo {
+ template
+ static void ignore() {}
+ Foo() { ignore(); }
+ struct Inner {};
+};
+
+struct Base {
+ Base();
+ ~Base();
+};
+
+#define STAMP(thiz, prev) using thiz = Foo< \
+ prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, \
+ prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, \
+ prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev, prev \
+ >;
+STAMP(A, Base);
+STAMP(B, A);
+STAMP(C, B);
+
+int main() {
+ C::Inner val_1;
+}
+
+// CHECK: %"struct.Foo TypeName;
- llvm::raw_svector_ostream OS(TypeName);
+ PrintingPolicy Policy = RD->getASTContext().getPrintingPolicy();
+ Policy.NotForCompilation = true;
+ llvm::raw_abbrev_ostream OS;
+ OS.setHash().setTrunc().setBeginMarker("...");
+
+ // Set truncation limit. Long value helps in debugging but can result in
+ // higher memory consumption.
+ OS.setLimit(400);
+
OS << RD->getKindName() << '.';
-
+
// Name the codegen type after the typedef name
// if there is no tag type name available
if (RD->getIdentifier()) {
+OS.startAbbrev();
// FIXME: We should not have to check for a null decl context here.
// Right now we do it because the implicit Obj-C decls don't have one.
if (RD->getDeclContext())
- RD->printQualifiedName(OS);
+ RD->printQualifiedName(OS, Policy);
else
RD->printName(OS);
} else if (const TypedefNameDecl *TDD = RD->getTypedefNameForAnonDecl()) {
+OS.startAbbrev();
// FIXME: We should not have to check for a null decl context here.
// Right now we do it because the implicit Obj-C decls don't have one.
if (TDD->getDeclContext())
- TDD->printQualifiedName(OS);
+ TDD->printQualifiedName(OS, Policy);
else
TDD->printName(OS);
} else
OS << "anon";
+ OS.stopAbbrev();
if (!suffix.empty())
OS << suffix;
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1540,9 +1540,13 @@
bool NeedSpace = false;
bool FirstArg = true;
for (const auto &Arg : Args) {
-// Print the argument into a string.
-SmallString<128> Buf;
-llvm::raw_svector_ostream ArgOS(Buf);
+SmallString<0> Buffer;
+std::unique_ptr BufOS;
+if (!Policy.NotForCompilation) {
+ // Print the argument into a string.
+ BufOS.reset(new llvm::raw_svector_ostream(Buffer));
+}
+llvm::raw_ostream &ArgOS = Policy.NotForCompilation ? OS : *BufOS;
const TemplateArgument &Argument = getArgument(Arg);
if (Argument.getKind() == TemplateArgument::Pack) {
if (Argument.pack_size() && !FirstArg)
@@ -1553,17 +1557,19 @@
OS << Comma;
Argument.print(Policy, ArgOS);
}
-StringRef ArgString = ArgOS.str();
+if (!Policy.NotForCompilation) {
+ StringRef ArgString = BufOS->str();
-// If this is the first argument and its string representation
-// begins with the global scope specifier ('::foo'), add a space
-// to avoid printing the diagraph '<:'.
-if (FirstArg && !ArgString.empty() && ArgString[0] == ':')
- OS << ' ';
+ // If this is the first argument and its string representation
+ // begins with the global scope specifier ('::foo'), add a space
+ // to avoid printing the diagraph '<:'.
+ if (FirstArg && !ArgString.empty() && ArgString[0] == ':')
+OS << ' ';
-OS << ArgString;
+ OS << ArgString;
-NeedSpace = (!ArgString.empty() && ArgString.back() == '>');
+ NeedSpace = (!ArgString.empty() && ArgString.back() == '>');
+}
FirstArg = false;
}
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -52,7 +52,7 @@
Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
IncludeNewlines(true), MSVCFormatting(false),
ConstantsAsWritten(false), SuppressImplicitBase(false),
- FullyQualifiedName(false) { }
+ FullyQualifiedName(false), NotForCompilation(false) { }
/// Adjust this printing policy for cases where it's known that we're
/// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -225,6 +225,10 @@
/// When true, print
[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement
NoQ added a comment. The reason why i don't want to commit the MAX/4 approach now (for `>`/`<` case) is that it has too little useful effects until the iterator checker is enabled by default. However, it is a core change that immediately affects all users with all its negative effects (such as performance and code complexity). When i say that (1) this approach has little useful effects and (2) this approach may cause performance issues, both (1) and (2) are only based on intuition (my or Devin's). If somebody investigates the impact of the MAX/4 change and shows that both concerns are in fact wrong (the approach is indeed very useful for all modeling and/or has negligible performance impact), i think it should land. Otherwise, i think it shouldn't land now, but delayed until the iterator checker is ready to be enabled by default. For the type extension approach, somebody simply needs to do the math and prove that it works soundly in all cases. Devin has been heroically coming up with counter-examples so far, but even if he doesn't find any, it doesn't mean that it works, so ideally we shouldn't make him do this. The thing about the MAX/4 approach is that the math is fairly obvious: it is clear that overflows or truncations never occur under these constraints, therefore school algebra rules apply. If the type extension approach is proven to be sound, and covers more cases than the MAX/4 approach, i'd gladly welcome it. https://reviews.llvm.org/D35109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.
NoQ added a comment. Hey wb! Get well :) Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:282 +} +// We might want to handle the case when the mutex lock function was inlined +// and returned an Unknown or Undefined value. a.sidorin wrote: > TODO? That'd make a terrible project of choice for people grepping for TODOs and FIXMEs, so I guess rather not. https://reviews.llvm.org/D37806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.
a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:271 } -assert(lockFail && lockSucc); -C.addTransition(lockFail); - +// We might want to handle the case when the mutex lock function was inlined +// and returned an Unknown or Undefined value. NoQ wrote: > zaks.anna wrote: > > This comment is repeated several times... > Because it is relevant in both places. Should i rephrase with "also"/"as > well"/"here too"? Small rephrasing will be good because it will clearly state that it is not a copy-paste error :) https://reviews.llvm.org/D37806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40508: Replace long type names in IR with hashes
sepavloff marked 3 inline comments as done.
sepavloff added a comment.
In https://reviews.llvm.org/D40508#936617, @rnk wrote:
> It's not clear to me that this abbreviation functionality should live in
> Support. Clang probably wants enough control over this (assuming we're doing
> it) that the logic should live in clang.
>
> I also think we might want to try solving this a completely different way:
> just don't bother emitting more than two template arguments for IR type
> names. We don't really need to worry about type name uniqueness or matching
> them across TUs.
Actually the intention is to have unique type names across different TUs.
I will publish the relevant patch a bit latter, but the problem we are solving
is in incorrect behavior of `llvm-link`. If one TU contains opaque type and the
other has the type definition, these two types must be merged into one. The
merge procedure relies on type names, so it is important to have the same type
names for types in different TUs that are equivalent in the sense of C++.
Comment at: include/clang/AST/PrettyPrinter.h:231
+ /// make things like breaking digraphs and '>>' in template parameters.
+ bool NotForCompilation : 1;
};
rjmccall wrote:
> This saves, what, a few spaces from a few thousand IR type names? I'm
> skeptical that this even offsets the code-size impact of adding this option.
Not these few spaces themselves make the issue. The real evil is that to insert
these spaces, `printTemplateArgumentList` had to print each template parameter
into intermediate stream. We could try to use `raw_abbrev_ostream` to reduce
memory consumption, it would not work because these intermediate streams are of
type `raw_svector_ostream` and all these huge parameter texts would be
materialized first and only then would go to compacting stream.
If no need to maintain compilable output, intermediate streams are not needed
and all input can go directly to `raw_abbrev_ostream`.
Comment at: lib/AST/TypePrinter.cpp:1532-1534
+namespace {
+template
+void printTo(raw_ostream &OS, ArrayRef Args, const PrintingPolicy &Policy,
rnk wrote:
> `static` is nicer than a short anonymous namespace.
Yes, but this is function template. It won't create symbol in object file.
Actually anonymous namespace has no effect here, it is only a documentation
hint.
https://reviews.llvm.org/D40508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input
erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2238-2240 + // All other pointers are unique. + if (Ty->isPointerType() || Ty->isMemberPointerType()) +return true; rsmith wrote: > erichkeane wrote: > > rsmith wrote: > > > This is not correct: member pointer representations can have padding > > > bits. In the MS ABI, a pointer to member function is a pointer followed > > > by 0-3 ints, and if there's an odd number of ints, I expect there'll be 4 > > > bytes of padding at the end of the representation on 64-bit targets. > > > > > > I think you'll need to either add a `clang::CXXABI` entry point for > > > determining whether a `MemberPointerType` has padding, or perhaps extend > > > the existing `getMemberPointerWidthAndAlign` to also provide this > > > information. > > Based on looking through the two, I think I can just do this as Width%Align > > == 0, right? I've got an incoming patch that does that, so hopefully that > > is sufficient. > I don't think that will work; the MS implementation *sometimes* rounds the > size up to the alignment. (... which sounds pretty dodgy to me; shouldn't the > returned size always be a multiple of the returned alignment?) Ah, right... I missed this part of the MicrosoftCXXABI: if (Target.getTriple().isArch64Bit()) Width = llvm::alignTo(Width, Align); So likely I can just do the same math as I implemented here, except before the width is reset in the alignTo here. Thanks for getting back so quickly! I'll get back on this once I get a patch for the array align error. https://reviews.llvm.org/D39347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40566: Check if MemberExpr arguments are type-dependent.
mattd created this revision.
Fixes: PR28290
When checking an argument list, arguments from the templated class instance,
were
returning 'is dependent' based on the 'this' pointer. In that case, arguments
were
being marked dependent, and name lookup was delayed until template
instantiation. This
had the side-effect of -fdelayed-template-parsing in certain cases
(attached test case), when that should not have been the case, especially
since that flag was never passed.
According to the standard the referenced member's type needs
to be checked:
[temp.dep.expr]p5:
A class member access expression (5.2.5) is type-dependent if the expression
refers to a member of the current instantiation and the type of the referenced
member is dependent, or the class member access expression refers to a member
of an unknown specialization.
This change decides if the argument belongs to a MemberExpr. If so, then the
type of the expression is checked if it is dependent or not.
https://reviews.llvm.org/D40566
Files:
lib/AST/Expr.cpp
test/CXX/class.access/class.access.dcl/p1.cpp
test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
test/SemaTemplate/template-with-invalid-decl.cpp
Index: test/SemaTemplate/template-with-invalid-decl.cpp
===
--- test/SemaTemplate/template-with-invalid-decl.cpp
+++ test/SemaTemplate/template-with-invalid-decl.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify %s
+
+class UBS;
+
+template struct FIBH
+{
+ FIBH() { GDN(BS); } // expected-error {{use of undeclared identifier 'GDN'}}
+ class UBS* BS;
+};
+
+extern void foo()
+{
+ FIBH IBH;
+}
+
+extern void GDN(UBS* BS);
Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -29,7 +29,7 @@
};
}
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2{{candidate constructor {{.*
namespace test1 {
struct A {
@@ -112,7 +112,7 @@
}
void test5() {
- Opaque0 _ = hiding;
+ Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int'
to 'Opaque0'}}
}
};
}
Index: test/CXX/class.access/class.access.dcl/p1.cpp
===
--- test/CXX/class.access/class.access.dcl/p1.cpp
+++ test/CXX/class.access/class.access.dcl/p1.cpp
@@ -56,7 +56,7 @@
};
}
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2 {{candidate constructor {{.*
namespace test1 {
struct A {
@@ -196,7 +196,7 @@
}
void test5() {
- Opaque0 _ = hiding;
+ Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int'
to 'Opaque0'}}
}
};
}
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2738,12 +2738,27 @@
return false;
}
+/// \brief Returns true if the expression is a MemberExpr
+/// with a dependent type.
+static bool isDependentMember(const Expr *E)
+{
+ if (const auto ME = dyn_cast_or_null(E))
+if (const auto Member = ME->getMemberDecl())
+ return Member->getType()->isDependentType();
+
+ return false;
+}
+
/// hasAnyTypeDependentArguments - Determines if any of the expressions
/// in Exprs is type-dependent.
bool Expr::hasAnyTypeDependentArguments(ArrayRef Exprs) {
- for (unsigned I = 0; I < Exprs.size(); ++I)
-if (Exprs[I]->isTypeDependent())
+ for (const auto E : Exprs) {
+if (isa(E)) {
+ if (isDependentMember(E))
+return true;
+} else if (E->isTypeDependent())
return true;
+ }
return false;
}
Index: test/SemaTemplate/template-with-invalid-decl.cpp
===
--- test/SemaTemplate/template-with-invalid-decl.cpp
+++ test/SemaTemplate/template-with-invalid-decl.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify %s
+
+class UBS;
+
+template struct FIBH
+{
+ FIBH() { GDN(BS); } // expected-error {{use of undeclared identifier 'GDN'}}
+ class UBS* BS;
+};
+
+extern void foo()
+{
+ FIBH IBH;
+}
+
+extern void GDN(UBS* BS);
Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -29,7 +29,7 @@
};
}
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2{{candidate constructor {{.*
namespace test1 {
struct A {
@@ -112,7 +112,7 @@
}
void test5() {
- Opaque0 _ = hiding;
+ Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
}
};
}
Index: test/CXX/class.access/class.access.dcl/p1.cpp
===
--- t
[PATCH] D40508: Replace long type names in IR with hashes
sepavloff added a comment. In https://reviews.llvm.org/D40508#937212, @rjmccall wrote: > In Swift's IRGen, we have an option controlling whether to emit meaningful > value names. That option can be set directly from the command line, but it > defaults to whether we're emitting IR assembly (i.e. .ll, not .bc), which > means that the clients most interested in getting meaningful value names — > humans, presumably — always get good names, but nobody else pays for them. I > have many, many times wished that we'd taken that same approach in Clang > instead of basing it on build configuration. > > If type names are a significant burden on compile times, should we just start > taking that same approach? Just don't use real type names in .bc output > unless we're asked to. Maybe we can eventually retroactively use the same > option for value names. This is indeed reasonable approach, I will implement it the subsequent patch. Actually we could vary name length to achieve better readability or same memory, as the hash is calculated for entire type name and remains the same. > I agree with Reid that it's really weird for there to be a raw_ostream that > automatically rewrites the string contents to be a hash when some limit is > reached; that does not feel like generalizable technology. It reduces type names and at the same time keeps type uniqueness across TUs. It also does not require massive changes in printing methods. Probably the intent will be more clear when I publish the next patch of this set. https://reviews.llvm.org/D40508 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r319195 - [Target] Make a copy of TargetOptions feature list before sorting during CodeGen
Author: ctopper
Date: Tue Nov 28 10:00:32 2017
New Revision: 319195
URL: http://llvm.org/viewvc/llvm-project?rev=319195&view=rev
Log:
[Target] Make a copy of TargetOptions feature list before sorting during CodeGen
Currently CodeGen is calling std::sort on the features vector in TargetOptions
for every function, but I don't think CodeGen should be modifying TargetOptions.
Differential Revision: https://reviews.llvm.org/D40228
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=319195&r1=319194&r2=319195&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Nov 28 10:00:32 2017
@@ -1877,13 +1877,13 @@ void CodeGenModule::ConstructAttributeLi
// we have a decl for the function and it has a target attribute then
// parse that and add it to the feature set.
StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
if (FD && FD->hasAttr()) {
llvm::StringMap FeatureMap;
getFunctionFeatureMap(FeatureMap, FD);
// Produce the canonical string for this set of features.
- std::vector Features;
for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@ void CodeGenModule::ConstructAttributeLi
if (ParsedAttr.Architecture != "" &&
getTarget().isValidCPUName(ParsedAttr.Architecture))
TargetCPU = ParsedAttr.Architecture;
- if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
} else {
// Otherwise just add the existing target cpu and target features to the
// function.
- std::vector &Features =
getTarget().getTargetOpts().Features;
- if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
+ Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+ FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+ std::sort(Features.begin(), Features.end());
+ FuncAttrs.addAttribute(
+ "target-features",
+ llvm::join(Features, ","));
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40228: [Target] Make a copy of TargetOptions feature list before sorting during CodeGen
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319195: [Target] Make a copy of TargetOptions feature list
before sorting during CodeGen (authored by ctopper).
Changed prior to commit:
https://reviews.llvm.org/D40228?vs=123862&id=124599#toc
Repository:
rL LLVM
https://reviews.llvm.org/D40228
Files:
cfe/trunk/lib/CodeGen/CGCall.cpp
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1877,13 +1877,13 @@
// we have a decl for the function and it has a target attribute then
// parse that and add it to the feature set.
StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
if (FD && FD->hasAttr()) {
llvm::StringMap FeatureMap;
getFunctionFeatureMap(FeatureMap, FD);
// Produce the canonical string for this set of features.
- std::vector Features;
for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@
if (ParsedAttr.Architecture != "" &&
getTarget().isValidCPUName(ParsedAttr.Architecture))
TargetCPU = ParsedAttr.Architecture;
- if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
} else {
// Otherwise just add the existing target cpu and target features to the
// function.
- std::vector &Features =
getTarget().getTargetOpts().Features;
- if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
+ Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+ FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+ std::sort(Features.begin(), Features.end());
+ FuncAttrs.addAttribute(
+ "target-features",
+ llvm::join(Features, ","));
}
}
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1877,13 +1877,13 @@
// we have a decl for the function and it has a target attribute then
// parse that and add it to the feature set.
StringRef TargetCPU = getTarget().getTargetOpts().CPU;
+std::vector Features;
const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
if (FD && FD->hasAttr()) {
llvm::StringMap FeatureMap;
getFunctionFeatureMap(FeatureMap, FD);
// Produce the canonical string for this set of features.
- std::vector Features;
for (llvm::StringMap::const_iterator it = FeatureMap.begin(),
ie = FeatureMap.end();
it != ie; ++it)
@@ -1898,26 +1898,19 @@
if (ParsedAttr.Architecture != "" &&
getTarget().isValidCPUName(ParsedAttr.Architecture))
TargetCPU = ParsedAttr.Architecture;
- if (TargetCPU != "")
- FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
} else {
// Otherwise just add the existing target cpu and target features to the
// function.
- std::vector &Features = getTarget().getTargetOpts().Features;
- if (TargetCPU != "")
-FuncAttrs.addAttribute("target-cpu", TargetCPU);
- if (!Features.empty()) {
-std::sort(Features.begin(), Features.end());
-FuncAttrs.addAttribute(
-"target-features",
-llvm::join(Features, ","));
- }
+ Features = getTarget().getTargetOpts().Features;
+}
+
+if (TargetCPU != "")
+ FuncAttrs.addAttribute("target-cpu", TargetCPU);
+if (!Features.empty()) {
+ std::sort(Features.begin(), Features.end());
+ FuncAttrs.addAttribute(
+ "target-features",
+ llvm::join(Features, ","));
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40108: [clang-tidy] Adding Fuchsia checkers to clang-tidy
aaron.ballman added a comment. Thanks for the nit fix. If you'd like me to commit this on your behalf, just let me know. https://reviews.llvm.org/D40108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40567: Always show template parameters in IR type names
sepavloff created this revision.
Herald added subscribers: JDevlieghere, eraman.
If a module contains opaque type and another one contains definition
of equivalent type in sense of C++, `llvm-link` merges these types.
In this procedure it can rely only on type name. An issue arises if
the type is a class template specialization. See the example.
File 1
template struct ABC;
ABC *var_1;
File 2
template struct ABC {
T *f;
};
extern ABC var_1;
ABC var_2;
llvm-link produces module:
%struct.ABC = type { i32** }
@var_1 = global %struct.ABC* null, align 8
@var_2 = global %struct.ABC zeroinitializer, align 8
Incomplete type `ABC` from the first module becomes `ABC`. It
happens because structure types obtained from template specialization
share the same type name and differ from each other only by version
suffixes, in the example the types are named as `ABC` and `ABC.0`.
`llvm-link` cannot correctly merge these types.
This change enables template arguments in class template specializations.
Clang already prints them in class context, now they will appear in the
class name itself.
https://reviews.llvm.org/D40567
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CodeGenAction.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenCXX/apple-kext-indirect-call.cpp
test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
test/CodeGenCXX/captured-statements.cpp
test/CodeGenCXX/const-init-cxx11.cpp
test/CodeGenCXX/constructor-init.cpp
test/CodeGenCXX/ctor-dtor-alias.cpp
test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
test/CodeGenCXX/debug-info-template.cpp
test/CodeGenCXX/dllexport-pr26549.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
test/CodeGenCXX/float128-declarations.cpp
test/CodeGenCXX/float16-declarations.cpp
test/CodeGenCXX/mangle-abi-tag.cpp
test/CodeGenCXX/mangle-ms-templates-memptrs-2.cpp
test/CodeGenCXX/microsoft-abi-extern-template.cpp
test/CodeGenCXX/microsoft-abi-vftables.cpp
test/CodeGenCXX/ms-property.cpp
test/CodeGenCXX/noinline-template.cpp
test/CodeGenCXX/pr29160.cpp
test/CodeGenCXX/static-init-3.cpp
test/CodeGenCXX/template-anonymous-types.cpp
test/CodeGenCXX/template-instantiation.cpp
test/CodeGenCXX/template-linkage.cpp
test/CodeGenCXX/value-init.cpp
test/CodeGenCXX/virtual-base-destructor-call.cpp
test/CodeGenCoroutines/coro-await.cpp
test/CodeGenObjCXX/arc-cxx11-init-list.mm
test/CodeGenObjCXX/property-lvalue-capture.mm
test/OpenMP/declare_reduction_codegen.cpp
test/OpenMP/target_codegen.cpp
test/OpenMP/target_parallel_codegen.cpp
test/OpenMP/target_simd_codegen.cpp
test/OpenMP/target_teams_codegen.cpp
Index: test/OpenMP/target_teams_codegen.cpp
===
--- test/OpenMP/target_teams_codegen.cpp
+++ test/OpenMP/target_teams_codegen.cpp
@@ -303,7 +303,7 @@
// CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
// CHECK: [[FAIL]]
- // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+ // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"{{[^,]+}})
// CHECK-NEXT: br label %[[END]]
// CHECK: [[END]]
#pragma omp target teams if(target: n>20)
Index: test/OpenMP/target_simd_codegen.cpp
===
--- test/OpenMP/target_simd_codegen.cpp
+++ test/OpenMP/target_simd_codegen.cpp
@@ -292,7 +292,7 @@
// CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
// CHECK: [[FAIL]]
- // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+ // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"*{{[^,]+}})
// CHECK-NEXT: br label %[[END]]
// CHECK: [[END]]
#pragma omp target simd if(target: n>20)
Index: test/OpenMP/target_parallel_codegen.cpp
===
--- test/OpenMP/target_parallel_codegen.cpp
+++ test/OpenMP/target_parallel_codegen.cpp
@@ -279,7 +279,7 @@
// CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
// CHECK: [[FAIL]]
- // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}})
+ // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT{{[^,]+}})
// CHECK-NEXT: br label %[[END]]
// CHECK: [[END]]
#pragma omp target parallel if(target: n>20)
In
[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer
kcc created this revision.
Herald added subscribers: cfe-commits, fedor.sergeev.
preliminary design document for a hardware-assisted memory safety (HWAMS) tool,
similar to AddressSanitizer
The name TaggedAddressSanitizer and the rest of the document, are early draft,
suggestions are welcome.
The code will follow shortly.
Repository:
rC Clang
https://reviews.llvm.org/D40568
Files:
docs/TaggedAddressSanitizerDesign.rst
docs/index.rst
Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -84,6 +84,7 @@
PTHInternals
PCHInternals
ItaniumMangleAbiTags
+ TaggedAddressSanitizerDesign
Indices and tables
Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,118 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag,
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+ (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+ (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+ with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` on mismatch.
+The function name encodes the type and the size of access, e.g. `__hwams_load4(void *ptr)`.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+ * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+ * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+ * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+ * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+ similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+ "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.c
[PATCH] D40508: Replace long type names in IR with hashes
sepavloff added a comment. https://reviews.llvm.org/D40567 presents a patch that adds template argument names to class template specializations. It also describes the use case in which type names are used to identify type across different TUs. Adding template parameters obviously increase memory consumption. This patch provides a way to reduce it. Note that this issue arises only if source is compiled to bitcode (.ll or .bc). If compilation is made to machine representation (.s or .o), IR type name are not needed. https://reviews.llvm.org/D40508 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer
kcc updated this revision to Diff 124604.
kcc added a comment.
minor edit (explained shadow)
Repository:
rC Clang
https://reviews.llvm.org/D40568
Files:
docs/TaggedAddressSanitizerDesign.rst
Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,118 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The shadow, the redzones, and the quarantine are the
+major sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+ (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+ (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+ with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function
+that loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` on mismatch.
+The function name encodes the type and the size of access, e.g. `__hwams_load4(void *ptr)`.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+ * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+ * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+ * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+ * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+ similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+ "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
+.. _AddressSanitizer paper: https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
+.. _Address Tagging: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
tra added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
rjmccall wrote:
> tra wrote:
> > tra wrote:
> > > rjmccall wrote:
> > > > Please write this check so that it trips in an "ordinary" build on a
> > > > target that just happens to not support VLAs, something like:
> > > >
> > > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > >
> > > > If you want to include the explicit OpenMP check there, it would need
> > > > to be:
> > > >
> > > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > > (!getLangOpts().OpenMP || shouldDiagnoseTargetSupportFromOpenMP()))
> > > >
> > > > but I think the first looks better.
> > > >
> > > > The CUDA and OpenMP paths here seem to be trying to achieve analogous
> > > > things; it's unfortunate that we can't find a way to unify their
> > > > approaches, even if we'd eventually want to use different diagnostic
> > > > text. I imagine that the target-environment language restrictions are
> > > > basically the same, since they arise for the same fundamental reasons,
> > > > so all the places using CUDADiagIfDeviceCode are likely to have a check
> > > > for shouldDiagnoseTargetSupportFromOpenMP() and vice-versa.
> > > The problem is that in CUDA we can't just do
> > > ```
> > > if (!Context.getTargetInfo().isVLASupported() &&
> > > shouldDiagnoseTargetSupportFromOpenMP())
> > >Diag(Loc, diag::err_vla_unsupported);
> > > ```
> > >
> > > In some situations diag messages will only be emitted if we attempt to
> > > generate unsupported code on device side.
> > > Consider
> > > ```
> > > __host__ __device__ void foo(int n) {
> > > int vla[n];
> > > }
> > > ```
> > >
> > > When Sema sees this code during compilation, it can not tell whether
> > > there is an error. Calling foo from the host code is perfectly valid.
> > > Calling it from device code is not. `CUDADiagIfDeviceCode` creates
> > > 'postponed' diagnostics which only gets emitted if we ever need to
> > > generate code for the function on device.
> > >
> > > So, while CUDA and OpenMP do similar things, they are not quite the same.
> > > If your goal to generalize CUDA and OpenMP handling, then it would have
> > > to be folded into diagnostic-emitting itself and we'll need an analog of
> > > CUDADiagIfDeviceCode which can handle both OpenMP and CUDA.
> > > E.g. something like this:
> > > ```
> > > ??? DiagIfDeviceCode(???) {
> > >if (OpenCL || (OpenMP && shouldDiagnoseTargetSupportFromOpenMP()))
> > >Diag(...);
> > >else if (CUDA)
> > >CUDADiagIfDeviceCode()
> > > }
> > >
> > > ...
> > >
> > > if (!Context.getTargetInfo().isVLASupported())
> > >DiagIfDeviceCode();
> > >
> > > ```
> > >
> > > Would that work for you?
> > There's another issue with this approach -- diagnostics itself. Each
> > dialect has its own. Specifically CUDA diags have details that are relevant
> > only to CUDA. I suspect OpenMP has something specific as well. If we insist
> > emitting only one kind of error for particular case across all dialects,
> > we'll have to stick to bare bones "feature X is not supported" which will
> > not have sufficient details to explain why the error was triggered in CUDA.
> >
> > IMO dialect-specific handling of cuda errors in this case is the lesser
> > evil.
> >
> > I'll update the patch to handle non-cuda cases the way you suggested.
> If there really is interesting language-specific information to provide in a
> diagnostic, I agree that it's hard to avoid having different code for
> different targets. On the other hand, the CUDA-specific information in this
> specific diagnostic seems unnecessary — does the user really care about
> whether the function was '__device__' vs. '__host__ __device__'? in fact,
> isn't the latter a bit misleading? — and I feel like a generic "cannot use
> variable-length arrays when compiling for device 'nvptx64'" would be
> perfectly satisfactory in both CUDA and OpenMP, and that's probably true for
> almost all of these diagnostics.
>
> On a completely different note, I do want to point out that the only thing
> you actually *need* to ban here is declaring a local variable of VLA type.
> There's no reason at all to ban VLA types in general; they just compile to
> extra arithmetic.
Agreed. While host/device attributes are part of a function signature in CUDA,
in this case only 'device' part makes the difference and therefore common-style
reporting the way you suggest would be sufficient to indicate the error in the
device-side code.
As for the VLA types, clang can currently compile code with VLA arguments:
https://godbolt.org/g/43hVu9
Clang explicitly does not support GCC's VLA-in-structure extension.
Are th
[PATCH] D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics
spatel added a comment. Ping * 2. https://reviews.llvm.org/D40044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39730: Enabling constructor code completion
arphaman added a comment. I'll take a look, but my guess is that you might have to use scoping information https://reviews.llvm.org/D39730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r319201 - [CUDA] Report "unsupported VLA" errors only on device side.
Author: tra
Date: Tue Nov 28 10:51:42 2017
New Revision: 319201
URL: http://llvm.org/viewvc/llvm-project?rev=319201&view=rev
Log:
[CUDA] Report "unsupported VLA" errors only on device side.
This fixes erroneously reported CUDA compilation errors
in host-side code during device-side compilation.
I've also restricted OpenMP-specific checks to trigger only
if we're compiling with OpenMP enabled.
Differential Revision: https://reviews.llvm.org/D40275
Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
cfe/trunk/test/SemaCUDA/vla.cu
Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=319201&r1=319200&r2=319201&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Nov 28 10:51:42 2017
@@ -2180,14 +2180,17 @@ QualType Sema::BuildArrayType(QualType T
Diag(Loc, diag::err_opencl_vla);
return QualType();
}
- // CUDA device code doesn't support VLAs.
- if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
- // Some targets don't support VLAs.
- if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported() &&
- shouldDiagnoseTargetSupportFromOpenMP()) {
-Diag(Loc, diag::err_vla_unsupported);
-return QualType();
+
+ if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
+if (getLangOpts().CUDA) {
+ // CUDA device code doesn't support VLAs.
+ CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
+} else if (!getLangOpts().OpenMP ||
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
+ Diag(Loc, diag::err_vla_unsupported);
+ return QualType();
+}
}
// If this is not C99, extwarn about VLA's and C99 array size modifiers.
Modified: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu?rev=319201&r1=319200&r2=319201&view=diff
==
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu (original)
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu Tue Nov 28 10:51:42
2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only
-verify %s
#include "Inputs/cuda.h"
Modified: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu?rev=319201&r1=319200&r2=319201&view=diff
==
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu (original)
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu Tue Nov 28
10:51:42 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only
-verify %s
#include "Inputs/cuda.h"
Modified: cfe/trunk/test/SemaCUDA/vla.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/vla.cu?rev=319201&r1=319200&r2=319201&view=diff
==
--- cfe/trunk/test/SemaCUDA/vla.cu (original)
+++ cfe/trunk/test/SemaCUDA/vla.cu Tue Nov 28 10:51:42 2017
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
#include "Inputs/cuda.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319201: [CUDA] Report "unsupported VLA" errors only on
device side. (authored by tra).
Changed prior to commit:
https://reviews.llvm.org/D40275?vs=123831&id=124607#toc
Repository:
rL LLVM
https://reviews.llvm.org/D40275
Files:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
cfe/trunk/test/SemaCUDA/vla.cu
Index: cfe/trunk/test/SemaCUDA/vla.cu
===
--- cfe/trunk/test/SemaCUDA/vla.cu
+++ cfe/trunk/test/SemaCUDA/vla.cu
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
#include "Inputs/cuda.h"
Index: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
===
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only
-verify %s
#include "Inputs/cuda.h"
Index: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
===
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only
-verify %s
#include "Inputs/cuda.h"
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -2180,14 +2180,17 @@
Diag(Loc, diag::err_opencl_vla);
return QualType();
}
- // CUDA device code doesn't support VLAs.
- if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
- // Some targets don't support VLAs.
- if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported() &&
- shouldDiagnoseTargetSupportFromOpenMP()) {
-Diag(Loc, diag::err_vla_unsupported);
-return QualType();
+
+ if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
+if (getLangOpts().CUDA) {
+ // CUDA device code doesn't support VLAs.
+ CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
+} else if (!getLangOpts().OpenMP ||
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
+ Diag(Loc, diag::err_vla_unsupported);
+ return QualType();
+}
}
// If this is not C99, extwarn about VLA's and C99 array size modifiers.
Index: cfe/trunk/test/SemaCUDA/vla.cu
===
--- cfe/trunk/test/SemaCUDA/vla.cu
+++ cfe/trunk/test/SemaCUDA/vla.cu
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -DHOST %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -verify -DHOST %s
#include "Inputs/cuda.h"
Index: cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
===
--- cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
+++ cfe/trunk/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only -verify %s
#include "Inputs/cuda.h"
Index: cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
===
--- cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
+++ cfe/trunk/test/SemaCUDA/call-stack-for-deferred-err.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -fsyntax-only -verify %s
#include "Inputs/cuda.h"
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -2180,14 +2180,17 @@
Diag(Loc, diag::err_opencl_vla);
return QualType();
}
- // CUDA device code doesn't support VLAs.
- if (getLangOpts().CUDA && T->isVariableArrayType())
-CUDADiagIfDeviceCode(Loc, diag::err_cuda_vla) << CurrentCUDATarget();
- // Some targets don't support VLAs.
[PATCH] D40310: Restructure how we break tokens.
krasimir added inline comments.
Comment at: lib/Format/BreakableToken.cpp:178
Split Split) const {
// Example: consider the content
// lala lala
Offtopic: Should add a FIXME that this doesn't really work in the presence of
tabs.
Comment at: lib/Format/BreakableToken.h:154
/// \p Split has been compressed into a single space.
unsigned getLineLengthAfterCompression(unsigned RemainingTokenColumns,
Split Split) const;
nit: the comment doesn't make sense anymore.
Comment at: lib/Format/ContinuationIndenter.cpp:1625
+ if (ExcessCharactersPenalty < NewBreakPenalty) {
+ContinueOnLine = true;
+ }
nit: no braces around single-statement if body
Comment at: lib/Format/ContinuationIndenter.cpp:1707
+ RemainingTokenColumns = Token->getRemainingLength(
+ NextLineIndex, TailOffset, ContentStartColumn);
+ Reflow = true;
When we're reflowing we replace the reflow split with a space, so IMO this
should be:
```
RemainingTokenColumns = Token->getRemainingLength(
NextLineIndex, TailOffset, ContentStartColumn + 1);
```
Comment at: lib/Format/ContinuationIndenter.cpp:1709
+ Reflow = true;
+ if (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+DEBUG(llvm::dbgs() << "Over limit after reflow, need: "
IMO this should be `ContentStartColumn + RemainingTokenColumns + 1`, accounting
for the space that replaces the reflow split.
Comment at: lib/Format/ContinuationIndenter.cpp:1721
+Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+ContentStartColumn, CommentPragmasRegex);
+if (Split.first == StringRef::npos) {
Looks like `ContentStartColumn` is consistently used as the start column of the
reflown content on the next line.
I suggest to `++ContentStartColumn` at the beginning of the body of this if
statement (and adjust below appropriately).
https://reviews.llvm.org/D40310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input
erichkeane updated this revision to Diff 124609.
erichkeane added a comment.
Add has padding to CXXABI getMemberPointerWidthAndAlign to correct padding
behavior here.
https://reviews.llvm.org/D39347
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
lib/AST/ASTContext.cpp
lib/AST/CXXABI.h
lib/AST/ItaniumCXXABI.cpp
lib/AST/MicrosoftCXXABI.cpp
lib/AST/Type.cpp
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/type-traits.cpp
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
return false;
}
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
- assert((*this)->isUnionType() && "must be union type");
- CharUnits UnionSize = Context.getTypeSizeInChars(*this);
- const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
- for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
- return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
- return false;
- }
- return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
- assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
- const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
- if (!RD->field_empty())
-return false;
-
- if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
- }
-
- return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext &Context) const {
- assert((*this)->isStructureOrClassType() && "Must be struct or class");
- const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
- if (isStructEmpty(*this))
-return false;
-
- // Check base types.
- CharUnits BaseSize{};
- if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
- if (Base.isVirtual())
-return false;
-
- // Empty bases are permitted, otherwise ensure base has unique
- // representation. Also, Empty Base Optimization means that an
- // Empty base takes up 0 size.
- if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
- return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
- }
-}
- }
-
- CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
- // This struct obviously has bases that keep it from being 'empty', so
- // checking fields is no longer required. Ensure that the struct size
- // is the sum of the bases.
- if (RD->field_empty())
-return StructSize == BaseSize;
-
- CharUnits CurOffset =
- Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
- // If the first field isn't at the sum of the size of the bases, there
- // is padding somewhere.
- if (BaseSize != CurOffset)
-return false;
-
- for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
- return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
- return false;
-CurOffset += FieldSize;
- }
- // Check for tail padding.
- return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext &Context) const {
- // C++17 [meta.unary.prop]:
- // The predicate condition for a template specialization
- // has_unique_object_representations shall be
- // satisfied if and only if:
- // (9.1) - T is trivially copyable, and
- // (9.2) - any two objects of type T with the same value have the same
- // object representation, where two objects
- // of array or non-union class type are considered to have the same value
- // if their respective sequences of
- // direct subobjects have the same values, and two objects of union type
- // are considered to have the same
- // value if they have the same active member and the corresponding members
- // have the same value.
- // The set of scalar types for which this condition holds is
- // implementation-defined. [ Note: If a type has padding
- // bits, the condition does not hold; otherwise, the condition holds true
- // for unsigned integral types. -- end note ]
- if (isNull())
-return false;
-
- // Arrays are unique only if their element type is unique.
- if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
- // (9.1) - T is trivially copyable, and
- if (!isTriviallyCopyableType(Context))
-return false;
-
- // Functions are not unique.
- if ((*this)->isFunctionType())
-return false;
-
- /
[PATCH] D40310: Restructure how we break tokens.
krasimir added a comment. Re: "I tried to write a test for this, and convinced myself that while +1 is correct, it is currently impossible to change behavior based on the missing +1.": In order to have different outcome based on the start column, you could use tabs. Consider the content `"aaa\tb"` with 4 spaces of tabstop. This takes up 5 columns (say, under the column limit) if it starts at column 0, and 8 columns (say, over the column limit) if it starts at column 1. https://reviews.llvm.org/D40310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.
bsdjhb added a comment. Ping @compnerd, @sdardis https://reviews.llvm.org/D38110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
LLVM buildmaster will be restarted in few minutes
Hello everyone, LLVM buildmaster will be restarted in few minutes. Thank you for understanding. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.
jacobly created this revision.
Forcing the 4 byte alignment caused an issue in my out of tree backend that
doesn't even have a 4 byte aligned stack. Using the default target-specific
alignment for i32 seems more reasonable.
Repository:
rC Clang
https://reviews.llvm.org/D40569
Files:
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -433,7 +433,7 @@
};
/// i32s containing the indexes of the cleanup destinations.
- llvm::AllocaInst *NormalCleanupDest;
+ Address NormalCleanupDest;
unsigned NextCleanupDestIndex;
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -70,7 +70,7 @@
IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
- NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+ NormalCleanupDest(Address::invalid()), NextCleanupDestIndex(1),
FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
DisableDebugInfo(false), DidCallStackSave(false),
IndirectBranch(nullptr),
@@ -433,10 +433,11 @@
// if compiled with no optimizations. We do it for coroutine as the lifetime
// of CleanupDestSlot alloca make correct coroutine frame building very
// difficult.
- if (NormalCleanupDest && isCoroutine()) {
+ if (NormalCleanupDest.isValid() && isCoroutine()) {
llvm::DominatorTree DT(*CurFn);
-llvm::PromoteMemToReg(NormalCleanupDest, DT);
-NormalCleanupDest = nullptr;
+llvm::PromoteMemToReg(
+cast(NormalCleanupDest.getPointer()), DT);
+NormalCleanupDest = Address::invalid();
}
}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -833,7 +833,7 @@
if (NormalCleanupDestSlot->hasOneUse()) {
NormalCleanupDestSlot->user_back()->eraseFromParent();
NormalCleanupDestSlot->eraseFromParent();
- NormalCleanupDest = nullptr;
+ NormalCleanupDest = Address::invalid();
}
llvm::BasicBlock *BranchAfter = Scope.getBranchAfterBlock(0);
@@ -1250,10 +1250,10 @@
}
Address CodeGenFunction::getNormalCleanupDestSlot() {
- if (!NormalCleanupDest)
+ if (!NormalCleanupDest.isValid())
NormalCleanupDest =
- CreateTempAlloca(Builder.getInt32Ty(), "cleanup.dest.slot");
- return Address(NormalCleanupDest, CharUnits::fromQuantity(4));
+ CreateDefaultAlignTempAlloca(Builder.getInt32Ty(), "cleanup.dest.slot");
+ return NormalCleanupDest;
}
/// Emits all the code to cause the given temporary to be cleaned up.
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -433,7 +433,7 @@
};
/// i32s containing the indexes of the cleanup destinations.
- llvm::AllocaInst *NormalCleanupDest;
+ Address NormalCleanupDest;
unsigned NextCleanupDestIndex;
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -70,7 +70,7 @@
IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
- NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+ NormalCleanupDest(Address::invalid()), NextCleanupDestIndex(1),
FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
@@ -433,10 +433,11 @@
// if compiled with no optimizations. We do it for coroutine as the lifetime
// of CleanupDestSlot alloca make correct coroutine frame building very
// difficult.
- if (NormalCleanupDest && isCoroutine()) {
+ if (NormalCleanupDest.isValid() && isCoroutine()) {
llvm::DominatorTree DT(*CurFn);
-llvm::PromoteMemToReg(NormalCleanupDest, DT);
-NormalCleanupDest = nullptr;
+llvm::PromoteMemToReg(
+cast(NormalCleanupDest.getPointer()), DT);
+NormalCleanupDest = Address::invalid();
}
}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -833,7 +833,7 @@
[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion
jtony added inline comments. Comment at: test/Driver/ppc-features.cpp:140 + // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-htm -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOHTM %s // CHECK-NOHTM: "-target-feature" "-htm" We probably need to add test for mno_save_toc_indirect also once you add it according to Hal's suggestion. https://reviews.llvm.org/D39376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.
dcoughlin added a comment.
Thanks for tackling this! There is one major comment inline (you are generating
an extra node that you shouldn't, which is causing the analyzer to explore code
it shouldn't) and a suggestion for simpler diagnostic text.
Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+ str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";
"Unary operator" is probably not something we should expect users to know
about. How about just "The expression is an uninitialized value. The computed
value will also be garbage."
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1046
if (V2_untested.isUnknownOrUndef()) {
Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+
Instead of generating a node node here, you'll want to generate a new state:
```
state = state->BindExpr(U, LCtx, V2_untested);
```
and use that state in the call to evalStore().
Otherwise, you'll end up exploring from both the new generated node and from *I.
Here is a test that demonstrates why this is a problem. You should add it to
your tests.
```
void foo() {
int x;
x++; // expected-warning {{The expression of the unary operator is an
uninitialized value. The computed value will also be garbage}}
clang_analyzer_warnIfReached(); // no-warning
```
The undefined assignment checker generates what we call "fatal" errors. These
errors are so bad that it decides not to explore further on that path to report
additional errors. This means that the analyzer should treat any code that is
dominated by such an error as not reachable. You'll need to add the
`debug.ExprInspection` checker to your test RUN line and a prototype for
clang_analyzer_warnIfReached() for this test to to work.
Comment at: test/Analysis/malloc-plist.c:2
// RUN: rm -f %t
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc
-analyzer-output=plist -analyzer-config path-diagnostics-alternate=false -o %t
%s
// RUN: FileCheck -input-file %t %s
xazax.hun wrote:
> In case you willing to replace the plist based test with
> `-analyzer-output=text` based, it would be awesome, though not required for
> this patch to be accepted.
Let's hold off on replacing the plist test with text. We'll need to make sure
we're still testing plist emission separately.
Comment at: test/Analysis/malloc-plist.c:13
int *p = malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is
an uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning
Once you change the core modeling to not generate an extra node, you'll want to
initialize `*p` to `0` or something to preserve the intent of this test. For
this test it is important that the increment not stop further exploration of
the path.
Comment at: test/Analysis/malloc-plist.c:111
p = (int*)malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is an
uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning
Same point applies here.
Comment at: test/Analysis/objc-for.m:131
int i;
int j;
for (NSString *a in A) {
You should just initialize `j` in order to preserve the test's intent. (And
similarly for the other new warnings in this file.
Comment at: test/Analysis/uninit-const.c:127
+ int x; // expected-note 5 {{'x' declared without an initial value}}
+ x++; // expected-warning {{The expression of the unary operator is an
uninitialized value. The computed value will also be garbage}}
+ // expected-note@-1 {{The expression of the unary operator is an
uninitialized value. The computed value will also be garbage}}
You'll need to split this test into multiple functions once you stop generating
the extra node.
Comment at: test/Analysis/uninit-const.cpp:11
+// https://bugs.llvm.org/show_bug.cgi?id=35419
+void f11(void) {
I don't think this test is needed. The logic for C vs. C++ is shared for your
new functionality, so I think it is sufficient to just test the C case.
Repository:
rL LLVM
https://reviews.llvm.org/D40463
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, seems reasonable enough. Repository: rC Clang https://reviews.llvm.org/D40569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40275: [CUDA] Report "unsupported VLA" errors only on device side.
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
tra wrote:
> rjmccall wrote:
> > tra wrote:
> > > tra wrote:
> > > > rjmccall wrote:
> > > > > Please write this check so that it trips in an "ordinary" build on a
> > > > > target that just happens to not support VLAs, something like:
> > > > >
> > > > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > > >
> > > > > If you want to include the explicit OpenMP check there, it would need
> > > > > to be:
> > > > >
> > > > > else if (!Context.getTargetInfo().isVLASupported() &&
> > > > > (!getLangOpts().OpenMP || shouldDiagnoseTargetSupportFromOpenMP()))
> > > > >
> > > > > but I think the first looks better.
> > > > >
> > > > > The CUDA and OpenMP paths here seem to be trying to achieve analogous
> > > > > things; it's unfortunate that we can't find a way to unify their
> > > > > approaches, even if we'd eventually want to use different diagnostic
> > > > > text. I imagine that the target-environment language restrictions
> > > > > are basically the same, since they arise for the same fundamental
> > > > > reasons, so all the places using CUDADiagIfDeviceCode are likely to
> > > > > have a check for shouldDiagnoseTargetSupportFromOpenMP() and
> > > > > vice-versa.
> > > > The problem is that in CUDA we can't just do
> > > > ```
> > > > if (!Context.getTargetInfo().isVLASupported() &&
> > > > shouldDiagnoseTargetSupportFromOpenMP())
> > > >Diag(Loc, diag::err_vla_unsupported);
> > > > ```
> > > >
> > > > In some situations diag messages will only be emitted if we attempt to
> > > > generate unsupported code on device side.
> > > > Consider
> > > > ```
> > > > __host__ __device__ void foo(int n) {
> > > > int vla[n];
> > > > }
> > > > ```
> > > >
> > > > When Sema sees this code during compilation, it can not tell whether
> > > > there is an error. Calling foo from the host code is perfectly valid.
> > > > Calling it from device code is not. `CUDADiagIfDeviceCode` creates
> > > > 'postponed' diagnostics which only gets emitted if we ever need to
> > > > generate code for the function on device.
> > > >
> > > > So, while CUDA and OpenMP do similar things, they are not quite the
> > > > same. If your goal to generalize CUDA and OpenMP handling, then it
> > > > would have to be folded into diagnostic-emitting itself and we'll need
> > > > an analog of CUDADiagIfDeviceCode which can handle both OpenMP and
> > > > CUDA.
> > > > E.g. something like this:
> > > > ```
> > > > ??? DiagIfDeviceCode(???) {
> > > >if (OpenCL || (OpenMP && shouldDiagnoseTargetSupportFromOpenMP()))
> > > >Diag(...);
> > > >else if (CUDA)
> > > >CUDADiagIfDeviceCode()
> > > > }
> > > >
> > > > ...
> > > >
> > > > if (!Context.getTargetInfo().isVLASupported())
> > > >DiagIfDeviceCode();
> > > >
> > > > ```
> > > >
> > > > Would that work for you?
> > > There's another issue with this approach -- diagnostics itself. Each
> > > dialect has its own. Specifically CUDA diags have details that are
> > > relevant only to CUDA. I suspect OpenMP has something specific as well.
> > > If we insist emitting only one kind of error for particular case across
> > > all dialects, we'll have to stick to bare bones "feature X is not
> > > supported" which will not have sufficient details to explain why the
> > > error was triggered in CUDA.
> > >
> > > IMO dialect-specific handling of cuda errors in this case is the lesser
> > > evil.
> > >
> > > I'll update the patch to handle non-cuda cases the way you suggested.
> > If there really is interesting language-specific information to provide in
> > a diagnostic, I agree that it's hard to avoid having different code for
> > different targets. On the other hand, the CUDA-specific information in
> > this specific diagnostic seems unnecessary — does the user really care
> > about whether the function was '__device__' vs. '__host__ __device__'? in
> > fact, isn't the latter a bit misleading? — and I feel like a generic
> > "cannot use variable-length arrays when compiling for device 'nvptx64'"
> > would be perfectly satisfactory in both CUDA and OpenMP, and that's
> > probably true for almost all of these diagnostics.
> >
> > On a completely different note, I do want to point out that the only thing
> > you actually *need* to ban here is declaring a local variable of VLA type.
> > There's no reason at all to ban VLA types in general; they just compile to
> > extra arithmetic.
> Agreed. While host/device attributes are part of a function signature in
> CUDA, in this case only 'device' part makes the difference and therefore
> common-style reporting the way you suggest would be suffi
[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh
rnk requested changes to this revision. rnk added inline comments. This revision now requires changes to proceed. Comment at: lib/Driver/ToolChain.cpp:450-458 + const llvm::Target *TT = llvm::TargetRegistry::lookupTarget(TS, Error); + if (!TT) +return llvm::ExceptionHandling::None; + std::unique_ptr MRI(TT->createMCRegInfo(TS)); + if (!MRI) +return llvm::ExceptionHandling::None; + std::unique_ptr MAI(TT->createMCAsmInfo(*MRI, TS)); I think this is problematic because Clang is supposed to be able to generate LLVM IR for targets that are not registered. With this change, we have silent behavior difference between clang with LLVM backend and clang without LLVM backends. Building clang without a single backend is pretty silly, but our test suite frequently generates IR for targets that are not compiled in. Some of the tests you've written will probably fail on buildbots configured this way. Repository: rL LLVM https://reviews.llvm.org/D39673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40508: Replace long type names in IR with hashes
rjmccall added a comment.
My skepticism about the raw_ostream is not about the design of having a custom
raw_ostream subclass, it's that that subclass could conceivably be re-used by
some other client. It feels like it belongs as an internal hack in Clang
absent some real evidence that someone else would use it.
Comment at: lib/AST/TypePrinter.cpp:1532-1534
+namespace {
+template
+void printTo(raw_ostream &OS, ArrayRef Args, const PrintingPolicy &Policy,
sepavloff wrote:
> rnk wrote:
> > `static` is nicer than a short anonymous namespace.
> Yes, but this is function template. It won't create symbol in object file.
> Actually anonymous namespace has no effect here, it is only a documentation
> hint.
Nonetheless, we generally prefer to use 'static' on internal functions, even
function templates, instead of putting them in anonymous namespaces.
https://reviews.llvm.org/D40508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40572: [OpenMP] Make test robust against quotation, NFC.
Hahnfeld created this revision.
This is needed to not break with an upcoming change in LLVM to use
dollar signs (which need to be quoted) instead of dots for
generating unique names.
https://reviews.llvm.org/D40572
Files:
test/OpenMP/nvptx_parallel_codegen.cpp
Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -92,20 +92,20 @@
//
// CHECK: [[EXEC_PARALLEL]]
// CHECK: [[WF1:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
- // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1:@.+]]_wrapper to i8*)
+ // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1:@.+]]_wrapper{{"?}} to i8*)
// CHECK: br i1 [[WM1]], label {{%?}}[[EXEC_PFN1:.+]], label {{%?}}[[CHECK_NEXT1:.+]]
//
// CHECK: [[EXEC_PFN1]]
- // CHECK: call void [[PARALLEL_FN1]]_wrapper(
+ // CHECK: call void [[PARALLEL_FN1]]_wrapper{{"?}}(
// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
//
// CHECK: [[CHECK_NEXT1]]
// CHECK: [[WF2:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
- // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2:@.+]]_wrapper to i8*)
+ // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2:@.+]]_wrapper{{"?}} to i8*)
// CHECK: br i1 [[WM2]], label {{%?}}[[EXEC_PFN2:.+]], label {{%?}}[[CHECK_NEXT2:.+]]
//
// CHECK: [[EXEC_PFN2]]
- // CHECK: call void [[PARALLEL_FN2]]_wrapper(
+ // CHECK: call void [[PARALLEL_FN2]]_wrapper{{"?}}(
// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
//
// CHECK: [[CHECK_NEXT2]]
@@ -152,13 +152,13 @@
// CHECK-DAG: [[MWS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
// CHECK: [[MTMP1:%.+]] = sub i32 [[MNTH]], [[MWS]]
// CHECK: call void @__kmpc_kernel_init(i32 [[MTMP1]]
- // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1]]_wrapper to i8*),
+ // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1]]_wrapper{{"?}} to i8*),
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK: call void @__kmpc_serialized_parallel(
- // CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]](
+ // CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]]{{"?}}(
// CHECK: call void @__kmpc_end_serialized_parallel(
- // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2]]_wrapper to i8*),
+ // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2]]_wrapper{{"?}} to i8*),
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK-64-DAG: load i32, i32* [[REF_A]]
@@ -173,17 +173,17 @@
// CHECK: [[EXIT]]
// CHECK: ret void
- // CHECK-DAG: define internal void [[PARALLEL_FN1]](
+ // CHECK-DAG: define internal void [[PARALLEL_FN1]]{{"?}}(
// CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
// CHECK: store i[[SZ]] 42, i[[SZ]]* %a,
// CHECK: ret void
- // CHECK-DAG: define internal void [[PARALLEL_FN3]](
+ // CHECK-DAG: define internal void [[PARALLEL_FN3]]{{"?}}(
// CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
// CHECK: store i[[SZ]] 43, i[[SZ]]* %a,
// CHECK: ret void
- // CHECK-DAG: define internal void [[PARALLEL_FN2]](
+ // CHECK-DAG: define internal void [[PARALLEL_FN2]]{{"?}}(
// CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
// CHECK: store i[[SZ]] 44, i[[SZ]]* %a,
// CHECK: ret void
@@ -217,11 +217,11 @@
//
// CHECK: [[EXEC_PARALLEL]]
// CHECK: [[WF:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
- // CHECK: [[WM:%.+]] = icmp eq i8* [[WF]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4:@.+]]_wrapper to i8*)
+ // CHECK: [[WM:%.+]] = icmp eq i8* [[WF]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4:@.+]]_wrapper{{"?}} to i8*)
// CHECK: br i1 [[WM]], label {{%?}}[[EXEC_PFN:.+]], label {{%?}}[[CHECK_NEXT:.+]]
//
// CHECK: [[EXEC_PFN]]
- // CHECK: call void [[PARALLEL_FN4]]_wrapper(
+ // CHECK: call void [[PARALLEL_FN4]]_wrapper{{"?}}(
// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
//
// CHECK: [[CHECK_NEXT]]
@@ -283,14 +283,14 @@
// CHECK: br i1 [[CMP]], label {{%?}}[[IF_THEN:.+]], label {{%?}}[[IF_ELSE:.+]]
//
// CHECK: [[IF_THEN]]
- // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4]]_wrapper to i8*),
+ // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN4]]_wrapper{{"?}} to i8*),
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK: call void @llvm.nvvm.barrier0()
// CHECK: br label {{%?}}[[IF_END:.+]]
//
// CHECK: [[IF_ELSE]]
// CHECK: call void @__kmpc_serialized_parallel(
- // CHECK: {{call|invoke}} void [[PARALLEL_FN4]](
+
[PATCH] D40299: [Complex] Don't use __div?c3 when building with fast-math.
fhahn added reviewers: arphaman, GorNishanov, hfinkel, fhahn.
fhahn added a comment.
Looks good to me, with some nits. However it seems that we do not use the fast
math flags anywhere else in Clang codegen, so it would be good to clarify if it
is OK to use it here.
Comment at: lib/CodeGen/CGExprComplex.cpp:768
+
// If we have a complex operand on the RHS, we delegate to a libcall to
// handle all of the complexities and minimize underflow/overflow cases.
Maybe we should update the comment that we do a similar simplification for
floats with fast math enabled as for integers?
Comment at: lib/CodeGen/CGExprComplex.cpp:773
// supported imaginary types in addition to complex types.
-if (RHSi) {
+if (RHSi && !FMF.isFast()) {
BinOpInfo LibCallOp = Op;
Would the following structure be slightly easier to read?
if (RHSi) {
if (FMF.isFast()) { simplify } else {libcall}
}
Comment at: lib/CodeGen/CGExprComplex.cpp:800
+ // (a+ib) / (c+id) = ((ac+bd)/(cc+dd)) + i((bc-ad)/(cc+dd))
+ llvm::Value *AC = Builder.CreateFMul(LHSr, RHSr); // a*c
+ llvm::Value *BD = Builder.CreateFMul(LHSi, RHSi); // b*d
This is basically the same as the simplification for integers, unfortunate that
we have to duplicate it (because of different instructions).
https://reviews.llvm.org/D40299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r319222 - [OpenMP] Stable sort Privates to remove non-deterministic ordering
Author: mgrang
Date: Tue Nov 28 12:41:13 2017
New Revision: 319222
URL: http://llvm.org/viewvc/llvm-project?rev=319222&view=rev
Log:
[OpenMP] Stable sort Privates to remove non-deterministic ordering
Summary:
This fixes the following failures uncovered by D39245:
Clang :: OpenMP/task_firstprivate_codegen.cpp
Clang :: OpenMP/task_private_codegen.cpp
Clang :: OpenMP/taskloop_firstprivate_codegen.cpp
Clang :: OpenMP/taskloop_lastprivate_codegen.cpp
Clang :: OpenMP/taskloop_private_codegen.cpp
Clang :: OpenMP/taskloop_simd_firstprivate_codegen.cpp
Clang :: OpenMP/taskloop_simd_lastprivate_codegen.cpp
Clang :: OpenMP/taskloop_simd_private_codegen.cpp
Reviewers: rjmccall, ABataev, AndreyChurbanov
Reviewed By: rjmccall, ABataev
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D39947
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=319222&r1=319221&r2=319222&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Nov 28 12:41:13 2017
@@ -4056,9 +4056,9 @@ emitTaskPrivateMappingFunction(CodeGenMo
return TaskPrivatesMap;
}
-static int array_pod_sort_comparator(const PrivateDataTy *P1,
- const PrivateDataTy *P2) {
- return P1->first < P2->first ? 1 : (P2->first < P1->first ? -1 : 0);
+static bool stable_sort_comparator(const PrivateDataTy P1,
+ const PrivateDataTy P2) {
+ return P1.first > P2.first;
}
/// Emit initialization for private variables in task-based directives.
@@ -4286,8 +4286,7 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFun
/*PrivateElemInit=*/nullptr)));
++I;
}
- llvm::array_pod_sort(Privates.begin(), Privates.end(),
- array_pod_sort_comparator);
+ std::stable_sort(Privates.begin(), Privates.end(), stable_sort_comparator);
auto KmpInt32Ty = C.getIntTypeForBitwidth(/*DestWidth=*/32, /*Signed=*/1);
// Build type kmp_routine_entry_t (if not built yet).
emitKmpRoutineEntryT(KmpInt32Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
