[PATCH] D11980: Add test for PR24379
john.brawn created this revision.
john.brawn added reviewers: rafael, ab.
john.brawn added a subscriber: cfe-commits.
john.brawn set the repository for this revision to rL LLVM.
The fix for this is in LLVM but it depends on how clang handles the alias
attribute, so add a test to the clang tests to make sure everything works
together as expected.
Repository:
rL LLVM
http://reviews.llvm.org/D11980
Files:
test/CodeGen/alias.c
Index: test/CodeGen/alias.c
===
--- test/CodeGen/alias.c
+++ test/CodeGen/alias.c
@@ -1,19 +1,42 @@
// RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck
-check-prefix=CHECKBASIC %s
// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s |
FileCheck -check-prefix=CHECKCC %s
+// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | FileCheck
-check-prefix=CHECKASM %s
int g0;
-// CHECKBASIC: @g0 = common global i32 0
+// CHECKBASIC-DAG: @g0 = common global i32 0
+// CHECKASM-DAG: .comm g0,4,4
__thread int TL_WITH_ALIAS;
// CHECKBASIC-DAG: @TL_WITH_ALIAS = thread_local global i32 0, align 4
+// CHECKASM-DAG: .globl TL_WITH_ALIAS
+// CHECKASM-DAG: .size TL_WITH_ALIAS, 4
static int bar1 = 42;
-// CHECKBASIC: @bar1 = internal global i32 42
+// CHECKBASIC-DAG: @bar1 = internal global i32 42
+// CHECKASM-DAG: bar1:
+// CHECKASM-DAG: .size bar1, 4
+
+// PR24379: alias variable expected to have same size as aliasee even when
types differ
+const int wacom_usb_ids[] = {1, 1, 2, 3, 5, 8, 13, 0};
+// CHECKBASIC-DAG: @wacom_usb_ids = constant [8 x i32] [i32 1, i32 1, i32 2,
i32 3, i32 5, i32 8, i32 13, i32 0], align 4
+// CHECKASM-DAG: .globl wacom_usb_ids
+// CHECKASM-DAG: .size wacom_usb_ids, 32
+extern const int __mod_usb_device_table __attribute__
((alias(wacom_usb_ids)));
+// CHECKBASIC-DAG: @__mod_usb_device_table = alias getelementptr inbounds ([8
x i32], [8 x i32]* @wacom_usb_ids, i32 0, i32 0)
+// CHECKASM-DAG: .globl __mod_usb_device_table
+// CHECKASM-DAG: __mod_usb_device_table = wacom_usb_ids
+// CHECKASM-DAG-NOT: .size __mod_usb_device_table
extern int g1;
extern int g1 __attribute((alias(g0)));
// CHECKBASIC-DAG: @g1 = alias i32* @g0
+// CHECKASM-DAG: .globl g1
+// CHECKASM-DAG: g1 = g0
+// CHECKASM-DAG-NOT: .size g1
extern __thread int __libc_errno __attribute__ ((alias (TL_WITH_ALIAS)));
// CHECKBASIC-DAG: @__libc_errno = thread_local alias i32* @TL_WITH_ALIAS
+// CHECKASM-DAG: .globl __libc_errno
+// CHECKASM-DAG: __libc_errno = TL_WITH_ALIAS
+// CHECKASM-DAG-NOT: .size __libc_errno
void f0(void) { }
extern void f1(void);
Index: test/CodeGen/alias.c
===
--- test/CodeGen/alias.c
+++ test/CodeGen/alias.c
@@ -1,19 +1,42 @@
// RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=CHECKBASIC %s
// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s | FileCheck -check-prefix=CHECKCC %s
+// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | FileCheck -check-prefix=CHECKASM %s
int g0;
-// CHECKBASIC: @g0 = common global i32 0
+// CHECKBASIC-DAG: @g0 = common global i32 0
+// CHECKASM-DAG: .comm g0,4,4
__thread int TL_WITH_ALIAS;
// CHECKBASIC-DAG: @TL_WITH_ALIAS = thread_local global i32 0, align 4
+// CHECKASM-DAG: .globl TL_WITH_ALIAS
+// CHECKASM-DAG: .size TL_WITH_ALIAS, 4
static int bar1 = 42;
-// CHECKBASIC: @bar1 = internal global i32 42
+// CHECKBASIC-DAG: @bar1 = internal global i32 42
+// CHECKASM-DAG: bar1:
+// CHECKASM-DAG: .size bar1, 4
+
+// PR24379: alias variable expected to have same size as aliasee even when types differ
+const int wacom_usb_ids[] = {1, 1, 2, 3, 5, 8, 13, 0};
+// CHECKBASIC-DAG: @wacom_usb_ids = constant [8 x i32] [i32 1, i32 1, i32 2, i32 3, i32 5, i32 8, i32 13, i32 0], align 4
+// CHECKASM-DAG: .globl wacom_usb_ids
+// CHECKASM-DAG: .size wacom_usb_ids, 32
+extern const int __mod_usb_device_table __attribute__ ((alias(wacom_usb_ids)));
+// CHECKBASIC-DAG: @__mod_usb_device_table = alias getelementptr inbounds ([8 x i32], [8 x i32]* @wacom_usb_ids, i32 0, i32 0)
+// CHECKASM-DAG: .globl __mod_usb_device_table
+// CHECKASM-DAG: __mod_usb_device_table = wacom_usb_ids
+// CHECKASM-DAG-NOT: .size __mod_usb_device_table
extern int g1;
extern int g1 __attribute((alias(g0)));
// CHECKBASIC-DAG: @g1 = alias i32* @g0
+// CHECKASM-DAG: .globl g1
+// CHECKASM-DAG: g1 = g0
+// CHECKASM-DAG-NOT: .size g1
extern __thread int __libc_errno __attribute__ ((alias (TL_WITH_ALIAS)));
// CHECKBASIC-DAG: @__libc_errno = thread_local alias i32* @TL_WITH_ALIAS
+// CHECKASM-DAG: .globl __libc_errno
+// CHECKASM-DAG: __libc_errno = TL_WITH_ALIAS
+// CHECKASM-DAG-NOT: .size __libc_errno
void f0(void) { }
extern void f1(void);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Re: [PATCH] D11963: Create a __config_site file to capture configuration decisions.
jroelofs added inline comments. Comment at: include/__config:19 @@ -18,1 +18,3 @@ +#include __config_site + #ifdef __GNUC__ jroelofs wrote: mclow.lists wrote: espositofulvio wrote: mclow.lists wrote: I'm reluctant to do this; because every include file slows down compilation - for every program that we compile. However, this may be the right thing to do. I'm with Jonathan here, having config params dealt with this way it's easier and make things more manageable while the price of a slowdown, I think, shouldn't be substantial. I just realized that this will complicate life for libc++ developers. Today, I can make a change in the checked-out directory, and test it by using `clang -I path to libcxx`. Now, I'll have to actually build and install the headers some where to test. [ Especially when testing against an installed compiler whose libc++ does not have a __config_site file ] Another option would be to rename `__config` to `__config.in`, and put the `#cmakedefine` lines in here. Then the include tree isn't changed. Now, I'll have to actually build and install the headers some where to test. Just building is sufficient. The headers get copied to the build directory, along with this new file. [ Especially when testing against an installed compiler whose libc++ does not have a __config_site file ] Why does your workflow rely on testing libcxx binaries against installed headers from a _different_ build of the library? That seems very dicey. http://reviews.llvm.org/D11963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244756 - Add test for PR24379
Author: john.brawn
Date: Wed Aug 12 10:15:27 2015
New Revision: 244756
URL: http://llvm.org/viewvc/llvm-project?rev=244756view=rev
Log:
Add test for PR24379
The fix for this is in LLVM but it depends on how clang handles the alias
attribute, so add a test to the clang tests to make sure everything works
together as expected.
Differential Revision: http://reviews.llvm.org/D11980
Modified:
cfe/trunk/test/CodeGen/alias.c
Modified: cfe/trunk/test/CodeGen/alias.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/alias.c?rev=244756r1=244755r2=244756view=diff
==
--- cfe/trunk/test/CodeGen/alias.c (original)
+++ cfe/trunk/test/CodeGen/alias.c Wed Aug 12 10:15:27 2015
@@ -1,19 +1,42 @@
// RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck
-check-prefix=CHECKBASIC %s
// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s |
FileCheck -check-prefix=CHECKCC %s
+// RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | FileCheck
-check-prefix=CHECKASM %s
int g0;
-// CHECKBASIC: @g0 = common global i32 0
+// CHECKBASIC-DAG: @g0 = common global i32 0
+// CHECKASM-DAG: .comm g0,4,4
__thread int TL_WITH_ALIAS;
// CHECKBASIC-DAG: @TL_WITH_ALIAS = thread_local global i32 0, align 4
+// CHECKASM-DAG: .globl TL_WITH_ALIAS
+// CHECKASM-DAG: .size TL_WITH_ALIAS, 4
static int bar1 = 42;
-// CHECKBASIC: @bar1 = internal global i32 42
+// CHECKBASIC-DAG: @bar1 = internal global i32 42
+// CHECKASM-DAG: bar1:
+// CHECKASM-DAG: .size bar1, 4
+
+// PR24379: alias variable expected to have same size as aliasee even when
types differ
+const int wacom_usb_ids[] = {1, 1, 2, 3, 5, 8, 13, 0};
+// CHECKBASIC-DAG: @wacom_usb_ids = constant [8 x i32] [i32 1, i32 1, i32 2,
i32 3, i32 5, i32 8, i32 13, i32 0], align 4
+// CHECKASM-DAG: .globl wacom_usb_ids
+// CHECKASM-DAG: .size wacom_usb_ids, 32
+extern const int __mod_usb_device_table __attribute__
((alias(wacom_usb_ids)));
+// CHECKBASIC-DAG: @__mod_usb_device_table = alias getelementptr inbounds ([8
x i32], [8 x i32]* @wacom_usb_ids, i32 0, i32 0)
+// CHECKASM-DAG: .globl __mod_usb_device_table
+// CHECKASM-DAG: __mod_usb_device_table = wacom_usb_ids
+// CHECKASM-DAG-NOT: .size __mod_usb_device_table
extern int g1;
extern int g1 __attribute((alias(g0)));
// CHECKBASIC-DAG: @g1 = alias i32* @g0
+// CHECKASM-DAG: .globl g1
+// CHECKASM-DAG: g1 = g0
+// CHECKASM-DAG-NOT: .size g1
extern __thread int __libc_errno __attribute__ ((alias (TL_WITH_ALIAS)));
// CHECKBASIC-DAG: @__libc_errno = thread_local alias i32* @TL_WITH_ALIAS
+// CHECKASM-DAG: .globl __libc_errno
+// CHECKASM-DAG: __libc_errno = TL_WITH_ALIAS
+// CHECKASM-DAG-NOT: .size __libc_errno
void f0(void) { }
extern void f1(void);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11963: Create a __config_site file to capture configuration decisions.
mclow.lists added inline comments. Comment at: include/__config:19 @@ -18,1 +18,3 @@ +#include __config_site + #ifdef __GNUC__ espositofulvio wrote: mclow.lists wrote: I'm reluctant to do this; because every include file slows down compilation - for every program that we compile. However, this may be the right thing to do. I'm with Jonathan here, having config params dealt with this way it's easier and make things more manageable while the price of a slowdown, I think, shouldn't be substantial. I just realized that this will complicate life for libc++ developers. Today, I can make a change in the checked-out directory, and test it by using `clang -I path to libcxx`. Now, I'll have to actually build and install the headers some where to test. [ Especially when testing against an installed compiler whose libc++ does not have a __config_site file ] Comment at: test/std/atomics/libcpp-has-no-threads.pass.cpp:11 @@ -10,1 +10,3 @@ +#include __config_site + jroelofs wrote: mclow.lists wrote: This is not the right include file. How about #include ciso646, which defined in the standard (to do nothing), but includes __config Really, this test doesn't belong in test/std/, but in test/libcxx, but this change didn't put it there. I think it'd be better for me to just move the test. What is ciso646 supposed to be for? The standard says that including it does nothing. :-) It's a great file to include to get all the config information. http://reviews.llvm.org/D11963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11582: Fix assertion failure in TransformOpaqueValueExpr
rcraik added a comment. ping 2! http://reviews.llvm.org/D11582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11980: Add test for PR24379
rafael accepted this revision. rafael added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM http://reviews.llvm.org/D11980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin updated this revision to Diff 31979.
krasin added a comment.
Windows compat
http://reviews.llvm.org/D11968
Files:
include/clang/Frontend/DependencyOutputOptions.h
include/clang/Frontend/Utils.h
lib/Frontend/CompilerInstance.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/DependencyFile.cpp
lib/Frontend/HeaderIncludeGen.cpp
test/Frontend/dependency-gen.c
Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -20,7 +20,16 @@
// RUN: cd a/b
// RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
// CHECK-SIX: {{ }}x.h
-
+// RUN: echo fun:foo %t.blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
+// CHECK-SEVEN: {{ }}x.h
+// CHECK-SEVEN: .blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
+// CHECK-EIGHT: {{ }}x.h
+// CHECK-EIGHT: asan_blacklist.txt
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
+// CHECK-NINE: {{ }}x.h
+// CHECK-NINE-NOT: asan_blacklist.txt
#ifndef INCLUDE_FLAG_TEST
#include x.h
#endif
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -46,7 +46,36 @@
};
}
-void clang::AttachHeaderIncludeGen(Preprocessor PP, bool ShowAllHeaders,
+static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename,
+bool ShowDepth, unsigned CurrentIncludeDepth,
+bool MSStyle) {
+// Write to a temporary string to avoid unnecessary flushing on errs().
+SmallString512 Pathname(Filename);
+if (!MSStyle)
+ Lexer::Stringify(Pathname);
+
+SmallString256 Msg;
+if (MSStyle)
+ Msg += Note: including file:;
+
+if (ShowDepth) {
+ // The main source file is at depth 1, so skip one dot.
+ for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
+Msg += MSStyle ? ' ' : '.';
+
+ if (!MSStyle)
+Msg += ' ';
+}
+Msg += Pathname;
+Msg += '\n';
+
+OutputFile-write(Msg.data(), Msg.size());
+OutputFile-flush();
+}
+
+void clang::AttachHeaderIncludeGen(Preprocessor PP,
+ const std::vectorstd::string ExtraHeaders,
+ bool ShowAllHeaders,
StringRef OutputPath, bool ShowDepth,
bool MSStyle) {
raw_ostream *OutputFile = MSStyle ? llvm::outs() : llvm::errs();
@@ -69,6 +98,14 @@
}
}
+ // Print header info for extra headers, pretending they were discovered
+ // by the regular preprocessor. The primary use case is to support
+ // proper generation of Make / Ninja file dependencies for implicit includes,
+ // such as sanitizer blacklists. It's only important for cl.exe
+ // compatibility, the GNU way to generate rules is -M / -MM / -MD / -MMD.
+ for (auto Header : ExtraHeaders) {
+PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle);
+ }
PP.addPPCallbacks(llvm::make_uniqueHeaderIncludesCallback(PP,
ShowAllHeaders,
OutputFile,
@@ -112,27 +149,7 @@
// Dump the header include information we are past the predefines buffer or
// are showing all headers.
if (ShowHeader Reason == PPCallbacks::EnterFile) {
-// Write to a temporary string to avoid unnecessary flushing on errs().
-SmallString512 Filename(UserLoc.getFilename());
-if (!MSStyle)
- Lexer::Stringify(Filename);
-
-SmallString256 Msg;
-if (MSStyle)
- Msg += Note: including file:;
-
-if (ShowDepth) {
- // The main source file is at depth 1, so skip one dot.
- for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
-Msg += MSStyle ? ' ' : '.';
-
- if (!MSStyle)
-Msg += ' ';
-}
-Msg += Filename;
-Msg += '\n';
-
-OutputFile-write(Msg.data(), Msg.size());
-OutputFile-flush();
+PrintHeaderInfo(OutputFile, UserLoc.getFilename(),
+ShowDepth, CurrentIncludeDepth, MSStyle);
}
}
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -182,7 +182,11 @@
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
- OutputFormat(Opts.OutputFormat) {}
+ OutputFormat(Opts.OutputFormat) {
+for (auto ExtraDep : Opts.ExtraDeps) {
+
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin added a comment. Thank you, Peter. I will commit once I have restored my password (the email to Chris is sent) http://reviews.llvm.org/D11968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10356: scan-build: Add --analyzer-target option
honggyu.kim accepted this revision. honggyu.kim added a reviewer: honggyu.kim. honggyu.kim added a comment. This revision is now accepted and ready to land. In http://reviews.llvm.org/D10356#219772, @krememek wrote: Committed r244400 Thanks for accepting it! http://reviews.llvm.org/D10356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244843 - Wdeprecated: LambdaScopeInfos need to be copied, so make the ScopeInfo hierarchy safely copyable
Author: dblaikie
Date: Wed Aug 12 18:59:02 2015
New Revision: 244843
URL: http://llvm.org/viewvc/llvm-project?rev=244843view=rev
Log:
Wdeprecated: LambdaScopeInfos need to be copied, so make the ScopeInfo
hierarchy safely copyable
Making the base class's copy ctor protected and the derived classes
final to avoid any slicing-prone APIs.
Modified:
cfe/trunk/include/clang/Sema/ScopeInfo.h
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=244843r1=244842r2=244843view=diff
==
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Wed Aug 12 18:59:02 2015
@@ -291,6 +291,9 @@ private:
/// Part of the implementation of -Wrepeated-use-of-weak.
WeakObjectUseMap WeakObjectUses;
+protected:
+ FunctionScopeInfo(const FunctionScopeInfo) = default;
+
public:
/// Record that a weak object was accessed.
///
@@ -364,6 +367,9 @@ public:
};
class CapturingScopeInfo : public FunctionScopeInfo {
+protected:
+ CapturingScopeInfo(const CapturingScopeInfo) = default;
+
public:
enum ImplicitCaptureStyle {
ImpCap_None, ImpCap_LambdaByval, ImpCap_LambdaByref, ImpCap_Block,
@@ -549,7 +555,7 @@ public:
};
/// \brief Retains information about a block that is currently being parsed.
-class BlockScopeInfo : public CapturingScopeInfo {
+class BlockScopeInfo final : public CapturingScopeInfo {
public:
BlockDecl *TheDecl;
@@ -576,7 +582,7 @@ public:
};
/// \brief Retains information about a captured region.
-class CapturedRegionScopeInfo: public CapturingScopeInfo {
+class CapturedRegionScopeInfo final : public CapturingScopeInfo {
public:
/// \brief The CapturedDecl for this statement.
CapturedDecl *TheCapturedDecl;
@@ -617,7 +623,7 @@ public:
}
};
-class LambdaScopeInfo : public CapturingScopeInfo {
+class LambdaScopeInfo final : public CapturingScopeInfo {
public:
/// \brief The class that describes the lambda.
CXXRecordDecl *Lambda;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244829 - -Wdeprecated: Job objects are stored in a vector yet are not really copyable, make them movable instead
Author: dblaikie
Date: Wed Aug 12 18:09:24 2015
New Revision: 244829
URL: http://llvm.org/viewvc/llvm-project?rev=244829view=rev
Log:
-Wdeprecated: Job objects are stored in a vector yet are not really copyable,
make them movable instead
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=244829r1=244828r2=244829view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Aug 12 18:09:24 2015
@@ -6555,7 +6555,13 @@ class DataRecursiveIntBinOpEvaluator {
EvalResult LHSResult; // meaningful only for binary operator expression.
enum { AnyExprKind, BinOpKind, BinOpVisitedLHSKind } Kind;
-Job() : StoredInfo(nullptr) {}
+Job() = default;
+Job(Job J)
+: E(J.E), LHSResult(J.LHSResult), Kind(J.Kind),
+ StoredInfo(J.StoredInfo), OldEvalStatus(J.OldEvalStatus) {
+ J.StoredInfo = nullptr;
+}
+
void startSpeculativeEval(EvalInfo Info) {
OldEvalStatus = Info.EvalStatus;
Info.EvalStatus.Diag = nullptr;
@@ -6567,7 +6573,7 @@ class DataRecursiveIntBinOpEvaluator {
}
}
private:
-EvalInfo *StoredInfo; // non-null if status changed.
+EvalInfo *StoredInfo = nullptr; // non-null if status changed.
Expr::EvalStatus OldEvalStatus;
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D11991: Represent 2 parallel string arrays as one string[][2] array.
dougk created this revision.
dougk added a reviewer: chandlerc.
dougk added a subscriber: cfe-commits.
I think this conveys the intent better. Alternatively, since it's effectively
trying to take llvm::sys::path::parent_path() for a number of times determined
by the occurrences of '/' in one string, we could just do that without storing
the string of dot-dot-slashes although syntactic processing of pathnames is not
quite the same as semantic processing (via the actual filesystem).
http://reviews.llvm.org/D11991
Files:
lib/Driver/ToolChains.cpp
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1914,34 +1914,33 @@
llvm::Triple::ArchType TargetArch = TargetTriple.getArch();
// There are various different suffixes involving the triple we
// check for. We also record what is necessary to walk from each back
- // up to the lib directory.
- const std::string LibSuffixes[] = {
- /gcc/ + CandidateTriple.str(),
+ // up to the lib directory. Specifically, the number of up steps
+ // in the second half of each row is 1 + the number of path separators
+ // in the first half.
+ const std::string LibAndInstallSuffixes[][2] = {
+ {/gcc/ + CandidateTriple.str(), /../../..},
+
// Debian puts cross-compilers in gcc-cross
- /gcc-cross/ + CandidateTriple.str(),
- / + CandidateTriple.str() + /gcc/ + CandidateTriple.str(),
+ {/gcc-cross/ + CandidateTriple.str(), /../../..},
+
+ {/ + CandidateTriple.str() + /gcc/ + CandidateTriple.str(),
+ /../../../..},
// The Freescale PPC SDK has the gcc libraries in
// sysroot/usr/lib/triple/x.y.z so have a look there as well.
- / + CandidateTriple.str(),
+ {/ + CandidateTriple.str(), /../..},
// Ubuntu has a strange mis-matched pair of triples that this happens to
// match.
// FIXME: It may be worthwhile to generalize this and look for a second
// triple.
- /i386-linux-gnu/gcc/ + CandidateTriple.str()};
- const std::string InstallSuffixes[] = {
- /../../..,// gcc/
- /../../..,// gcc-cross/
- /../../../.., // triple/gcc/
- /../.., // triple/
- /../../../.. // i386-linux-gnu/gcc/triple/
- };
+ {/i386-linux-gnu/gcc/ + CandidateTriple.str(), /../../../..}};
+
// Only look at the final, weird Ubuntu suffix for i386-linux-gnu.
- const unsigned NumLibSuffixes =
- (llvm::array_lengthof(LibSuffixes) - (TargetArch != llvm::Triple::x86));
+ const unsigned NumLibSuffixes = (llvm::array_lengthof(LibAndInstallSuffixes)
-
+ (TargetArch != llvm::Triple::x86));
for (unsigned i = 0; i NumLibSuffixes; ++i) {
-StringRef LibSuffix = LibSuffixes[i];
+StringRef LibSuffix = LibAndInstallSuffixes[i][0];
std::error_code EC;
for (llvm::sys::fs::directory_iterator LI(LibDir + LibSuffix, EC), LE;
!EC LI != LE; LI = LI.increment(EC)) {
@@ -1975,8 +1974,9 @@
// FIXME: We hack together the directory name here instead of
// using LI to ensure stable path separators across Windows and
// Linux.
- GCCInstallPath = LibDir + LibSuffixes[i] + / + VersionText.str();
- GCCParentLibPath = GCCInstallPath + InstallSuffixes[i];
+ GCCInstallPath =
+ LibDir + LibAndInstallSuffixes[i][0] + / + VersionText.str();
+ GCCParentLibPath = GCCInstallPath + LibAndInstallSuffixes[i][1];
IsValid = true;
}
}
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1914,34 +1914,33 @@
llvm::Triple::ArchType TargetArch = TargetTriple.getArch();
// There are various different suffixes involving the triple we
// check for. We also record what is necessary to walk from each back
- // up to the lib directory.
- const std::string LibSuffixes[] = {
- /gcc/ + CandidateTriple.str(),
+ // up to the lib directory. Specifically, the number of up steps
+ // in the second half of each row is 1 + the number of path separators
+ // in the first half.
+ const std::string LibAndInstallSuffixes[][2] = {
+ {/gcc/ + CandidateTriple.str(), /../../..},
+
// Debian puts cross-compilers in gcc-cross
- /gcc-cross/ + CandidateTriple.str(),
- / + CandidateTriple.str() + /gcc/ + CandidateTriple.str(),
+ {/gcc-cross/ + CandidateTriple.str(), /../../..},
+
+ {/ + CandidateTriple.str() + /gcc/ + CandidateTriple.str(),
+ /../../../..},
// The Freescale PPC SDK has the gcc libraries in
// sysroot/usr/lib/triple/x.y.z so have a look there as well.
- / + CandidateTriple.str(),
+ {/ + CandidateTriple.str(), /../..},
// Ubuntu has a strange mis-matched pair of triples that this happens to
// match.
// FIXME: It may be worthwhile to generalize this
Re: [PATCH] D11808: Driver: Fix include and lib dirs when not using gcc under mingw
martell updated this revision to Diff 32007.
martell added a comment.
Updated to address yaron's comments.
Okay for me to merge now ?
http://reviews.llvm.org/D11808
Files:
lib/Driver/MinGWToolChain.cpp
test/Driver/Inputs/mingw_clang_tree/mingw32/i686-w64-mingw32/include/.keep
test/Driver/Inputs/mingw_clang_tree/mingw32/include/.keep
test/Driver/mingw.cpp
Index: test/Driver/mingw.cpp
===
--- test/Driver/mingw.cpp
+++ test/Driver/mingw.cpp
@@ -1,3 +1,8 @@
+// RUN: %clang -target i686-windows-gnu -c -###
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s 21 | FileCheck
-check-prefix=CHECK_MINGW_CLANG_TREE %s
+// CHECK_MINGW_CLANG_TREE:
{{.*}}/Inputs/mingw_clang_tree/mingw32{{/|}}i686-w64-mingw32{{/|}}include
+// CHECK_MINGW_CLANG_TREE:
{{.*}}/Inputs/mingw_clang_tree/mingw32{{/|}}include
+
+
// RUN: %clang -target i686-pc-windows-gnu -stdlib=libstdc++ -c -###
--sysroot=%S/Inputs/mingw_mingw_org_tree/mingw %s 21 | FileCheck
-check-prefix=CHECK_MINGW_ORG_TREE %s
// CHECK_MINGW_ORG_TREE:
{{.*}}/Inputs/mingw_mingw_org_tree/mingw{{/|}}lib{{/|}}gcc{{/|}}mingw32{{/|}}4.8.1{{/|}}include{{/|}}c++
// CHECK_MINGW_ORG_TREE:
{{.*}}/Inputs/mingw_mingw_org_tree/mingw{{/|}}lib{{/|}}gcc{{/|}}mingw32{{/|}}4.8.1{{/|}}include{{/|}}c++{{/|}}mingw32
Index: lib/Driver/MinGWToolChain.cpp
===
--- lib/Driver/MinGWToolChain.cpp
+++ lib/Driver/MinGWToolChain.cpp
@@ -47,7 +47,7 @@
Archs.emplace_back(getTriple().getArchName());
Archs[0] += -w64-mingw32;
Archs.emplace_back(mingw32);
- Arch = unknown;
+ Arch = Archs[0].str();
// lib: Arch Linux, Ubuntu, Windows
// lib64: openSUSE Linux
for (StringRef CandidateLib : {lib, lib64}) {
Index: test/Driver/mingw.cpp
===
--- test/Driver/mingw.cpp
+++ test/Driver/mingw.cpp
@@ -1,3 +1,8 @@
+// RUN: %clang -target i686-windows-gnu -c -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s 21 | FileCheck -check-prefix=CHECK_MINGW_CLANG_TREE %s
+// CHECK_MINGW_CLANG_TREE: {{.*}}/Inputs/mingw_clang_tree/mingw32{{/|}}i686-w64-mingw32{{/|}}include
+// CHECK_MINGW_CLANG_TREE: {{.*}}/Inputs/mingw_clang_tree/mingw32{{/|}}include
+
+
// RUN: %clang -target i686-pc-windows-gnu -stdlib=libstdc++ -c -### --sysroot=%S/Inputs/mingw_mingw_org_tree/mingw %s 21 | FileCheck -check-prefix=CHECK_MINGW_ORG_TREE %s
// CHECK_MINGW_ORG_TREE: {{.*}}/Inputs/mingw_mingw_org_tree/mingw{{/|}}lib{{/|}}gcc{{/|}}mingw32{{/|}}4.8.1{{/|}}include{{/|}}c++
// CHECK_MINGW_ORG_TREE: {{.*}}/Inputs/mingw_mingw_org_tree/mingw{{/|}}lib{{/|}}gcc{{/|}}mingw32{{/|}}4.8.1{{/|}}include{{/|}}c++{{/|}}mingw32
Index: lib/Driver/MinGWToolChain.cpp
===
--- lib/Driver/MinGWToolChain.cpp
+++ lib/Driver/MinGWToolChain.cpp
@@ -47,7 +47,7 @@
Archs.emplace_back(getTriple().getArchName());
Archs[0] += -w64-mingw32;
Archs.emplace_back(mingw32);
- Arch = unknown;
+ Arch = Archs[0].str();
// lib: Arch Linux, Ubuntu, Windows
// lib64: openSUSE Linux
for (StringRef CandidateLib : {lib, lib64}) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244852 - Add SourceManager::dump() to dump the current set of SLocEntries.
Author: rsmith
Date: Wed Aug 12 19:45:11 2015
New Revision: 244852
URL: http://llvm.org/viewvc/llvm-project?rev=244852view=rev
Log:
Add SourceManager::dump() to dump the current set of SLocEntries.
Modified:
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/lib/Basic/SourceManager.cpp
Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=244852r1=244851r2=244852view=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Wed Aug 12 19:45:11 2015
@@ -1463,6 +1463,8 @@ public:
///
void PrintStats() const;
+ void dump() const;
+
/// \brief Get the number of local SLocEntries we have.
unsigned local_sloc_entry_size() const { return LocalSLocEntryTable.size(); }
Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=244852r1=244851r2=244852view=diff
==
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Wed Aug 12 19:45:11 2015
@@ -2135,6 +2135,63 @@ void SourceManager::PrintStats() const {
NumBinaryProbes binary.\n;
}
+LLVM_DUMP_METHOD void SourceManager::dump() const {
+ llvm::raw_ostream out = llvm::errs();
+
+ auto DumpSLocEntry = [](int ID, const SrcMgr::SLocEntry Entry,
+ llvm::Optionalunsigned NextStart) {
+out SLocEntry FileID ID (Entry.isFile() ? file :
expansion)
+ SourceLocation Entry.getOffset() :;
+if (NextStart)
+ out *NextStart \n;
+else
+ out ???\?\n;
+if (Entry.isFile()) {
+ auto FI = Entry.getFile();
+ if (FI.NumCreatedFIDs)
+outcovers FileID ID : int(ID + FI.NumCreatedFIDs)
+ \n;
+ if (FI.getIncludeLoc().isValid())
+outincluded from FI.getIncludeLoc().getOffset() \n;
+ if (auto *CC = FI.getContentCache()) {
+outfor (CC-OrigEntry ? CC-OrigEntry-getName() :
none)
+ \n;
+if (CC-BufferOverridden)
+ outcontents overridden\n;
+if (CC-ContentsEntry != CC-OrigEntry) {
+ outcontents from
+ (CC-ContentsEntry ? CC-ContentsEntry-getName() : none)
+ \n;
+}
+ }
+} else {
+ auto EI = Entry.getExpansion();
+ outspelling from EI.getSpellingLoc().getOffset() \n;
+ outmacro (EI.isMacroArgExpansion() ? arg : body)
+range EI.getExpansionLocStart().getOffset() :
+ EI.getExpansionLocEnd().getOffset() \n;
+}
+ };
+
+ // Dump local SLocEntries.
+ for (unsigned ID = 0, NumIDs = LocalSLocEntryTable.size(); ID != NumIDs;
++ID) {
+DumpSLocEntry(ID, LocalSLocEntryTable[ID],
+ ID == NumIDs - 1 ? NextLocalOffset
+ : LocalSLocEntryTable[ID + 1].getOffset());
+ }
+ // Dump loaded SLocEntries.
+ llvm::Optionalunsigned NextStart;
+ for (unsigned Index = 0; Index != LoadedSLocEntryTable.size(); ++Index) {
+int ID = -(int)Index - 2;
+if (SLocEntryLoaded[Index]) {
+ DumpSLocEntry(ID, LoadedSLocEntryTable[Index], NextStart);
+ NextStart = LoadedSLocEntryTable[Index].getOffset();
+} else {
+ NextStart = None;
+}
+ }
+}
+
ExternalSLocEntrySource::~ExternalSLocEntrySource() { }
/// Return the amount of memory used by memory buffers, breaking down
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method
On Wed, Aug 12, 2015 at 11:16 AM, David Blaikie dblai...@gmail.com wrote:
On Wed, Aug 12, 2015 at 11:12 AM, Nico Weber tha...@chromium.org wrote:
On Wed, Aug 12, 2015 at 11:06 AM, David Blaikie dblai...@gmail.com
wrote:
On Wed, Aug 12, 2015 at 10:11 AM, Nico Weber tha...@chromium.org
wrote:
On Tue, Aug 11, 2015 at 10:15 PM, Daniel Marjamäki
daniel.marjam...@evidente.se wrote:
ideally there should be no -Wunused-parameter compiler warning when
the parameter is used.
would it feel better to move the FP warnings about virtual
functions, for instance to clang-tidy?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
imho there are some compiler warnings that are too noisy. I don't like
to get a warning when there is obviously no bug:
sign compare:
if the signed value is obviously not negative then there is no bug:
signed x; .. if (x10 x s.size())
unused parameter:
could check in the current translation unit if the parameter is used
in an overloaded method.
It doesn't warn about the presence of the parameter, but about the
presence of the name. If you say f(int, int) instead of f(int a, int b)
then the warning won't fire.
(And if you don't like this warning, then don't enable it.)
This isn't usually the approach we take with Clang's warnings - we try
to remove false positives (where false positive is usually defined as
diagnoses something which is not a bug (where bug is defined as the
resulting program behaves in a way that the user doesn't intend/expect))
where practical.
Sure, for warnings that are supposed to find bugs. The -Wunused warnings
warn about stuff that's unused, not bugs.
Seems a reasonable analog here, though, would be that a true positive for
-Wunused is when the thing really is unused and should be removed.
Commenting out the variable name is the suppression mechanism to workaround
false positives.
I disagree with this assessment. The warning warns about unused parameter
names. So this is a true positive.
If there's a targetable subset of cases where the s/n is low enough, it
could be reasonable to suppress the warning in that subset, I think.
(see improvements to -Wunreachable-code to suppress cases that are
unreachable in this build (sizeof(int) == 4 conditions, macros, etc), or
represent valid defensive programming (default in a covered enum switch) to
make the diagnostic more useful/less noisy)
If the subset of cases where this warning fires on parameters to virtual
functions produces more noise (if a significant % of cases just result in
commenting out the parameter name rather than removing the parameter) than
signal, it's certainly within the realm of discussion that we have about
whether that subset of cases is worth keeping in the warning.
- David
constructor initialization order:
should not warn if the order obviously does not matter. for instance
initialization order of pod variables using constants.
etc
Best regards,
Daniel Marjamäki
..
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)709 12 42 62
E-mail: daniel.marjam...@evidente.se
www.evidente.se
Från: tha...@google.com [tha...@google.com] f#246;r Nico Weber [
tha...@chromium.org]
Skickat: den 11 augusti 2015 20:50
Till: David Blaikie
Kopia: reviews+d11940+public+578c1335b27aa...@reviews.llvm.org;
Daniel Marjamäki; cfe-commits@lists.llvm.org
Ämne: Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual
method or method that overrides base class method
On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie dblai...@gmail.com
mailto:dblai...@gmail.com wrote:
On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
Can't you just change your signature to
virtual void a(int /* x */) {}
in these cases?
You could - does it add much value to do that, though?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
(perhaps it does - it means you express the intent that the parameter
is not used and the compiler helps you check that (so that for the
parameters you think /should/ be used (you haven't commented out their
name
but accidentally shadow or otherwise fail to reference, you still get a
warning))
- David
On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: krememek.
danielmarjamaki added a subscriber: cfe-commits.
Don't diagnose -Wunused-parameter in
[PATCH] D12000: Bugfix - Clang handles __builtin_object_size in wrong way
george.burgess.iv created this revision.
george.burgess.iv added a subscriber: cfe-commits.
Attached is a fix for https://llvm.org/bugs/show_bug.cgi?id=15212 .
Summary: Tighten up some of the results for __builtin_object_size(ptr, type)
when given type == 1 or type == 3 + fixed a bug where we would report incorrect
results for type == 3.
90% of the patch is courtesy of Richard Smith. :)
http://reviews.llvm.org/D12000
Files:
lib/AST/ExprConstant.cpp
test/CodeGen/object-size.c
test/Sema/const-eval.c
Index: test/Sema/const-eval.c
===
--- test/Sema/const-eval.c
+++ test/Sema/const-eval.c
@@ -118,10 +118,6 @@
const float constfloat = 0;
EVAL_EXPR(43, varfloat constfloat) // expected-error {{must have a constant size}}
-// rdar://problem/11205586
-// (Make sure we continue to reject this.)
-EVAL_EXPR(44, x[0]); // expected-error {{variable length array}}
-
// rdar://problem/10962435
EVAL_EXPR(45, ((char*)-1) + 1 == 0 ? 1 : -1)
EVAL_EXPR(46, ((char*)-1) + 1 (char*) -1 ? 1 : -1)
Index: test/CodeGen/object-size.c
===
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -146,3 +146,36 @@
// CHECK: call i64 @llvm.objectsize.i64
return __builtin_object_size(cond ? a : b, 0);
}
+
+// CHECK: @test19
+void test19() {
+ struct {
+char a[4];
+char b[4];
+ } foo;
+
+ // CHECK: store i32 8
+ gi = __builtin_object_size(foo.a, 0);
+ // CHECK: store i32 4
+ gi = __builtin_object_size(foo.a, 1);
+ // CHECK: store i32 8
+ gi = __builtin_object_size(foo.a, 2);
+ // CHECK: store i32 4
+ gi = __builtin_object_size(foo.a, 3);
+}
+
+// CHECK: @test20
+void test20() {
+ struct {
+int t[10];
+ } t[10];
+
+ // CHECK: store i32 380
+ gi = __builtin_object_size(t[0].t[5], 0);
+ // CHECK: store i32 20
+ gi = __builtin_object_size(t[0].t[5], 1);
+ // CHECK: store i32 380
+ gi = __builtin_object_size(t[0].t[5], 2);
+ // CHECK: store i32 20
+ gi = __builtin_object_size(t[0].t[5], 3);
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -967,10 +967,6 @@
// Check this LValue refers to an object. If not, set the designator to be
// invalid and emit a diagnostic.
bool checkSubobject(EvalInfo Info, const Expr *E, CheckSubobjectKind CSK) {
- // Outside C++11, do not build a designator referring to a subobject of
- // any object: we won't use such a designator for anything.
- if (!Info.getLangOpts().CPlusPlus11)
-Designator.setInvalid();
return (CSK == CSK_ArrayToPointer || checkNullPointer(Info, E, CSK))
Designator.checkSubobject(Info, E, CSK);
}
@@ -2713,8 +2709,7 @@
// Check for special cases where there is no existing APValue to look at.
const Expr *Base = LVal.Base.dyn_castconst Expr*();
- if (!LVal.Designator.Invalid Base !LVal.CallIndex
- !Type.isVolatileQualified()) {
+ if (Base !LVal.CallIndex !Type.isVolatileQualified()) {
if (const CompoundLiteralExpr *CLE = dyn_castCompoundLiteralExpr(Base)) {
// In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating the
// initializer until now for such expressions. Such an expression can't be
@@ -5998,8 +5993,7 @@
bool VisitSizeOfPackExpr(const SizeOfPackExpr *E);
private:
- static QualType GetObjectType(APValue::LValueBase B);
- bool TryEvaluateBuiltinObjectSize(const CallExpr *E);
+ bool TryEvaluateBuiltinObjectSize(const CallExpr *E, unsigned Type);
// FIXME: Missing: array subscript of vector, member of vector
};
} // end anonymous namespace
@@ -6171,7 +6165,7 @@
/// Retrieves the underlying object type of the given expression,
/// as used by __builtin_object_size.
-QualType IntExprEvaluator::GetObjectType(APValue::LValueBase B) {
+static QualType getObjectType(APValue::LValueBase B) {
if (const ValueDecl *D = B.dyn_castconst ValueDecl*()) {
if (const VarDecl *VD = dyn_castVarDecl(D))
return VD-getType();
@@ -6183,68 +6177,109 @@
return QualType();
}
-bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E) {
+bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E,
+unsigned Type) {
+ // Determine the denoted object.
LValue Base;
-
{
// The operand of __builtin_object_size is never evaluated for side-effects.
// If there are any, but we can determine the pointed-to object anyway, then
// ignore the side-effects.
SpeculativeEvaluationRAII SpeculativeEval(Info);
+FoldConstant Fold(Info, true);
if (!EvaluatePointer(E-getArg(0), Base, Info))
return false;
}
- if (!Base.getLValueBase()) {
-// It is not possible to determine which objects ptr points to at compile time,
-// __builtin_object_size should
Re: [PATCH] D11948: Add some macros to abstract marking of parameters as not null, and use them in cstring
- Original Message - From: Dan Albert via cfe-commits cfe-commits@lists.llvm.org To: Marshall Clow mclow.li...@gmail.com Cc: Joerg Sonnenberger jo...@britannica.bec.de, cfe-commits cfe-commits@lists.llvm.org Sent: Wednesday, August 12, 2015 6:03:30 PM Subject: Re: [PATCH] D11948: Add some macros to abstract marking of parameters as not null, and use them in cstring My testing was varied. I could not get GCC or clang to optimize it away for Linux, but both did for ARM Android. Regardless, the fact that GCC is already doing this doesn't mean it's desirable. We end up hunting and fixing bugs from this optimization this every release, and our time could be better spent. I could say the same thing about many optimizations -- I spend a lot of time tracking down bugs they expose -- the real thing to investigate here are the potential benefits to real code from performing the optimization. I'm generally in favor of these kinds of optimizations because they tend to lower abstraction penalties, but actual data here would be helpful. What would also be helpful is properly supporting -fno-delete-null-pointer-checks. -Hal On Wed, Aug 12, 2015 at 3:42 PM, Marshall Clow via cfe-commits cfe-commits@lists.llvm.org wrote: On Tue, Aug 11, 2015 at 2:28 PM, Joerg Sonnenberger via cfe-commits cfe-commits@lists.llvm.org wrote: On Tue, Aug 11, 2015 at 02:06:58PM -0700, Marshall Clow via cfe-commits wrote: On Tue, Aug 11, 2015 at 1:34 PM, Dan Albert danalb...@google.com wrote: Yeah, those sound like exactly what we want. Helping people find UB is good, but optimizing assuming we've fixed all of the UB isn't something we can do. Dan -- that's the situation you're in today. GCC has done that kind of optimization for *years*. Only on platforms that use this markup. Which is exactly the point I am raising. The gain by this optimisation is questionable at best and it has created (or exposed, however you want to call it) non-trivial bugs in the real world. There is a reason why there is a lot of push back outside glibc for this markers. I don't think that this is true. My tests (from a previous message - run on a Mac, which does not use glibc) show that gcc recognizes this and optimizes based on that. -- Marshall ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11948: Add some macros to abstract marking of parameters as not null, and use them in cstring
My testing was varied. I could not get GCC or clang to optimize it away for Linux, but both did for ARM Android. Regardless, the fact that GCC is already doing this doesn't mean it's desirable. We end up hunting and fixing bugs from this optimization this every release, and our time could be better spent. On Wed, Aug 12, 2015 at 3:42 PM, Marshall Clow via cfe-commits cfe-commits@lists.llvm.org wrote: On Tue, Aug 11, 2015 at 2:28 PM, Joerg Sonnenberger via cfe-commits cfe-commits@lists.llvm.org wrote: On Tue, Aug 11, 2015 at 02:06:58PM -0700, Marshall Clow via cfe-commits wrote: On Tue, Aug 11, 2015 at 1:34 PM, Dan Albert danalb...@google.com wrote: Yeah, those sound like exactly what we want. Helping people find UB is good, but optimizing assuming we've fixed all of the UB isn't something we can do. Dan -- that's the situation you're in today. GCC has done that kind of optimization for *years*. Only on platforms that use this markup. Which is exactly the point I am raising. The gain by this optimisation is questionable at best and it has created (or exposed, however you want to call it) non-trivial bugs in the real world. There is a reason why there is a lot of push back outside glibc for this markers. I don't think that this is true. My tests (from a previous message - run on a Mac, which does not use glibc) show that gcc recognizes this and optimizes based on that. -- Marshall ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive
On Wed, Aug 12, 2015 at 6:05 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 6:00 PM, Sean Silva chisophu...@gmail.com wrote: On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva chisophu...@gmail.com wrote: silvas added a subscriber: silvas. Comment at: lib/Parse/Parser.cpp:2003 @@ +2002,3 @@ +Diag(Tok, diag::err_unexpected_module_start); +// Recover by skipping content of the included submodule. +unsigned ModuleNesting = 1; rsmith wrote: This is liable to produce bad follow-on diagnostics; I don't think this is a reasonable way to recover. I can see a few feasible options here: 1) Emit a fatal error when this happens (suppressing further diagnostics) 2) Break out to the top level of parsing and resume from there (that is, assume that a modular header expects to be included at the top level and that the user didn't screw up their module map) 3) Enter the module and carry on parsing from here My preference would be either (1) or (2). But either way, we should diagnose the still-open bracketing constructs, because the problem is likely to be a missing close brace at the end of an unrelated header file. I strongly prefer (1). In all cases I have recorded in my notes when modularizing, the `error: expected '}'` diagnostic indicated one of two things: - that a header needed to be marked textual in the module map. - that a #include had to be moved to the top of the file (this case was likely behaving unexpectedly in the non-modules case and happened to work). For the sake of our users, it is probably best to immediately fatal error and suggest an alternative (the suggestions can be a separate patch; my recommendations are below). I believe a user is most likely to run into this diagnostic when developing an initial set of module maps, so our diagnostic should be tailored to that audience. I think your observations may be biased by your current experiences modularizing existing code, and not representative of the complete development cycle. Modularization is a one-time effort; maintaining the code after modularization is a continuous process. I think it's *far* more likely that a header listed in your module map was expected to be modular, and that a brace mismatch within that file is unintentional and due to a missing brace somewhere. I don't think so. That implies that the inclusion is not at the top of the file, which is extremely unlikely in a modular codebase. This also happens when there's a missing brace at the end of your modular header, which is almost certainly the most common way to hit this problem in an already-modularized codebase. And it happens in codebases that use a mixture of modular and non-modular headers, where there's no reason to expect all the modular includes are before the non-modular ones. :lets-do-both.jpg: Do we have a way to guard just notes behind warning flags? Maybe -Winitial-module-map or something could enable the assume this module map might have errors heuristics (which in my experience are highly reliable for that use case). We can document to users to turn this on when initially modularizing. -- Sean Silva 125993 #include lines. However, we should aim to provide diagnostics that are helpful for either case. These users may have little experience with modules when they encounter this diagnostic, so a notes suggesting a likely remedy helps them develop confidence by providing authoritative advice (and avoiding a round trip to the documentation). My fine-grained recommendations from experience are: - #include inside extern C: always meant to be modular, always the fix is to move it out of the braced block. - #include inside namespace: always meant to be modular, always the fix is to move it out of the braced block. - #include inside class: always textual (e.g. bringing in methods like TableGen .inc files; sometimes similar .inc files are manually written and not autogenerated. I have observed e.g. classfoo_methods.h files; another instance of this is stamping out .def files) - #include inside array initializer: always textual. Typically .def files - #include inside function: always textual. Typically .def files (e.g. generate a switch) .inl files are theoretically a problem inside namespaces, but I have not observed this issue in practice. In my experience code taking the effort to have .inl files is quite clean and avoids such textual dependencies. The issues I have seen in practice with .inl files usually end up coming out through a different part of clang. Improving that is for a separate patch though. http://reviews.llvm.org/D11844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11297: PR17829: Functions declared extern C with a name matching a mangled C++ function are allowed
rjmccall added a comment. You only have one attempt to define the function here; I don't see the problem. Recall that I said to add a flag to getOrCreateLLVMFunction that says whether the caller intends to define the function. The rule should be that only callers that pass true should be allowed to assume that they'll get a normal llvm::Function back. Everybody needs to be prepared to receive a bitcast. Whenever you find yourself needing to replace an existing function, just queue it up to be replaced at the end of IRGen. I don't think we need to fall over ourselves ensuring that these aliased uses properly mark functions as used or instantiate templates or anything. http://reviews.llvm.org/D11297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11808: Driver: Fix include and lib dirs when not using gcc under mingw
yaron.keren added a comment. Yes, go ahead. http://reviews.llvm.org/D11808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12002: Initial WebAssembly support in clang
sunfish updated this revision to Diff 32025.
sunfish marked 3 inline comments as done.
sunfish added a comment.
Minor changes to address review comments.
Repository:
rL LLVM
http://reviews.llvm.org/D12002
Files:
include/clang/Basic/BuiltinsWebAssembly.def
include/clang/Basic/TargetBuiltins.h
include/clang/Basic/TargetCXXABI.h
include/clang/module.modulemap
lib/AST/ASTContext.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/TargetInfo.cpp
lib/Driver/ToolChains.h
lib/Driver/Tools.cpp
test/CodeGen/builtins-wasm.c
test/CodeGen/target-data.c
test/CodeGen/wasm-arguments.c
test/CodeGen/wasm-regparm.c
test/CodeGenCXX/member-alignment.cpp
test/CodeGenCXX/member-function-pointers.cpp
test/CodeGenCXX/static-init-wasm.cpp
test/Driver/wasm32-unknown-unknown.cpp
test/Driver/wasm64-unknown-unknown.cpp
test/Preprocessor/init.c
Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -8418,3 +8418,601 @@
// XCORE:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
// XCORE:#define __LITTLE_ENDIAN__ 1
// XCORE:#define __XS1B__ 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-unknown-unknown /dev/null | FileCheck -check-prefix=WEBASSEMBLY32 %s
+//
+// WEBASSEMBLY32:#define _ILP32 1
+// WEBASSEMBLY32-NOT:#define _LP64
+// WEBASSEMBLY32:#define __ATOMIC_ACQUIRE 2
+// WEBASSEMBLY32:#define __ATOMIC_ACQ_REL 4
+// WEBASSEMBLY32:#define __ATOMIC_CONSUME 1
+// WEBASSEMBLY32:#define __ATOMIC_RELAXED 0
+// WEBASSEMBLY32:#define __ATOMIC_RELEASE 3
+// WEBASSEMBLY32:#define __ATOMIC_SEQ_CST 5
+// WEBASSEMBLY32:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY32:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+// WEBASSEMBLY32:#define __CHAR16_TYPE__ unsigned short
+// WEBASSEMBLY32:#define __CHAR32_TYPE__ unsigned int
+// WEBASSEMBLY32:#define __CHAR_BIT__ 8
+// WEBASSEMBLY32:#define __CONSTANT_CFSTRINGS__ 1
+// WEBASSEMBLY32:#define __DBL_DECIMAL_DIG__ 17
+// WEBASSEMBLY32:#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// WEBASSEMBLY32:#define __DBL_DIG__ 15
+// WEBASSEMBLY32:#define __DBL_EPSILON__ 2.2204460492503131e-16
+// WEBASSEMBLY32:#define __DBL_HAS_DENORM__ 1
+// WEBASSEMBLY32:#define __DBL_HAS_INFINITY__ 1
+// WEBASSEMBLY32:#define __DBL_HAS_QUIET_NAN__ 1
+// WEBASSEMBLY32:#define __DBL_MANT_DIG__ 53
+// WEBASSEMBLY32:#define __DBL_MAX_10_EXP__ 308
+// WEBASSEMBLY32:#define __DBL_MAX_EXP__ 1024
+// WEBASSEMBLY32:#define __DBL_MAX__ 1.7976931348623157e+308
+// WEBASSEMBLY32:#define __DBL_MIN_10_EXP__ (-307)
+// WEBASSEMBLY32:#define __DBL_MIN_EXP__ (-1021)
+// WEBASSEMBLY32:#define __DBL_MIN__ 2.2250738585072014e-308
+// WEBASSEMBLY32:#define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
+// WEBASSEMBLY32:#define __FINITE_MATH_ONLY__ 0
+// WEBASSEMBLY32:#define __FLT_DECIMAL_DIG__ 9
+// WEBASSEMBLY32:#define __FLT_DENORM_MIN__ 1.40129846e-45F
+// WEBASSEMBLY32:#define __FLT_DIG__ 6
+// WEBASSEMBLY32:#define __FLT_EPSILON__ 1.19209290e-7F
+// WEBASSEMBLY32:#define __FLT_EVAL_METHOD__ 0
+// WEBASSEMBLY32:#define __FLT_HAS_DENORM__ 1
+// WEBASSEMBLY32:#define __FLT_HAS_INFINITY__ 1
+// WEBASSEMBLY32:#define __FLT_HAS_QUIET_NAN__ 1
+// WEBASSEMBLY32:#define __FLT_MANT_DIG__ 24
+// WEBASSEMBLY32:#define __FLT_MAX_10_EXP__ 38
+// WEBASSEMBLY32:#define __FLT_MAX_EXP__ 128
+// WEBASSEMBLY32:#define __FLT_MAX__ 3.40282347e+38F
+// WEBASSEMBLY32:#define __FLT_MIN_10_EXP__ (-37)
+// WEBASSEMBLY32:#define __FLT_MIN_EXP__ (-125)
+// WEBASSEMBLY32:#define __FLT_MIN__ 1.17549435e-38F
+// WEBASSEMBLY32:#define __FLT_RADIX__ 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_INT_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// WEBASSEMBLY32:#define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_SHORT_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
+// WEBASSEMBLY32:#define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GNUC_MINOR__
+// WEBASSEMBLY32:#define __GNUC_PATCHLEVEL__
+// WEBASSEMBLY32:#define __GNUC_STDC_INLINE__ 1
+// WEBASSEMBLY32:#define __GNUC__
+// WEBASSEMBLY32:#define __GXX_ABI_VERSION 1002
+// WEBASSEMBLY32:#define __GXX_RTTI 1
+// WEBASSEMBLY32:#define __ILP32__ 1
+// WEBASSEMBLY32:#define __INT16_C_SUFFIX__ {{$}}
+// WEBASSEMBLY32:#define __INT16_FMTd__ hd
+// WEBASSEMBLY32:#define __INT16_FMTi__ hi
+// WEBASSEMBLY32:#define __INT16_MAX__ 32767
+// WEBASSEMBLY32:#define __INT16_TYPE__ short
+// WEBASSEMBLY32:#define __INT32_C_SUFFIX__ {{$}}
+// WEBASSEMBLY32:#define __INT32_FMTd__ d
+//
r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Author: krasin
Date: Wed Aug 12 23:04:37 2015
New Revision: 244867
URL: http://llvm.org/viewvc/llvm-project?rev=244867view=rev
Log:
Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Summary:
Clang sanitizers, such as AddressSanitizer, ThreadSanitizer, MemorySanitizer,
Control Flow Integrity and others, use blacklists to specify which types /
functions
should not be instrumented to avoid false positives or suppress known failures.
This change adds the blacklist filenames to the list of dependencies of the
rules,
generated with -M/-MM/-MD/-MMD. This lets CMake/Ninja recognize that certain
C/C++/ObjC files need to be recompiled (if a blacklist is updated).
Reviewers: pcc
Subscribers: rsmith, honggyu.kim, pcc, cfe-commits
Differential Revision: http://reviews.llvm.org/D11968
Modified:
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp
cfe/trunk/test/Frontend/dependency-gen.c
cfe/trunk/test/Frontend/print-header-includes.c
Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Wed Aug 12
23:04:37 2015
@@ -47,6 +47,9 @@ public:
/// must contain at least one entry.
std::vectorstd::string Targets;
+ /// A list of filenames to be used as extra dependencies for every target.
+ std::vectorstd::string ExtraDeps;
+
/// \brief The file to write GraphViz-formatted header dependencies to.
std::string DOTOutputFile;
Modified: cfe/trunk/include/clang/Frontend/Utils.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/include/clang/Frontend/Utils.h (original)
+++ cfe/trunk/include/clang/Frontend/Utils.h Wed Aug 12 23:04:37 2015
@@ -148,6 +148,9 @@ public:
/// AttachHeaderIncludeGen - Create a header include list generator, and attach
/// it to the given preprocessor.
///
+/// \param ExtraHeaders - If not empty, will write the header filenames, just
+/// like they were included during a regular preprocessing. Useful for
+/// implicit include dependencies, like sanitizer blacklists.
/// \param ShowAllHeaders - If true, show all header information instead of
just
/// headers following the predefines buffer. This is useful for making sure
/// includes mentioned on the command line are also reported, but differs from
@@ -156,7 +159,9 @@ public:
/// information to, instead of writing to stderr.
/// \param ShowDepth - Whether to indent to show the nesting of the includes.
/// \param MSStyle - Whether to print in cl.exe /showIncludes style.
-void AttachHeaderIncludeGen(Preprocessor PP, bool ShowAllHeaders = false,
+void AttachHeaderIncludeGen(Preprocessor PP,
+const std::vectorstd::string ExtraHeaders,
+bool ShowAllHeaders = false,
StringRef OutputPath = ,
bool ShowDepth = true, bool MSStyle = false);
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Aug 12 23:04:37 2015
@@ -354,17 +354,19 @@ void CompilerInstance::createPreprocesso
// Handle generating header include information, if requested.
if (DepOpts.ShowHeaderIncludes)
-AttachHeaderIncludeGen(*PP);
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps);
if (!DepOpts.HeaderIncludeOutputFile.empty()) {
StringRef OutputPath = DepOpts.HeaderIncludeOutputFile;
if (OutputPath == -)
OutputPath = ;
-AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/true, OutputPath,
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+ /*ShowAllHeaders=*/true, OutputPath,
/*ShowDepth=*/false);
}
if (DepOpts.PrintShowIncludes) {
-AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/false, /*OutputPath=*/,
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+ /*ShowAllHeaders=*/false, /*OutputPath=*/,
/*ShowDepth=*/true, /*MSStyle=*/true);
}
}
Modified:
Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive
On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva chisophu...@gmail.com wrote: silvas added a subscriber: silvas. Comment at: lib/Parse/Parser.cpp:2003 @@ +2002,3 @@ +Diag(Tok, diag::err_unexpected_module_start); +// Recover by skipping content of the included submodule. +unsigned ModuleNesting = 1; rsmith wrote: This is liable to produce bad follow-on diagnostics; I don't think this is a reasonable way to recover. I can see a few feasible options here: 1) Emit a fatal error when this happens (suppressing further diagnostics) 2) Break out to the top level of parsing and resume from there (that is, assume that a modular header expects to be included at the top level and that the user didn't screw up their module map) 3) Enter the module and carry on parsing from here My preference would be either (1) or (2). But either way, we should diagnose the still-open bracketing constructs, because the problem is likely to be a missing close brace at the end of an unrelated header file. I strongly prefer (1). In all cases I have recorded in my notes when modularizing, the `error: expected '}'` diagnostic indicated one of two things: - that a header needed to be marked textual in the module map. - that a #include had to be moved to the top of the file (this case was likely behaving unexpectedly in the non-modules case and happened to work). For the sake of our users, it is probably best to immediately fatal error and suggest an alternative (the suggestions can be a separate patch; my recommendations are below). I believe a user is most likely to run into this diagnostic when developing an initial set of module maps, so our diagnostic should be tailored to that audience. I think your observations may be biased by your current experiences modularizing existing code, and not representative of the complete development cycle. Modularization is a one-time effort; maintaining the code after modularization is a continuous process. I think it's *far* more likely that a header listed in your module map was expected to be modular, and that a brace mismatch within that file is unintentional and due to a missing brace somewhere. I don't think so. That implies that the inclusion is not at the top of the file, which is extremely unlikely in a modular codebase. 125993 #include lines. However, we should aim to provide diagnostics that are helpful for either case. These users may have little experience with modules when they encounter this diagnostic, so a notes suggesting a likely remedy helps them develop confidence by providing authoritative advice (and avoiding a round trip to the documentation). My fine-grained recommendations from experience are: - #include inside extern C: always meant to be modular, always the fix is to move it out of the braced block. - #include inside namespace: always meant to be modular, always the fix is to move it out of the braced block. - #include inside class: always textual (e.g. bringing in methods like TableGen .inc files; sometimes similar .inc files are manually written and not autogenerated. I have observed e.g. classfoo_methods.h files; another instance of this is stamping out .def files) - #include inside array initializer: always textual. Typically .def files - #include inside function: always textual. Typically .def files (e.g. generate a switch) .inl files are theoretically a problem inside namespaces, but I have not observed this issue in practice. In my experience code taking the effort to have .inl files is quite clean and avoids such textual dependencies. The issues I have seen in practice with .inl files usually end up coming out through a different part of clang. Improving that is for a separate patch though. http://reviews.llvm.org/D11844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive
On Wed, Aug 12, 2015 at 6:00 PM, Sean Silva chisophu...@gmail.com wrote: On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva chisophu...@gmail.com wrote: silvas added a subscriber: silvas. Comment at: lib/Parse/Parser.cpp:2003 @@ +2002,3 @@ +Diag(Tok, diag::err_unexpected_module_start); +// Recover by skipping content of the included submodule. +unsigned ModuleNesting = 1; rsmith wrote: This is liable to produce bad follow-on diagnostics; I don't think this is a reasonable way to recover. I can see a few feasible options here: 1) Emit a fatal error when this happens (suppressing further diagnostics) 2) Break out to the top level of parsing and resume from there (that is, assume that a modular header expects to be included at the top level and that the user didn't screw up their module map) 3) Enter the module and carry on parsing from here My preference would be either (1) or (2). But either way, we should diagnose the still-open bracketing constructs, because the problem is likely to be a missing close brace at the end of an unrelated header file. I strongly prefer (1). In all cases I have recorded in my notes when modularizing, the `error: expected '}'` diagnostic indicated one of two things: - that a header needed to be marked textual in the module map. - that a #include had to be moved to the top of the file (this case was likely behaving unexpectedly in the non-modules case and happened to work). For the sake of our users, it is probably best to immediately fatal error and suggest an alternative (the suggestions can be a separate patch; my recommendations are below). I believe a user is most likely to run into this diagnostic when developing an initial set of module maps, so our diagnostic should be tailored to that audience. I think your observations may be biased by your current experiences modularizing existing code, and not representative of the complete development cycle. Modularization is a one-time effort; maintaining the code after modularization is a continuous process. I think it's *far* more likely that a header listed in your module map was expected to be modular, and that a brace mismatch within that file is unintentional and due to a missing brace somewhere. I don't think so. That implies that the inclusion is not at the top of the file, which is extremely unlikely in a modular codebase. 125993 #include lines. Oops, apparently I accidentally hit send... I'm gathering some real statistics on this. -- Sean Silva However, we should aim to provide diagnostics that are helpful for either case. These users may have little experience with modules when they encounter this diagnostic, so a notes suggesting a likely remedy helps them develop confidence by providing authoritative advice (and avoiding a round trip to the documentation). My fine-grained recommendations from experience are: - #include inside extern C: always meant to be modular, always the fix is to move it out of the braced block. - #include inside namespace: always meant to be modular, always the fix is to move it out of the braced block. - #include inside class: always textual (e.g. bringing in methods like TableGen .inc files; sometimes similar .inc files are manually written and not autogenerated. I have observed e.g. classfoo_methods.h files; another instance of this is stamping out .def files) - #include inside array initializer: always textual. Typically .def files - #include inside function: always textual. Typically .def files (e.g. generate a switch) .inl files are theoretically a problem inside namespaces, but I have not observed this issue in practice. In my experience code taking the effort to have .inl files is quite clean and avoids such textual dependencies. The issues I have seen in practice with .inl files usually end up coming out through a different part of clang. Improving that is for a separate patch though. http://reviews.llvm.org/D11844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244855 - Update for llvm api change.
Author: rafael Date: Wed Aug 12 20:07:06 2015 New Revision: 244855 URL: http://llvm.org/viewvc/llvm-project?rev=244855view=rev Log: Update for llvm api change. Modified: cfe/trunk/tools/driver/driver.cpp Modified: cfe/trunk/tools/driver/driver.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=244855r1=244854r2=244855view=diff == --- cfe/trunk/tools/driver/driver.cpp (original) +++ cfe/trunk/tools/driver/driver.cpp Wed Aug 12 20:07:06 2015 @@ -406,7 +406,7 @@ int main(int argc_, const char **argv_) const DriverSuffix *DS = parseDriverSuffix(ProgName); llvm::BumpPtrAllocator A; - llvm::BumpPtrStringSaver Saver(A); + llvm::StringSaver Saver(A); // Parse response files using the GNU syntax, unless we're in CL mode. There // are two ways to put clang in CL compatibility mode: argv[0] is either ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11932: [OPENMP] Link libomp.lib on Windows
ismail added a comment. Tests pass but, now when I tried to compile a file with -fopenmp I get: LINK: fatal error LNK1104: cannot open file 'libomp.lib' libomp.lib does exist in C:\Program Files\LLVM\lib but I guess we need to pass the library path down to linker. http://reviews.llvm.org/D11932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11963: Create a __config_site file to capture configuration decisions.
jroelofs updated this revision to Diff 31949.
jroelofs added a comment.
Add license text to the new file, and move the two has-no-threads tests to
test/libcxx.
http://reviews.llvm.org/D11963
Files:
CMakeLists.txt
include/CMakeLists.txt
include/__config
include/__config_site.in
test/libcxx/atomics/libcpp-has-no-threads.pass.cpp
test/libcxx/test/config.py
test/std/atomics/libcpp-has-no-threads.fail.cpp
test/std/atomics/libcpp-has-no-threads.pass.cpp
test/std/utilities/date.time/tested_elsewhere.pass.cpp
Index: test/std/utilities/date.time/tested_elsewhere.pass.cpp
===
--- test/std/utilities/date.time/tested_elsewhere.pass.cpp
+++ test/std/utilities/date.time/tested_elsewhere.pass.cpp
@@ -30,9 +30,11 @@
static_assert((std::is_samedecltype(std::difftime(t,t)), double::value), );
static_assert((std::is_samedecltype(std::mktime(tm)), std::time_t::value), );
static_assert((std::is_samedecltype(std::time(t)), std::time_t::value), );
+#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS
static_assert((std::is_samedecltype(std::asctime(tm)), char*::value), );
static_assert((std::is_samedecltype(std::ctime(t)), char*::value), );
static_assert((std::is_samedecltype(std::gmtime(t)), std::tm*::value), );
static_assert((std::is_samedecltype(std::localtime(t)), std::tm*::value), );
+#endif
static_assert((std::is_samedecltype(std::strftime(str,s,,tm)), std::size_t::value), );
}
Index: test/std/atomics/libcpp-has-no-threads.pass.cpp
===
--- test/std/atomics/libcpp-has-no-threads.pass.cpp
+++ test/std/atomics/libcpp-has-no-threads.pass.cpp
@@ -1,18 +0,0 @@
-//===--===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-// XFAIL: libcpp-has-no-threads
-
-#ifdef _LIBCPP_HAS_NO_THREADS
-#error This should be XFAIL'd for the purpose of detecting that the LIT feature\
- 'libcpp-has-no-threads' is available iff _LIBCPP_HAS_NO_THREADS is defined
-#endif
-
-int main()
-{
-}
Index: test/std/atomics/libcpp-has-no-threads.fail.cpp
===
--- test/std/atomics/libcpp-has-no-threads.fail.cpp
+++ test/std/atomics/libcpp-has-no-threads.fail.cpp
@@ -1,23 +0,0 @@
-//===--===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-
-// atomic
-
-// Test that including atomic fails to compile when _LIBCPP_HAS_NO_THREADS
-// is defined.
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-#define _LIBCPP_HAS_NO_THREADS
-#endif
-
-#include atomic
-
-int main()
-{
-}
Index: test/libcxx/test/config.py
===
--- test/libcxx/test/config.py
+++ test/libcxx/test/config.py
@@ -392,7 +392,7 @@
self.cxx.compile_flags += ['-I' + support_path]
self.cxx.compile_flags += ['-include', os.path.join(support_path, 'nasty_macros.hpp')]
libcxx_headers = self.get_lit_conf(
-'libcxx_headers', os.path.join(self.libcxx_src_root, 'include'))
+'libcxx_headers', os.path.join(self.cxx_library_root, '..', 'include', 'c++', 'v1'))
if not os.path.isdir(libcxx_headers):
self.lit_config.fatal(libcxx_headers='%s' is not a directory.
% libcxx_headers)
@@ -416,36 +416,28 @@
if not enable_global_filesystem_namespace:
self.config.available_features.add(
'libcpp-has-no-global-filesystem-namespace')
-self.cxx.compile_flags += [
-'-D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE']
def configure_compile_flags_no_stdin(self):
enable_stdin = self.get_lit_bool('enable_stdin', True)
if not enable_stdin:
self.config.available_features.add('libcpp-has-no-stdin')
-self.cxx.compile_flags += ['-D_LIBCPP_HAS_NO_STDIN']
def configure_compile_flags_no_stdout(self):
enable_stdout = self.get_lit_bool('enable_stdout', True)
if not enable_stdout:
self.config.available_features.add('libcpp-has-no-stdout')
-self.cxx.compile_flags += ['-D_LIBCPP_HAS_NO_STDOUT']
def configure_compile_flags_no_threads(self):
-self.cxx.compile_flags += ['-D_LIBCPP_HAS_NO_THREADS']
self.config.available_features.add('libcpp-has-no-threads')
def
Re: [PATCH] D10365: Add cmd to compilation database file format
diltsman updated this revision to Diff 31952.
diltsman marked an inline comment as done.
http://reviews.llvm.org/D10365
Files:
../llvm/tools/clang/include/clang/Tooling/JSONCompilationDatabase.h
../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
Index: ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ ../llvm/tools/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -36,8 +36,13 @@
expectFailure([{[]:\\}], Incorrectly typed entry);
expectFailure([{}], Empty entry);
expectFailure([{\directory\:\\,\command\:\\}], Missing file);
- expectFailure([{\directory\:\\,\file\:\\}], Missing command);
+ expectFailure([{\directory\:\\,\file\:\\}], Missing command or arguments);
expectFailure([{\command\:\\,\file\:\\}], Missing directory);
+ expectFailure([{\directory\:\\,\arguments\:[]}], Missing file);
+ expectFailure([{\arguments\:\\,\file\:\\}], Missing directory);
+ expectFailure([{\command\:\\,\arguments\:[],\file\:\\}], Command and arguments);
+ expectFailure([{\directory\:\\,\arguments\:\\,\file\:\\}], Arguments not array);
+ expectFailure([{\directory\:\\,\command\:[],\file\:\\}], Command not string);
}
static std::vectorstd::string getAllFiles(StringRef JSONDatabase,
Index: ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -221,9 +221,8 @@
SmallString8 DirectoryStorage;
SmallString1024 CommandStorage;
Commands.emplace_back(
-// FIXME: Escape correctly:
-CommandsRef[I].first-getValue(DirectoryStorage),
-unescapeCommandLine(CommandsRef[I].second-getValue(CommandStorage)));
+ CommandsRef[I].first-getValue(DirectoryStorage),
+ CommandsRef[I].second);
}
}
@@ -243,44 +242,66 @@
ErrorMessage = Expected array.;
return false;
}
- for (llvm::yaml::SequenceNode::iterator AI = Array-begin(),
- AE = Array-end();
- AI != AE; ++AI) {
-llvm::yaml::MappingNode *Object = dyn_castllvm::yaml::MappingNode(*AI);
+ for (auto NextObject : *Array) {
+llvm::yaml::MappingNode *Object = dyn_castllvm::yaml::MappingNode(NextObject);
if (!Object) {
ErrorMessage = Expected object.;
return false;
}
llvm::yaml::ScalarNode *Directory = nullptr;
-llvm::yaml::ScalarNode *Command = nullptr;
+std::vectorstd::string Args;
+bool CommandFound = false;
llvm::yaml::ScalarNode *File = nullptr;
-for (llvm::yaml::MappingNode::iterator KVI = Object-begin(),
- KVE = Object-end();
- KVI != KVE; ++KVI) {
- llvm::yaml::Node *Value = (*KVI).getValue();
+for (auto NextKeyValue : *Object) {
+ llvm::yaml::ScalarNode *KeyString =
+ dyn_castllvm::yaml::ScalarNode(NextKeyValue.getKey());
+ if (!KeyString) {
+ErrorMessage = Expected strings as key.;
+return false;
+ }
+ SmallString8 KeyStorage;
+ StringRef KeyValue = KeyString-getValue(KeyStorage);
+ llvm::yaml::Node *Value = NextKeyValue.getValue();
if (!Value) {
ErrorMessage = Expected value.;
return false;
}
llvm::yaml::ScalarNode *ValueString =
dyn_castllvm::yaml::ScalarNode(Value);
- if (!ValueString) {
-ErrorMessage = Expected string as value.;
+ llvm::yaml::SequenceNode *SequenceString =
+ dyn_castllvm::yaml::SequenceNode(Value);
+ if (KeyValue == arguments !SequenceString) {
+ErrorMessage = Expected sequence as value.;
return false;
- }
- llvm::yaml::ScalarNode *KeyString =
- dyn_castllvm::yaml::ScalarNode((*KVI).getKey());
- if (!KeyString) {
-ErrorMessage = Expected strings as key.;
+ } else if (KeyValue != arguments !ValueString) {
+ErrorMessage = Expected string as value.;
return false;
}
- SmallString8 KeyStorage;
- if (KeyString-getValue(KeyStorage) == directory) {
+ if (KeyValue == directory) {
Directory = ValueString;
- } else if (KeyString-getValue(KeyStorage) == command) {
-Command = ValueString;
- } else if (KeyString-getValue(KeyStorage) == file) {
+ } else if (KeyValue == command) {
+if (CommandFound) {
+ ErrorMessage = Multiple command and arguments found;
+ return false;
+}
+SmallString1024 CommandStorage;
+// FIXME: Escape correctly:
+Args = unescapeCommandLine(ValueString-getValue(CommandStorage));
+CommandFound = true;
+ } else if (KeyValue == file) {
File =
Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method
On Tue, Aug 11, 2015 at 10:15 PM, Daniel Marjamäki
daniel.marjam...@evidente.se wrote:
ideally there should be no -Wunused-parameter compiler warning when the
parameter is used.
would it feel better to move the FP warnings about virtual functions,
for instance to clang-tidy?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
imho there are some compiler warnings that are too noisy. I don't like to
get a warning when there is obviously no bug:
sign compare:
if the signed value is obviously not negative then there is no bug:
signed x; .. if (x10 x s.size())
unused parameter:
could check in the current translation unit if the parameter is used in
an overloaded method.
It doesn't warn about the presence of the parameter, but about the presence
of the name. If you say f(int, int) instead of f(int a, int b) then the
warning won't fire. (And if you don't like this warning, then don't enable
it.)
constructor initialization order:
should not warn if the order obviously does not matter. for instance
initialization order of pod variables using constants.
etc
Best regards,
Daniel Marjamäki
..
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)709 12 42 62
E-mail: daniel.marjam...@evidente.se
www.evidente.se
Från: tha...@google.com [tha...@google.com] f#246;r Nico Weber [
tha...@chromium.org]
Skickat: den 11 augusti 2015 20:50
Till: David Blaikie
Kopia: reviews+d11940+public+578c1335b27aa...@reviews.llvm.org; Daniel
Marjamäki; cfe-commits@lists.llvm.org
Ämne: Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual
method or method that overrides base class method
On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie dblai...@gmail.com
mailto:dblai...@gmail.com wrote:
On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
Can't you just change your signature to
virtual void a(int /* x */) {}
in these cases?
You could - does it add much value to do that, though?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
(perhaps it does - it means you express the intent that the parameter is
not used and the compiler helps you check that (so that for the parameters
you think /should/ be used (you haven't commented out their name but
accidentally shadow or otherwise fail to reference, you still get a
warning))
- David
On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: krememek.
danielmarjamaki added a subscriber: cfe-commits.
Don't diagnose -Wunused-parameter in methods that override other methods
because the overridden methods might use the parameter
Don't diagnose -Wunused-parameter in virtual methods because these might
be overriden by other methods that use the parameter.
Such diagnostics could be more accurately written if they are based on
whole-program-analysis that establish if such parameter is unused in all
methods.
http://reviews.llvm.org/D11940
Files:
lib/Sema/SemaDecl.cpp
test/SemaCXX/warn-unused-parameters.cpp
Index: test/SemaCXX/warn-unused-parameters.cpp
===
--- test/SemaCXX/warn-unused-parameters.cpp
+++ test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,20 @@
auto l = [t...]() { return sizeof...(s); };
return l();
}
+
+// Don't diagnose virtual methods or methods that override base class
+// methods.
+class Base {
+public:
+ virtual void f(int x);
+};
+
+class Derived : public Base {
+public:
+ // Don't warn in overridden methods.
+ virtual void f(int x) {}
+
+ // Don't warn in virtual methods.
+ virtual void a(int x) {}
+};
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10797,8 +10797,13 @@
if (!FD-isInvalidDecl()) {
// Don't diagnose unused parameters of defaulted or deleted
functions.
- if (!FD-isDeleted() !FD-isDefaulted())
-DiagnoseUnusedParameters(FD-param_begin(), FD-param_end());
+ if (!FD-isDeleted() !FD-isDefaulted()) {
+// Don't diagnose unused parameters in virtual methods or
+// in methods that override base class methods.
+const auto MD = dyn_castCXXMethodDecl(FD);
+if (!MD || (MD-size_overridden_methods() == 0U
!MD-isVirtual()))
+
r244761 - ReleaseNotes: Small version nbr fix
Author: hans Date: Wed Aug 12 11:40:42 2015 New Revision: 244761 URL: http://llvm.org/viewvc/llvm-project?rev=244761view=rev Log: ReleaseNotes: Small version nbr fix Modified: cfe/trunk/docs/ReleaseNotes.rst Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=244761r1=244760r2=244761view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Wed Aug 12 11:40:42 2015 @@ -54,7 +54,7 @@ Improvements to Clang's diagnostics Clang's diagnostics are constantly being improved to catch more issues, explain them more clearly, and provide more accurate source information -about them. The improvements since the 3.5 release include: +about them. The improvements since the 3.7 release include: - ... ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11946: Create clang-tidy module modernize. Add pass-by-value check.
alexfh added a comment.
Something weird happened with the file names in this CL after the latest
update, e.g. there are both `clang-tidy/CMakeLists.txt` and `CMakeLists.txt`.
How exactly did you create the Differential revision and update it?
Comment at: ModernizeTidyModule.cpp:13
@@ +12,3 @@
+#include ../ClangTidyModuleRegistry.h
+#include ../readability/BracesAroundStatementsCheck.h
+#include ../readability/FunctionSizeCheck.h
Looks like you've forgotten to remove the `../readability` includes after
copy-pasting.
Comment at: ModernizeTidyModule.cpp:31
@@ +30,3 @@
+
+ ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
Just don't override this method, if you don't add any module options.
Comment at: PassByValueCheck.cpp:26
@@ +25,3 @@
+
+/// \brief Matches move constructible classes.
+///
`move-constructible` seems slightly more common than `move constructible`, but
I'm not a native speaker.
Comment at: PassByValueCheck.cpp:30
@@ +29,3 @@
+/// \code
+/// // POD types are trivially move constructible
+/// struct Foo { int a; };
nit: Missing trailing period.
Comment at: PassByValueCheck.cpp:82
@@ +81,3 @@
+ /// given constructor.
+ bool hasExactlyOneUsageIn(const CXXConstructorDecl *Ctor) {
+Count = 0;
This method seems to be the only useful interface of this class. How about
making this method a free-standing function and make this class an
implementation detail of it (e.g. by defining it inside the function)?
Comment at: PassByValueCheck.cpp:93
@@ +92,3 @@
+ bool VisitDeclRefExpr(DeclRefExpr *D) {
+if (const ParmVarDecl *To = dyn_castParmVarDecl(D-getDecl()))
+ if (To == ParamDecl) {
nit: I'd put the body of this conditional statement in braces as it spans more
than one line.
Comment at: PassByValueCheck.cpp:93
@@ +92,3 @@
+ bool VisitDeclRefExpr(DeclRefExpr *D) {
+if (const ParmVarDecl *To = dyn_castParmVarDecl(D-getDecl()))
+ if (To == ParamDecl) {
alexfh wrote:
nit: I'd put the body of this conditional statement in braces as it spans
more than one line.
nit: Since the `To` variable is only used once, why not just compare
`dyn_cast...(...)` with `ParamDecl`?
Comment at: PassByValueCheck.cpp:96
@@ +95,3 @@
+++Count;
+if (Count 1)
+ // no need to look further, used more than once
nit: I'd put the body of this conditional statement in braces as it spans more
than one line.
Comment at: PassByValueCheck.cpp:97
@@ +96,3 @@
+if (Count 1)
+ // no need to look further, used more than once
+ return false;
Please use proper Capitalization and punctuation (namely, trailing period).
Comment at: PassByValueCheck.cpp:112
@@ +111,3 @@
+ const ParmVarDecl *ParamDecl) {
+ ExactlyOneUsageVisitor Visitor(ParamDecl);
+ return Visitor.hasExactlyOneUsageIn(Ctor);
nit: Consider moving the class definition here since it's not used elsewhere.
Also, maybe `return
ExactlyOneUsageVisitor(ParamDecl).hasExactlyOneUsageIn(Ctor);`?
Comment at: PassByValueCheck.cpp:118
@@ +117,3 @@
+/// redeclarations of \p Ctor.
+static void collectParamDecls(const CXXConstructorDecl *Ctor,
+ const ParmVarDecl *ParamDecl,
nit: Maybe return the result?
Comment at: PassByValueCheck.cpp:120
@@ +119,3 @@
+ const ParmVarDecl *ParamDecl,
+ SmallVectorImplconst ParmVarDecl * Results) {
+ unsigned ParamIdx = ParamDecl-getFunctionScopeIndex();
Please use `ArrayRef` (passed by value) instead. It's a more generic way to
pass a range of items to a function.
Comment at: PassByValueCheck.cpp:140
@@ +139,3 @@
+ hasType(qualType(
+ // match only constref or a non-const value
+ // parameters, rvalues and const-values
Capitalization. Punctuation.
Comment at: PassByValueCheck.cpp:161
@@ +160,3 @@
+void PassByValueCheck::check(const MatchFinder::MatchResult Result) {
+ const CXXConstructorDecl *Ctor =
+ Result.Nodes.getNodeAsCXXConstructorDecl(Ctor);
RHS already spells the type, use `const auto *Ctor`.
Comment at: PassByValueCheck.cpp:163
@@ +162,3 @@
+ Result.Nodes.getNodeAsCXXConstructorDecl(Ctor);
+ const ParmVarDecl *ParamDecl = Result.Nodes.getNodeAsParmVarDecl(Param);
+ const CXXCtorInitializer *Initializer =
ditto
Comment at: PassByValueCheck.cpp:164
@@ +163,3 @@
+ const
Re: [PATCH] D11658: [Sema] main can't be declared as global variable
davide updated this revision to Diff 31932.
davide added a comment.
- Refactored check
- Added test for -ffreestanding in C
- Changed the diagnostic emitted
Also, thanks for your time and guidance.
http://reviews.llvm.org/D11658
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/CXX/basic/basic.start/basic.start.main/p3.cpp
test/Sema/warn-extern-main.c
Index: test/Sema/warn-extern-main.c
===
--- test/Sema/warn-extern-main.c
+++ test/Sema/warn-extern-main.c
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST1
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST2
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST3
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST4
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST5
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST6
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST7
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST8
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST9 -ffreestanding
+
+#if TEST1
+int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+
+#elif TEST2
+extern int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+
+#elif TEST3
+// expected-no-diagnostics
+void x() {
+ static int main;
+}
+
+#elif TEST4
+void x() {
+ extern int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+}
+
+#elif TEST5
+// expected-no-diagnostics
+void x() {
+ int main;
+}
+
+#elif TEST6
+// expected-no-diagnostics
+static int main;
+
+#elif TEST7
+// expected-no-diagnostics
+void x() {
+ auto int main;
+}
+
+#elif TEST8
+// expected-no-diagnostics
+void x() {
+ register int main;
+}
+
+#elif TEST9
+// expected-no-diagnostics
+int main;
+
+#else
+#error Unknown Test
+#endif
Index: test/CXX/basic/basic.start/basic.start.main/p3.cpp
===
--- test/CXX/basic/basic.start/basic.start.main/p3.cpp
+++ test/CXX/basic/basic.start/basic.start.main/p3.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST1
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST2
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST3
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST4
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 -DTEST5
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 -DTEST6
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST7
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST8
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST9
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST10 -ffreestanding
+
+#if TEST1
+int main; // expected-error{{main can't be declared as global variable}}
+
+#elif TEST2
+// expected-no-diagnostics
+int f () {
+ int main;
+ return main;
+}
+
+#elif TEST3
+// expected-no-diagnostics
+void x(int main) {};
+int y(int main);
+
+#elif TEST4
+// expected-no-diagnostics
+class A {
+ static int main;
+};
+
+#elif TEST5
+// expected-no-diagnostics
+templateclass T constexpr T main;
+
+#elif TEST6
+extern templateclass T constexpr T main; //expected-error{{expected unqualified-id}}
+
+#elif TEST7
+// expected-no-diagnostics
+namespace foo {
+ int main;
+}
+
+#elif TEST8
+void z(void)
+{
+ extern int main; // expected-error{{main can't be declared as global variable}}
+}
+
+#elif TEST9
+// expected-no-diagnostics
+int q(void)
+{
+ static int main;
+ return main;
+}
+
+#elif TEST10
+// expected-no-diagnostics
+int main;
+
+#else
+#error Unknown Test
+#endif
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6111,6 +6111,22 @@
}
}
+ // Special handling of variable named 'main'.
+ if (Name.isIdentifier() Name.getAsIdentifierInfo()-isStr(main)
+ NewVD-getDeclContext()-getRedeclContext()-isTranslationUnit()
+ !getLangOpts().Freestanding !NewVD-getDescribedVarTemplate()) {
+
+// C++ [basic.start.main]p3
+// A program that declares a variable main at global scope is ill-formed.
+if (getLangOpts().CPlusPlus)
+ Diag(D.getLocStart(), diag::err_main_global_variable);
+
+// In C, and external-linkage variable named main results in undefined
+// behavior.
+else if (NewVD-hasExternalFormalLinkage())
+ Diag(D.getLocStart(), diag::warn_main_redefined);
+ }
+
if (D.isRedeclaration() !Previous.empty()) {
checkDLLAttributeRedeclaration(
*this, dyn_castNamedDecl(Previous.getRepresentativeDecl()), NewVD,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -510,6 +510,9 @@
def err_main_arg_wrong : Error%select{first|second|third|fourth}0
parameter of 'main' (%select{argument count|argument
Re: [PATCH] D10365: Add cmd to compilation database file format
klimek added a comment.
Ok, we're very close now :) Thanks for taking the time to work through this!
Comment at: ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp:299
@@ +298,3 @@
+if (CommandFound) {
+ ErrorMessage = Multiple command and arguments found;
+ return false;
Any reason we don't want to allow both, but prefer the arguments?
Comment at: ../llvm/tools/clang/lib/Tooling/JSONCompilationDatabase.cpp:302-304
@@ +301,5 @@
+}
+for (llvm::yaml::SequenceNode::iterator CI = SequenceString-begin(),
+ CE = SequenceString-end();
+ CI != CE; ++CI) {
+ SmallString128 CommandStorage;
Can we use for-range loops with auto?
http://reviews.llvm.org/D10365
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method
On Wed, Aug 12, 2015 at 11:06 AM, David Blaikie dblai...@gmail.com wrote:
On Wed, Aug 12, 2015 at 10:11 AM, Nico Weber tha...@chromium.org wrote:
On Tue, Aug 11, 2015 at 10:15 PM, Daniel Marjamäki
daniel.marjam...@evidente.se wrote:
ideally there should be no -Wunused-parameter compiler warning when the
parameter is used.
would it feel better to move the FP warnings about virtual functions,
for instance to clang-tidy?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
imho there are some compiler warnings that are too noisy. I don't like
to get a warning when there is obviously no bug:
sign compare:
if the signed value is obviously not negative then there is no bug:
signed x; .. if (x10 x s.size())
unused parameter:
could check in the current translation unit if the parameter is used
in an overloaded method.
It doesn't warn about the presence of the parameter, but about the
presence of the name. If you say f(int, int) instead of f(int a, int b)
then the warning won't fire.
(And if you don't like this warning, then don't enable it.)
This isn't usually the approach we take with Clang's warnings - we try to
remove false positives (where false positive is usually defined as
diagnoses something which is not a bug (where bug is defined as the
resulting program behaves in a way that the user doesn't intend/expect))
where practical.
Sure, for warnings that are supposed to find bugs. The -Wunused warnings
warn about stuff that's unused, not bugs.
If the subset of cases where this warning fires on parameters to virtual
functions produces more noise (if a significant % of cases just result in
commenting out the parameter name rather than removing the parameter) than
signal, it's certainly within the realm of discussion that we have about
whether that subset of cases is worth keeping in the warning.
- David
constructor initialization order:
should not warn if the order obviously does not matter. for instance
initialization order of pod variables using constants.
etc
Best regards,
Daniel Marjamäki
..
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)709 12 42 62
E-mail: daniel.marjam...@evidente.se
www.evidente.se
Från: tha...@google.com [tha...@google.com] f#246;r Nico Weber [
tha...@chromium.org]
Skickat: den 11 augusti 2015 20:50
Till: David Blaikie
Kopia: reviews+d11940+public+578c1335b27aa...@reviews.llvm.org; Daniel
Marjamäki; cfe-commits@lists.llvm.org
Ämne: Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual
method or method that overrides base class method
On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie dblai...@gmail.com
mailto:dblai...@gmail.com wrote:
On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
Can't you just change your signature to
virtual void a(int /* x */) {}
in these cases?
You could - does it add much value to do that, though?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
(perhaps it does - it means you express the intent that the parameter is
not used and the compiler helps you check that (so that for the parameters
you think /should/ be used (you haven't commented out their name but
accidentally shadow or otherwise fail to reference, you still get a
warning))
- David
On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: krememek.
danielmarjamaki added a subscriber: cfe-commits.
Don't diagnose -Wunused-parameter in methods that override other methods
because the overridden methods might use the parameter
Don't diagnose -Wunused-parameter in virtual methods because these might
be overriden by other methods that use the parameter.
Such diagnostics could be more accurately written if they are based on
whole-program-analysis that establish if such parameter is unused in all
methods.
http://reviews.llvm.org/D11940
Files:
lib/Sema/SemaDecl.cpp
test/SemaCXX/warn-unused-parameters.cpp
Index: test/SemaCXX/warn-unused-parameters.cpp
===
--- test/SemaCXX/warn-unused-parameters.cpp
+++ test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,20 @@
auto l = [t...]() { return sizeof...(s); };
return l();
}
+
+// Don't diagnose virtual methods or methods that override base class
+// methods.
+class Base {
+public:
+ virtual void f(int
Re: [PATCH] D11778: Improvements on Diagnostic in Macro Expansions
rtrieu accepted this revision. rtrieu added a comment. This revision is now accepted and ready to land. This is a first step towards making macro diagnostics better. http://reviews.llvm.org/D11778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method
On Wed, Aug 12, 2015 at 11:12 AM, Nico Weber tha...@chromium.org wrote:
On Wed, Aug 12, 2015 at 11:06 AM, David Blaikie dblai...@gmail.com
wrote:
On Wed, Aug 12, 2015 at 10:11 AM, Nico Weber tha...@chromium.org wrote:
On Tue, Aug 11, 2015 at 10:15 PM, Daniel Marjamäki
daniel.marjam...@evidente.se wrote:
ideally there should be no -Wunused-parameter compiler warning when the
parameter is used.
would it feel better to move the FP warnings about virtual functions,
for instance to clang-tidy?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
imho there are some compiler warnings that are too noisy. I don't like
to get a warning when there is obviously no bug:
sign compare:
if the signed value is obviously not negative then there is no bug:
signed x; .. if (x10 x s.size())
unused parameter:
could check in the current translation unit if the parameter is used
in an overloaded method.
It doesn't warn about the presence of the parameter, but about the
presence of the name. If you say f(int, int) instead of f(int a, int b)
then the warning won't fire.
(And if you don't like this warning, then don't enable it.)
This isn't usually the approach we take with Clang's warnings - we try to
remove false positives (where false positive is usually defined as
diagnoses something which is not a bug (where bug is defined as the
resulting program behaves in a way that the user doesn't intend/expect))
where practical.
Sure, for warnings that are supposed to find bugs. The -Wunused warnings
warn about stuff that's unused, not bugs.
Seems a reasonable analog here, though, would be that a true positive for
-Wunused is when the thing really is unused and should be removed.
Commenting out the variable name is the suppression mechanism to workaround
false positives. If there's a targetable subset of cases where the s/n is
low enough, it could be reasonable to suppress the warning in that subset,
I think.
(see improvements to -Wunreachable-code to suppress cases that are
unreachable in this build (sizeof(int) == 4 conditions, macros, etc), or
represent valid defensive programming (default in a covered enum switch) to
make the diagnostic more useful/less noisy)
If the subset of cases where this warning fires on parameters to virtual
functions produces more noise (if a significant % of cases just result in
commenting out the parameter name rather than removing the parameter) than
signal, it's certainly within the realm of discussion that we have about
whether that subset of cases is worth keeping in the warning.
- David
constructor initialization order:
should not warn if the order obviously does not matter. for instance
initialization order of pod variables using constants.
etc
Best regards,
Daniel Marjamäki
..
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)709 12 42 62
E-mail: daniel.marjam...@evidente.se
www.evidente.se
Från: tha...@google.com [tha...@google.com] f#246;r Nico Weber [
tha...@chromium.org]
Skickat: den 11 augusti 2015 20:50
Till: David Blaikie
Kopia: reviews+d11940+public+578c1335b27aa...@reviews.llvm.org; Daniel
Marjamäki; cfe-commits@lists.llvm.org
Ämne: Re: [PATCH] D11940: don't diagnose -Wunused-parameter in virtual
method or method that overrides base class method
On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie dblai...@gmail.com
mailto:dblai...@gmail.com wrote:
On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
Can't you just change your signature to
virtual void a(int /* x */) {}
in these cases?
You could - does it add much value to do that, though?
If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?
(perhaps it does - it means you express the intent that the parameter
is not used and the compiler helps you check that (so that for the
parameters you think /should/ be used (you haven't commented out their name
but accidentally shadow or otherwise fail to reference, you still get a
warning))
- David
On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki
cfe-commits@lists.llvm.orgmailto:cfe-commits@lists.llvm.org wrote:
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: krememek.
danielmarjamaki added a subscriber: cfe-commits.
Don't diagnose -Wunused-parameter in methods that override other
methods because the overridden methods might use the parameter
Don't diagnose -Wunused-parameter in virtual methods because these
might be overriden by other methods that use
r244792 - RangRangify some more for loops; NFC.
Author: aaronballman
Date: Wed Aug 12 14:00:39 2015
New Revision: 244792
URL: http://llvm.org/viewvc/llvm-project?rev=244792view=rev
Log:
RangRangify some more for loops; NFC.
Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244792r1=244791r2=244792view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Aug 12 14:00:39 2015
@@ -358,11 +358,8 @@ RegistryMaps::RegistryMaps() {
}
RegistryMaps::~RegistryMaps() {
- for (ConstructorMap::iterator it = Constructors.begin(),
-end = Constructors.end();
- it != end; ++it) {
-delete it-second;
- }
+ for (auto E : Constructors)
+delete E.getValue();
}
static llvm::ManagedStaticRegistryMaps RegistryData;
@@ -433,12 +430,13 @@ Registry::getMatcherCompletions(ArrayRef
std::vectorMatcherCompletion Completions;
// Search the registry for acceptable matchers.
- for (ConstructorMap::const_iterator I = RegistryData-constructors().begin(),
- E = RegistryData-constructors().end();
- I != E; ++I) {
+ for (const auto M : RegistryData-constructors()) {
+const auto *Matcher = M.getValue();
+StringRef Name = M.getKey();
+
std::setASTNodeKind RetKinds;
-unsigned NumArgs = I-second-isVariadic() ? 1 : I-second-getNumArgs();
-bool IsPolymorphic = I-second-isPolymorphic();
+unsigned NumArgs = Matcher-isVariadic() ? 1 : Matcher-getNumArgs();
+bool IsPolymorphic = Matcher-isPolymorphic();
std::vectorstd::vectorArgKind ArgsKinds(NumArgs);
unsigned MaxSpecificity = 0;
for (const ArgKind Kind : AcceptedTypes) {
@@ -446,13 +444,13 @@ Registry::getMatcherCompletions(ArrayRef
continue;
unsigned Specificity;
ASTNodeKind LeastDerivedKind;
- if (I-second-isConvertibleTo(Kind.getMatcherKind(), Specificity,
- LeastDerivedKind)) {
+ if (Matcher-isConvertibleTo(Kind.getMatcherKind(), Specificity,
+ LeastDerivedKind)) {
if (MaxSpecificity Specificity)
MaxSpecificity = Specificity;
RetKinds.insert(LeastDerivedKind);
for (unsigned Arg = 0; Arg != NumArgs; ++Arg)
- I-second-getArgKinds(Kind.getMatcherKind(), Arg, ArgsKinds[Arg]);
+ Matcher-getArgKinds(Kind.getMatcherKind(), Arg, ArgsKinds[Arg]);
if (IsPolymorphic)
break;
}
@@ -463,9 +461,9 @@ Registry::getMatcherCompletions(ArrayRef
llvm::raw_string_ostream OS(Decl);
if (IsPolymorphic) {
-OS MatcherT I-first() (MatcherT;
+OS MatcherT Name (MatcherT;
} else {
-OS Matcher RetKinds I-first() (;
+OS Matcher RetKinds Name (;
for (const std::vectorArgKind Arg : ArgsKinds) {
if (Arg != ArgsKinds[0])
OS , ;
@@ -488,11 +486,11 @@ Registry::getMatcherCompletions(ArrayRef
}
}
}
- if (I-second-isVariadic())
+ if (Matcher-isVariadic())
OS ...;
OS );
- std::string TypedText = I-first();
+ std::string TypedText = Name;
TypedText += (;
if (ArgsKinds.empty())
TypedText += );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11916: [CONCEPTS] Add diagnostic; invalid tag when concept specified
LGTM!
~Aaron
On Wed, Aug 12, 2015 at 1:40 PM, Nathan Wilson nwilso...@gmail.com wrote:
nwilson updated this revision to Diff 31957.
nwilson added a comment.
Addressing comments which were discussed on the mailing list - Apply the same
text when diagnosing a free standing declaration as suggested by Aaron.
Replace diagnostic identifier err_concept_decl_non_template with
err_concept_wrong_decl_kind as suggested by Hubert and Richard. Fix tests
corresponding to the changes.
http://reviews.llvm.org/D11916
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/SemaCXX/cxx-concept-declaration.cpp
Index: test/SemaCXX/cxx-concept-declaration.cpp
===
--- test/SemaCXX/cxx-concept-declaration.cpp
+++ test/SemaCXX/cxx-concept-declaration.cpp
@@ -23,3 +23,13 @@
templatetypename T
concept bool D6; // expected-error {{variable concept declaration must be
initialized}}
+// Tag
+concept class CC1 {}; // expected-error {{'concept' can only appear on the
definition of a function template or variable template}}
+concept struct CS1 {}; // expected-error {{'concept' can only appear on the
definition of a function template or variable template}}
+concept union CU1 {}; // expected-error {{'concept' can only appear on the
definition of a function template or variable template}}
+concept enum CE1 {}; // expected-error {{'concept' can only appear on the
definition of a function template or variable template}}
+template typename T concept class TCC1 {}; // expected-error {{'concept'
can only appear on the definition of a function template or variable
template}}
+template typename T concept struct TCS1 {}; // expected-error {{'concept'
can only appear on the definition of a function template or variable
template}}
+template typename T concept union TCU1 {}; // expected-error {{'concept'
can only appear on the definition of a function template or variable
template}}
+
+concept bool; // expected-error {{'concept' can only appear on the
definition of a function template or variable template}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3662,6 +3662,14 @@
return TagD;
}
+ if (DS.isConceptSpecified()) {
+// C++ Concepts TS [dcl.spec.concept]p1: A concept definition refers to
+// either a function concept and its definition or a variable concept and
+// its initializer.
+Diag(DS.getConceptSpecLoc(), diag::err_concept_wrong_decl_kind);
+return TagD;
+ }
+
DiagnoseFunctionSpecifiers(DS);
if (DS.isFriendSpecified()) {
@@ -4865,7 +4873,7 @@
// template, declared in namespace scope
if (!TemplateParamLists.size()) {
Diag(D.getDeclSpec().getConceptSpecLoc(),
- diag::err_concept_decl_non_template);
+ diag:: err_concept_wrong_decl_kind);
return nullptr;
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1965,7 +1965,7 @@
use __attribute__((visibility(\hidden\))) attribute instead;
// C++ Concepts TS
-def err_concept_decl_non_template : Error
+def err_concept_wrong_decl_kind : Error
'concept' can only appear on the definition of a function template or
variable template;
def err_concept_decls_may_only_appear_in_namespace_scope : Error
concept declarations may only appear in namespace scope;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244791 - Don't compare getArchName() to tce as a string. NFC.
Author: dougk
Date: Wed Aug 12 13:36:12 2015
New Revision: 244791
URL: http://llvm.org/viewvc/llvm-project?rev=244791view=rev
Log:
Don't compare getArchName() to tce as a string. NFC.
Modified:
cfe/trunk/lib/Driver/Driver.cpp
Modified: cfe/trunk/lib/Driver/Driver.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=244791r1=244790r2=244791view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Aug 12 13:36:12 2015
@@ -325,7 +325,8 @@ static llvm::Triple computeTargetTriple(
}
// Skip further flag support on OSes which don't support '-m32' or '-m64'.
- if (Target.getArchName() == tce || Target.getOS() == llvm::Triple::Minix)
+ if (Target.getArch() == llvm::Triple::tce ||
+ Target.getOS() == llvm::Triple::Minix)
return Target;
// Handle pseudo-target flags '-m64', '-mx32', '-m32' and '-m16'.
@@ -2232,21 +2233,27 @@ const ToolChain Driver::getToolChain(co
default:
// Of these targets, Hexagon is the only one that might have
// an OS of Linux, in which case it got handled above already.
- if (Target.getArchName() == tce)
+ switch (Target.getArch()) {
+ case llvm::Triple::tce:
TC = new toolchains::TCEToolChain(*this, Target, Args);
- else if (Target.getArch() == llvm::Triple::hexagon)
+break;
+ case llvm::Triple::hexagon:
TC = new toolchains::HexagonToolChain(*this, Target, Args);
- else if (Target.getArch() == llvm::Triple::xcore)
+break;
+ case llvm::Triple::xcore:
TC = new toolchains::XCoreToolChain(*this, Target, Args);
- else if (Target.getArch() == llvm::Triple::shave)
+break;
+ case llvm::Triple::shave:
TC = new toolchains::SHAVEToolChain(*this, Target, Args);
- else if (Target.isOSBinFormatELF())
-TC = new toolchains::Generic_ELF(*this, Target, Args);
- else if (Target.isOSBinFormatMachO())
-TC = new toolchains::MachO(*this, Target, Args);
- else
-TC = new toolchains::Generic_GCC(*this, Target, Args);
- break;
+break;
+ default:
+if (Target.isOSBinFormatELF())
+ TC = new toolchains::Generic_ELF(*this, Target, Args);
+else if (Target.isOSBinFormatMachO())
+ TC = new toolchains::MachO(*this, Target, Args);
+else
+ TC = new toolchains::Generic_GCC(*this, Target, Args);
+ }
}
}
return *TC;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Looks fine, go ahead once you're satisfied that the performance is OK for the
`requires excluded` / umbrella dir hack.
Comment at: include/clang/Basic/Module.h:363
@@ +362,3 @@
+ /// This is more efficient than getFullModuleName().
+ bool fullModuleNameEqual(ArrayRefStringRef nameParts) const;
+
`fullModuleNameEquals` (extra 's' at the end) or `fullModuleNameIs` maybe?
Comment at: lib/Basic/Module.cpp:142-147
@@ +141,8 @@
+bool Module::fullModuleNameEqual(ArrayRefStringRef nameParts) const {
+ for (const Module *M = this; M; M = M-Parent) {
+if (nameParts.empty() || M-Name != nameParts.back())
+ return false;
+nameParts = nameParts.drop_back();
+ }
+ return nameParts.empty();
+}
Seems weird to do this by modifying `nameParts` rather than tracking an index,
but OK...
Comment at: lib/Lex/ModuleMap.cpp:1025-1026
@@ +1024,4 @@
+/// non-modular headers. For backwards compatibility, we continue to
+/// support
+/// this idiom for just these modules, and map the headers to 'textual' to
+/// match the original intent.
Unnecessary newline?
Comment at: lib/Lex/ModuleMap.cpp:1607-1608
@@ +1606,4 @@
+M-fullModuleNameEqual(TclPrivate))) {
+ IsRequiresExcludedHack = true;
+ return false;
+ } else if (Feature == cplusplus M-fullModuleNameEqual(IOKitAVC)) {
Overindented.
Repository:
rL LLVM
http://reviews.llvm.org/D11403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11983: Make getToolChain actually cache its result, as documented in Driver.h
chandlerc added a comment. This isn't necessary. The TC that we assign to is a reference to the pointer in the map. It should already be caching. http://reviews.llvm.org/D11983 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11944: Nativize filename in FileManager::getFile().
rsmith added a comment. In principle, normalizing slashes on Windows makes sense here. But we shouldn't use `llvm::sys::path::native`, because it's just too broken. Comment at: lib/Basic/FileManager.cpp:221-222 @@ -220,1 +220,4 @@ + SmallString1024 NativeFilename; + llvm::sys::path::native(Filename, NativeFilename); + I have two concerns with this: 1) It's needlessly inefficient on non-Win32 platforms (we'll make an unnecessary string copy) 2) It does something bizarre and wrong on non-Win32 platforms (it converts `\` to `/` unless the `\` is followed by another `\`, making most files containing `\` inaccessible) http://reviews.llvm.org/D11944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11778: Improvements on Diagnostic in Macro Expansions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL244788: Stop printing macro backtraces that don't help
diagnostics. (authored by rtrieu).
Changed prior to commit:
http://reviews.llvm.org/D11778?vs=31425id=31965#toc
Repository:
rL LLVM
http://reviews.llvm.org/D11778
Files:
cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
cfe/trunk/test/Misc/diag-presumed.c
cfe/trunk/test/Misc/reduced-diags-macros-backtrace.cpp
cfe/trunk/test/Misc/reduced-diags-macros.cpp
cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
Index: cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
===
--- cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
+++ cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
@@ -423,6 +423,38 @@
SpellingRanges, None, SM);
}
+static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
+ const SourceManager SM) {
+ SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
+ while (BegLoc != EndLoc) {
+if (!SM.isMacroArgExpansion(BegLoc))
+ return false;
+BegLoc.getLocWithOffset(1);
+ }
+
+ return SM.isMacroArgExpansion(BegLoc);
+}
+
+/// A helper function to check if the current ranges are all inside
+/// the macro expansions.
+static bool checkRangesForMacroArgExpansion(SourceLocation Loc,
+ArrayRefCharSourceRange Ranges,
+const SourceManager SM) {
+ assert(Loc.isMacroID() Must be a macro expansion!);
+
+ SmallVectorCharSourceRange, 4 SpellingRanges;
+ mapDiagnosticRanges(Loc, Ranges, SpellingRanges, SM);
+
+ if (!SM.isMacroArgExpansion(Loc))
+return false;
+
+ for (auto I = SpellingRanges.begin(), E = SpellingRanges.end(); I != E; ++I)
+if (!checkRangeForMacroArgExpansion(*I, SM))
+ return false;
+
+ return true;
+}
+
/// \brief Recursively emit notes for each macro expansion and caret
/// diagnostics where appropriate.
///
@@ -443,12 +475,19 @@
// Produce a stack of macro backtraces.
SmallVectorSourceLocation, 8 LocationStack;
+ unsigned IgnoredEnd = 0;
while (Loc.isMacroID()) {
LocationStack.push_back(Loc);
+if (checkRangesForMacroArgExpansion(Loc, Ranges, SM))
+ IgnoredEnd = LocationStack.size();
+
Loc = SM.getImmediateMacroCallerLoc(Loc);
assert(!Loc.isInvalid() must have a valid source location here);
}
+ LocationStack.erase(LocationStack.begin(),
+ LocationStack.begin() + IgnoredEnd);
+
unsigned MacroDepth = LocationStack.size();
unsigned MacroLimit = DiagOpts-MacroBacktraceLimit;
if (MacroDepth = MacroLimit || MacroLimit == 0) {
Index: cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
===
--- cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
+++ cfe/trunk/test/Preprocessor/macro_arg_slocentry_merge.c
@@ -3,5 +3,3 @@
#include macro_arg_slocentry_merge.h
// CHECK: macro_arg_slocentry_merge.h:7:19: error: unknown type name 'win'
-// CHECK: macro_arg_slocentry_merge.h:5:16: note: expanded from macro 'WINDOW'
-// CHECK: macro_arg_slocentry_merge.h:6:18: note: expanded from macro 'P_'
Index: cfe/trunk/test/Misc/reduced-diags-macros-backtrace.cpp
===
--- cfe/trunk/test/Misc/reduced-diags-macros-backtrace.cpp
+++ cfe/trunk/test/Misc/reduced-diags-macros-backtrace.cpp
@@ -0,0 +1,47 @@
+// RUN: not %clang_cc1 -fsyntax-only -fmacro-backtrace-limit 0 %s 21 | FileCheck %s --check-prefix=ALL
+// RUN: not %clang_cc1 -fsyntax-only -fmacro-backtrace-limit 2 %s 21 | FileCheck %s --check-prefix=SKIP
+
+#define F(x) x + 1
+#define G(x) F(x) + 2
+#define ADD(x,y) G(x) + y
+#define LEVEL4(x) ADD(p,x)
+#define LEVEL3(x) LEVEL4(x)
+#define LEVEL2(x) LEVEL3(x)
+#define LEVEL1(x) LEVEL2(x)
+
+int a = LEVEL1(b);
+
+// ALL: {{.*}}:12:9: error: use of undeclared identifier 'p'
+// ALL-NEXT: int a = LEVEL1(b);
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:10:19: note: expanded from macro 'LEVEL1'
+// ALL-NEXT: #define LEVEL1(x) LEVEL2(x)
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:9:19: note: expanded from macro 'LEVEL2'
+// ALL-NEXT: #define LEVEL2(x) LEVEL3(x)
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:8:19: note: expanded from macro 'LEVEL3'
+// ALL-NEXT: #define LEVEL3(x) LEVEL4(x)
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:7:23: note: expanded from macro 'LEVEL4'
+// ALL-NEXT: #define LEVEL4(x) ADD(p,x)
+// ALL-NEXT: ^
+// ALL-NEXT: {{.*}}:12:16: error: use of undeclared identifier 'b'
+// ALL-NEXT: int a = LEVEL1(b);
+// ALL-NEXT:^
+// ALL-NEXT: 2 errors generated.
+
+// SKIP: {{.*}}:12:9: error: use of undeclared identifier 'p'
+// SKIP-NEXT: int a = LEVEL1(b);
+// SKIP-NEXT: ^
+// SKIP-NEXT: {{.*}}:10:19: note:
[clang-tools-extra] r244793 - [clang-tidy] Make FileOptionsProvider fields protected to make extending it easier
Author: alexfh Date: Wed Aug 12 14:29:57 2015 New Revision: 244793 URL: http://llvm.org/viewvc/llvm-project?rev=244793view=rev Log: [clang-tidy] Make FileOptionsProvider fields protected to make extending it easier Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h?rev=244793r1=244792r2=244793view=diff == --- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h Wed Aug 12 14:29:57 2015 @@ -189,7 +189,7 @@ public: ClangTidyOptions getOptions(llvm::StringRef FileName) override; -private: +protected: /// \brief Try to read configuration files from \p Directory using registered /// \c ConfigHandlers. llvm::OptionalClangTidyOptions TryReadConfigFile(llvm::StringRef Directory); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] Fix types of size_t, intptr_t, and ptrdiff_t on OpenBSD/sparc.
On 08/12/15 11:24, James Y Knight wrote: Sorry for the miscommunication, I thought you were going to commit it already. LGTM. Oh, I was waiting for someone to say something. Thanks. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244792 - RangRangify some more for loops; NFC.
On Wed, Aug 12, 2015 at 12:00 PM, Aaron Ballman via cfe-commits
cfe-commits@lists.llvm.org wrote:
Author: aaronballman
Date: Wed Aug 12 14:00:39 2015
New Revision: 244792
URL: http://llvm.org/viewvc/llvm-project?rev=244792view=rev
Log:
RangRangify some more for loops; NFC.
Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244792r1=244791r2=244792view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Aug 12 14:00:39 2015
@@ -358,11 +358,8 @@ RegistryMaps::RegistryMaps() {
}
RegistryMaps::~RegistryMaps() {
- for (ConstructorMap::iterator it = Constructors.begin(),
-end = Constructors.end();
- it != end; ++it) {
-delete it-second;
- }
+ for (auto E : Constructors)
+delete E.getValue();
This looks like DeleteContainerSeconds (but even better if they could be
switched to unique_ptr)
}
static llvm::ManagedStaticRegistryMaps RegistryData;
@@ -433,12 +430,13 @@ Registry::getMatcherCompletions(ArrayRef
std::vectorMatcherCompletion Completions;
// Search the registry for acceptable matchers.
- for (ConstructorMap::const_iterator I =
RegistryData-constructors().begin(),
- E =
RegistryData-constructors().end();
- I != E; ++I) {
+ for (const auto M : RegistryData-constructors()) {
+const auto *Matcher = M.getValue();
+StringRef Name = M.getKey();
+
std::setASTNodeKind RetKinds;
-unsigned NumArgs = I-second-isVariadic() ? 1 :
I-second-getNumArgs();
-bool IsPolymorphic = I-second-isPolymorphic();
+unsigned NumArgs = Matcher-isVariadic() ? 1 : Matcher-getNumArgs();
+bool IsPolymorphic = Matcher-isPolymorphic();
std::vectorstd::vectorArgKind ArgsKinds(NumArgs);
unsigned MaxSpecificity = 0;
for (const ArgKind Kind : AcceptedTypes) {
@@ -446,13 +444,13 @@ Registry::getMatcherCompletions(ArrayRef
continue;
unsigned Specificity;
ASTNodeKind LeastDerivedKind;
- if (I-second-isConvertibleTo(Kind.getMatcherKind(), Specificity,
- LeastDerivedKind)) {
+ if (Matcher-isConvertibleTo(Kind.getMatcherKind(), Specificity,
+ LeastDerivedKind)) {
if (MaxSpecificity Specificity)
MaxSpecificity = Specificity;
RetKinds.insert(LeastDerivedKind);
for (unsigned Arg = 0; Arg != NumArgs; ++Arg)
- I-second-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
+ Matcher-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
if (IsPolymorphic)
break;
}
@@ -463,9 +461,9 @@ Registry::getMatcherCompletions(ArrayRef
llvm::raw_string_ostream OS(Decl);
if (IsPolymorphic) {
-OS MatcherT I-first() (MatcherT;
+OS MatcherT Name (MatcherT;
} else {
-OS Matcher RetKinds I-first() (;
+OS Matcher RetKinds Name (;
for (const std::vectorArgKind Arg : ArgsKinds) {
if (Arg != ArgsKinds[0])
OS , ;
@@ -488,11 +486,11 @@ Registry::getMatcherCompletions(ArrayRef
}
}
}
- if (I-second-isVariadic())
+ if (Matcher-isVariadic())
OS ...;
OS );
- std::string TypedText = I-first();
+ std::string TypedText = Name;
TypedText += (;
if (ArgsKinds.empty())
TypedText += );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244797 - Docs: keep copyright years up-to-date
Author: hans Date: Wed Aug 12 14:45:01 2015 New Revision: 244797 URL: http://llvm.org/viewvc/llvm-project?rev=244797view=rev Log: Docs: keep copyright years up-to-date Modified: cfe/trunk/docs/conf.py Modified: cfe/trunk/docs/conf.py URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/conf.py?rev=244797r1=244796r2=244797view=diff == --- cfe/trunk/docs/conf.py (original) +++ cfe/trunk/docs/conf.py Wed Aug 12 14:45:01 2015 @@ -12,6 +12,7 @@ # serve to show the default. import sys, os +from datetime import date # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the @@ -41,7 +42,7 @@ master_doc = 'index' # General information about the project. project = u'Clang' -copyright = u'2007-2015, The Clang Team' +copyright = u'2007-%d, The Clang Team' % date.today().year # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244792 - RangRangify some more for loops; NFC.
On Wed, Aug 12, 2015 at 3:37 PM, David Blaikie dblai...@gmail.com wrote:
On Wed, Aug 12, 2015 at 12:00 PM, Aaron Ballman via cfe-commits
cfe-commits@lists.llvm.org wrote:
Author: aaronballman
Date: Wed Aug 12 14:00:39 2015
New Revision: 244792
URL: http://llvm.org/viewvc/llvm-project?rev=244792view=rev
Log:
RangRangify some more for loops; NFC.
Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244792r1=244791r2=244792view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Aug 12 14:00:39
2015
@@ -358,11 +358,8 @@ RegistryMaps::RegistryMaps() {
}
RegistryMaps::~RegistryMaps() {
- for (ConstructorMap::iterator it = Constructors.begin(),
-end = Constructors.end();
- it != end; ++it) {
-delete it-second;
- }
+ for (auto E : Constructors)
+delete E.getValue();
This looks like DeleteContainerSeconds (but even better if they could be
switched to unique_ptr)
Ooh, I will switch to that. I tried to unique_ptr'ify the code, but
it's... complicated.
~Aaron
}
static llvm::ManagedStaticRegistryMaps RegistryData;
@@ -433,12 +430,13 @@ Registry::getMatcherCompletions(ArrayRef
std::vectorMatcherCompletion Completions;
// Search the registry for acceptable matchers.
- for (ConstructorMap::const_iterator I =
RegistryData-constructors().begin(),
- E =
RegistryData-constructors().end();
- I != E; ++I) {
+ for (const auto M : RegistryData-constructors()) {
+const auto *Matcher = M.getValue();
+StringRef Name = M.getKey();
+
std::setASTNodeKind RetKinds;
-unsigned NumArgs = I-second-isVariadic() ? 1 :
I-second-getNumArgs();
-bool IsPolymorphic = I-second-isPolymorphic();
+unsigned NumArgs = Matcher-isVariadic() ? 1 : Matcher-getNumArgs();
+bool IsPolymorphic = Matcher-isPolymorphic();
std::vectorstd::vectorArgKind ArgsKinds(NumArgs);
unsigned MaxSpecificity = 0;
for (const ArgKind Kind : AcceptedTypes) {
@@ -446,13 +444,13 @@ Registry::getMatcherCompletions(ArrayRef
continue;
unsigned Specificity;
ASTNodeKind LeastDerivedKind;
- if (I-second-isConvertibleTo(Kind.getMatcherKind(), Specificity,
- LeastDerivedKind)) {
+ if (Matcher-isConvertibleTo(Kind.getMatcherKind(), Specificity,
+ LeastDerivedKind)) {
if (MaxSpecificity Specificity)
MaxSpecificity = Specificity;
RetKinds.insert(LeastDerivedKind);
for (unsigned Arg = 0; Arg != NumArgs; ++Arg)
- I-second-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
+ Matcher-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
if (IsPolymorphic)
break;
}
@@ -463,9 +461,9 @@ Registry::getMatcherCompletions(ArrayRef
llvm::raw_string_ostream OS(Decl);
if (IsPolymorphic) {
-OS MatcherT I-first() (MatcherT;
+OS MatcherT Name (MatcherT;
} else {
-OS Matcher RetKinds I-first() (;
+OS Matcher RetKinds Name (;
for (const std::vectorArgKind Arg : ArgsKinds) {
if (Arg != ArgsKinds[0])
OS , ;
@@ -488,11 +486,11 @@ Registry::getMatcherCompletions(ArrayRef
}
}
}
- if (I-second-isVariadic())
+ if (Matcher-isVariadic())
OS ...;
OS );
- std::string TypedText = I-first();
+ std::string TypedText = Name;
TypedText += (;
if (ArgsKinds.empty())
TypedText += );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244792 - RangRangify some more for loops; NFC.
On Wed, Aug 12, 2015 at 1:03 PM, Aaron Ballman aa...@aaronballman.com
wrote:
On Wed, Aug 12, 2015 at 3:37 PM, David Blaikie dblai...@gmail.com wrote:
On Wed, Aug 12, 2015 at 12:00 PM, Aaron Ballman via cfe-commits
cfe-commits@lists.llvm.org wrote:
Author: aaronballman
Date: Wed Aug 12 14:00:39 2015
New Revision: 244792
URL: http://llvm.org/viewvc/llvm-project?rev=244792view=rev
Log:
RangRangify some more for loops; NFC.
Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=244792r1=244791r2=244792view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Aug 12 14:00:39
2015
@@ -358,11 +358,8 @@ RegistryMaps::RegistryMaps() {
}
RegistryMaps::~RegistryMaps() {
- for (ConstructorMap::iterator it = Constructors.begin(),
-end = Constructors.end();
- it != end; ++it) {
-delete it-second;
- }
+ for (auto E : Constructors)
+delete E.getValue();
This looks like DeleteContainerSeconds (but even better if they could
be
switched to unique_ptr)
Ooh, I will switch to that. I tried to unique_ptr'ify the code, but
it's... complicated.
:(
~Aaron
}
static llvm::ManagedStaticRegistryMaps RegistryData;
@@ -433,12 +430,13 @@ Registry::getMatcherCompletions(ArrayRef
std::vectorMatcherCompletion Completions;
// Search the registry for acceptable matchers.
- for (ConstructorMap::const_iterator I =
RegistryData-constructors().begin(),
- E =
RegistryData-constructors().end();
- I != E; ++I) {
+ for (const auto M : RegistryData-constructors()) {
+const auto *Matcher = M.getValue();
+StringRef Name = M.getKey();
+
std::setASTNodeKind RetKinds;
-unsigned NumArgs = I-second-isVariadic() ? 1 :
I-second-getNumArgs();
-bool IsPolymorphic = I-second-isPolymorphic();
+unsigned NumArgs = Matcher-isVariadic() ? 1 :
Matcher-getNumArgs();
+bool IsPolymorphic = Matcher-isPolymorphic();
std::vectorstd::vectorArgKind ArgsKinds(NumArgs);
unsigned MaxSpecificity = 0;
for (const ArgKind Kind : AcceptedTypes) {
@@ -446,13 +444,13 @@ Registry::getMatcherCompletions(ArrayRef
continue;
unsigned Specificity;
ASTNodeKind LeastDerivedKind;
- if (I-second-isConvertibleTo(Kind.getMatcherKind(),
Specificity,
- LeastDerivedKind)) {
+ if (Matcher-isConvertibleTo(Kind.getMatcherKind(), Specificity,
+ LeastDerivedKind)) {
if (MaxSpecificity Specificity)
MaxSpecificity = Specificity;
RetKinds.insert(LeastDerivedKind);
for (unsigned Arg = 0; Arg != NumArgs; ++Arg)
- I-second-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
+ Matcher-getArgKinds(Kind.getMatcherKind(), Arg,
ArgsKinds[Arg]);
if (IsPolymorphic)
break;
}
@@ -463,9 +461,9 @@ Registry::getMatcherCompletions(ArrayRef
llvm::raw_string_ostream OS(Decl);
if (IsPolymorphic) {
-OS MatcherT I-first() (MatcherT;
+OS MatcherT Name (MatcherT;
} else {
-OS Matcher RetKinds I-first() (;
+OS Matcher RetKinds Name (;
for (const std::vectorArgKind Arg : ArgsKinds) {
if (Arg != ArgsKinds[0])
OS , ;
@@ -488,11 +486,11 @@ Registry::getMatcherCompletions(ArrayRef
}
}
}
- if (I-second-isVariadic())
+ if (Matcher-isVariadic())
OS ...;
OS );
- std::string TypedText = I-first();
+ std::string TypedText = Name;
TypedText += (;
if (ArgsKinds.empty())
TypedText += );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
