[PATCH] D30733: [Driver] Add arch-specific rpath for libc++
Hahnfeld marked an inline comment as done. Hahnfeld added a comment. In https://reviews.llvm.org/D30733#697108, @jroelofs wrote: > As I said on https://reviews.llvm.org/D30214, it is inappropriate to be > installing libc++ in the resource directory... please **do not** do that. Can you give reason for that? I can understand that header files are independent of the target architecture but how do you handle multiple binary libraries for let's say 32 and 64 bit? This was the main motivation for the OpenMP runtime in http://lists.llvm.org/pipermail/openmp-dev/2016-December/001612.html, please also see https://bugs.llvm.org//show_bug.cgi?id=31300. I don't think `libc++` would be any different here. Additionally, my main motiviation was for `libunwind` as explained here: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html On that thread multiple people suggested to use an extra directory for runtime libraries, @rnk and @mgorny listed as reviewers for this patch. If `libunwind` goes there and is added together with `libc++`, we need to add the rpath here. And what's the reason against installing `libc++` in the same path then? Comment at: lib/Driver/ToolChain.cpp:652 +// libc++ may be installed per arch. +addArchSpecificRPath(*this, Args, CmdArgs); break; pirama wrote: > `addArchSpecificRPath` is a static function in Tools.cpp and isn't visible > here. No, it's not since recent refactoring. I do compile test my changes usually ;-) https://reviews.llvm.org/D30733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30810: Preserve vec3 type.
jaykang10 created this revision.
Preserve vec3 type with CodeGen option.
https://reviews.llvm.org/D30810
Files:
include/clang/Driver/CC1Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGExpr.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenOpenCL/preserve_vec3.cl
Index: test/CodeGenOpenCL/preserve_vec3.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/preserve_vec3.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown -preserve-vec3-type | FileCheck %s
+
+typedef float float3 __attribute__((ext_vector_type(3)));
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+void kernel foo(global float3 *a, global float3 *b) {
+ // CHECK: %[[LOAD_A:.*]] = load <3 x float>, <3 x float> addrspace(1)* %a
+ // CHECK: store <3 x float> %[[LOAD_A]], <3 x float> addrspace(1)* %b
+ *b = *a;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -713,6 +713,7 @@
}
}
+ Opts.PreserveVec3Type = Args.hasArg(OPT_preserve_vec3_type);
Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
Opts.XRayInstructionThreshold =
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1368,26 +1368,28 @@
QualType TBAABaseType,
uint64_t TBAAOffset,
bool isNontemporal) {
- // For better performance, handle vector loads differently.
- if (Ty->isVectorType()) {
-const llvm::Type *EltTy = Addr.getElementType();
-
-const auto *VTy = cast(EltTy);
-
-// Handle vectors of size 3 like size 4 for better performance.
-if (VTy->getNumElements() == 3) {
-
- // Bitcast to vec4 type.
- llvm::VectorType *vec4Ty = llvm::VectorType::get(VTy->getElementType(),
- 4);
- Address Cast = Builder.CreateElementBitCast(Addr, vec4Ty, "castToVec4");
- // Now load value.
- llvm::Value *V = Builder.CreateLoad(Cast, Volatile, "loadVec4");
-
- // Shuffle vector to get vec3.
- V = Builder.CreateShuffleVector(V, llvm::UndefValue::get(vec4Ty),
- {0, 1, 2}, "extractVec");
- return EmitFromMemory(V, Ty);
+ if (!CGM.getCodeGenOpts().PreserveVec3Type) {
+// For better performance, handle vector loads differently.
+if (Ty->isVectorType()) {
+ const llvm::Type *EltTy = Addr.getElementType();
+
+ const auto *VTy = cast(EltTy);
+
+ // Handle vectors of size 3 like size 4 for better performance.
+ if (VTy->getNumElements() == 3) {
+
+// Bitcast to vec4 type.
+llvm::VectorType *vec4Ty =
+llvm::VectorType::get(VTy->getElementType(), 4);
+Address Cast = Builder.CreateElementBitCast(Addr, vec4Ty, "castToVec4");
+// Now load value.
+llvm::Value *V = Builder.CreateLoad(Cast, Volatile, "loadVec4");
+
+// Shuffle vector to get vec3.
+V = Builder.CreateShuffleVector(V, llvm::UndefValue::get(vec4Ty),
+{0, 1, 2}, "extractVec");
+return EmitFromMemory(V, Ty);
+ }
}
}
@@ -1456,23 +1458,24 @@
bool isNontemporal) {
// Handle vectors differently to get better performance.
- if (Ty->isVectorType()) {
-llvm::Type *SrcTy = Value->getType();
-auto *VecTy = cast(SrcTy);
-// Handle vec3 special.
-if (VecTy->getNumElements() == 3) {
- // Our source is a vec3, do a shuffle vector to make it a vec4.
- llvm::Constant *Mask[] = {Builder.getInt32(0), Builder.getInt32(1),
-Builder.getInt32(2),
-llvm::UndefValue::get(Builder.getInt32Ty())};
- llvm::Value *MaskV = llvm::ConstantVector::get(Mask);
- Value = Builder.CreateShuffleVector(Value,
- llvm::UndefValue::get(VecTy),
- MaskV, "extractVec");
- SrcTy = llvm::VectorType::get(VecTy->getElementType(), 4);
-}
-if (Addr.getElementType() != SrcTy) {
- Addr = Builder.CreateElementBitCast(Addr, SrcTy, "storetmp");
+ if (!CGM.getCodeGenOpts().PreserveVec3Type) {
+if (Ty->isVectorType()) {
+ llvm::Type *SrcTy = Value->getType();
+ auto *VecTy = cast(SrcTy);
+ // Handle vec3 special.
+ if (VecTy->getNumElements() == 3) {
+// Our source is a vec3, do a shuffle vector to make it a vec4.
+llvm::Constant *Mask[] = {Builder.getInt32(0), Builder.getInt32(1),
+
LLVM buildmaster will be restarted in the nearest hour
Hello everyone, LLVM buildmaster will be updated and restarted in the nearest hour. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30809: [coroutines] Add codegen for await and yield expressions
GorNishanov created this revision.
Herald added a subscriber: mehdi_amini.
Emit suspend expression which roughly looks like:
auto && x = CommonExpr();
if (!x.await_ready()) {
llvm_coro_save();
x.await_suspend(...); (*)
llvm_coro_suspend(); (**)
}
x.await_resume();
where the result of the entire expression is the result of x.await_resume()
(*) If x.await_suspend return type is bool, it allows to veto a suspend:
if (x.await_suspend(...))
llvm_coro_suspend();
(**) llvm_coro_suspend() encodes three possible continuations as a switch
instruction:
%where-to = call i8 @llvm.coro.suspend(...)
switch i8 %where-to, label %coro.ret [ ; jump to epilogue to suspend
i8 0, label %yield.ready ; go here when resumed
i8 1, label %yield.cleanup ; go here when destroyed
]
https://reviews.llvm.org/D30809
Files:
lib/CodeGen/CGCoroutine.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCoroutines/coro-await.cpp
Index: test/CodeGenCoroutines/coro-await.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+
+namespace std {
+namespace experimental {
+template
+struct coroutine_traits;
+
+template struct coroutine_handle;
+
+template <>
+struct coroutine_handle {
+ void *ptr;
+ static coroutine_handle from_address(void *);
+};
+
+template
+struct coroutine_handle : coroutine_handle<> {
+ static coroutine_handle from_address(void *);
+};
+
+}
+}
+
+struct suspend_always {
+ int stuff;
+ bool await_ready();
+ void await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+ };
+};
+
+// CHECK-LABEL: f0(
+extern "C" void f0() {
+
+ co_await suspend_always{};
+ // See if we need to suspend:
+ // --
+ // CHECK: %[[READY:.+]] = call zeroext i1 @_ZN14suspend_always11await_readyEv(%struct.suspend_always* %[[AWAITABLE:.+]])
+ // CHECK: br i1 %[[READY]], label %[[READY_BB:.+]], label %[[SUSPEND_BB:.+]]
+
+ // If we are suspending:
+ // -
+ // CHECK: [[SUSPEND_BB]]:
+ // CHECK: %[[SUSPEND_ID:.+]] = call token @llvm.coro.save(
+ // ---
+ // Build the coroutine handle and pass it to await_suspend
+ // ---
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
+ // CHECK: call i8* @_ZNSt12experimental16coroutine_handleINS_16coroutine_traitsIJvEE12promise_typeEE12from_addressEPv(i8* %[[FRAME]])
+ // ... many lines of code to coerce coroutine_handle into an i8* scalar
+ // CHECK: %[[CH:.+]] = load i8*, i8** %{{.+}}
+ // CHECK: call void @_ZN14suspend_always13await_suspendENSt12experimental16coroutine_handleIvEE(%struct.suspend_always* %[[AWAITABLE]], i8* %[[CH]])
+ // -
+ // Generate a suspend point:
+ // -
+ // CHECK: %[[OUTCOME:.+]] = call i8 @llvm.coro.suspend(token %[[SUSPEND_ID]], i1 false)
+ // CHECK: switch i8 %[[OUTCOME]], label %[[RET_BB:.+]] [
+ // CHECK: i8 0, label %[[READY_BB]]
+ // CHECK: i8 1, label %[[CLEANUP_BB:.+]]
+ // CHECK: ]
+
+ // Cleanup code goes here:
+ // ---
+ // CHECK: [[CLEANUP_BB]]:
+
+ // When coroutine is resumed, call await_resume
+ // --
+ // CHECK: [[READY_BB]]:
+ // CHECK: call void @_ZN14suspend_always12await_resumeEv(%struct.suspend_always* %[[AWAITABLE]])
+}
+
+struct suspend_maybe {
+ float stuff;
+ ~suspend_maybe();
+ bool await_ready();
+ bool await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+
+
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+suspend_maybe yield_value(int);
+ };
+};
+
+// CHECK-LABEL: f1(
+extern "C" void f1(int) {
+ co_yield 42;
+ // CHECK: %[[PROMISE:.+]] = alloca %"struct.std::experimental::coroutine_traits::promise_type"
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJviEE12promise_type11yield_valueEi(%struct.suspend_maybe* sret %[[AWAITER:.+]], %"struct.std::experimental::coroutine_traits::promise_type"* %[[PROMISE]], i32 42)
+
+ // See if we need to suspend:
+ // --
+ // CHECK: %[[READY:.+]] = call zeroext i1 @_ZN13suspend_maybe11await_readyEv(%struct.suspend_maybe* %[[AWAITABLE]])
+ // CHECK: br i1 %[[READY]], label %[[READY_BB:.+]], label %[[SUSPEND_BB:.+]]
+
+ // If we are suspending:
+ // -
+
[PATCH] D30762: [ubsan] Add a nullability sanitizer
zaks.anna added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:101 + ``-fsanitize=nullability-assign``, and the argument check with + ``-fsanitize=nullability-arg``. While violating nullability rules does + not result in undefined behavior, it is often unintentional, so UBSan vsk wrote: > zaks.anna wrote: > > Have you investigated if -fsanitize=nullability-arg is too noisy? I think > > we should somehow "recommend" return and assignment check to most users. > > This is not clear from the help and options provided. The main concern here > > is that someone would try -fsanitize=nullability, see a ton of warnings > > they don't care about and think that the whole check is too noisy. > > > > I cannot come up with a great solution. Options that come into mind are: > > - Drop "arg" check completely. > > - Remove arg from the nullability group. > > - Have 2 groups for nullability. > > > > We can also wait for concrete data before making a decision here and > > address this in subsequent commits. > The nullability-arg check was not noisy on a small sample (n=4) of > Apple-internal projects. I only collected 11 diagnostics in total, all of > which seemed actionable. I.e the issues were not isolated to system headers: > the project code actually did violate arg nullability. Based on what I've > seen so far, I'd like to start off with including the arg check in a > "nullability-full" group. If arg check noisiness becomes an issue, then I'd > like to create a new "nullability-partial" group with just the > return/assignment checks. > > TL;DR: let's rename -fsanitize=nullability to -fsanitize=nullability-full, > keep the arg check, and introduce a nullability-partial group later if we > need to. Wdyt? > > Keeping things as is and introducing a new group if this becomes a problem sounds good to me. You could use "nullability" and "nullability-pedantic". That way you do not need to change the name and get to use "nullability". https://reviews.llvm.org/D30762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30776: [coroutines] Fix diagnostics depending on the first coroutine statement.
GorNishanov added a comment. In https://reviews.llvm.org/D30776#697233, @EricWF wrote: > Good to know. I'll update this tomorrow. Well, that is just the thought. Possibly we can check for the types of await_ready and await_suspend in BuildResolvedCoawaitExpr. Then, we don't have to keep the vector of await/yield-expressions. https://reviews.llvm.org/D30776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c
chandlerc created this revision.
Herald added a subscriber: mcrosier.
This follows the design direction discussed on cfe-dev here:
http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066.html
The idea is that for C standard library builtins, even if the library
vendor chooses to annotate their routines with __attribute__((nonnull)),
we will ignore those attributes which pertain to pointer arguments that
have an associated size. This allows the widespread (and seemingly
reasonable) pattern of calling these routines with a null pointer and
a zero size. I have only done this for the library builtins currently
recognized by Clang, but we can now trivially add to this set. This will
be controllable with -fno-builtin if anyone should care to do so.
Note that this does *not* change the AST. As a consequence, warnings,
static analysis, and source code rewriting are not impacted.
This isn't even a regression on any platform as neither Clang nor LLVM
have ever put 'nonnull' onto these arguments for declarations. All this
patch does is enable it on other declarations while preventing us from
ever accidentally enabling it on these libc functions due to a library
vendor.
https://reviews.llvm.org/D30806
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/Builtins.def
lib/AST/ASTContext.cpp
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CGCall.cpp
lib/Sema/SemaChecking.cpp
test/CodeGen/nonnull.c
Index: test/CodeGen/nonnull.c
===
--- test/CodeGen/nonnull.c
+++ test/CodeGen/nonnull.c
@@ -49,3 +49,87 @@
// CHECK: define void @bar8(i32* nonnull %a, i32* nonnull %b)
void bar8(int *a, int *b) __attribute__((nonnull))
__attribute__((nonnull(1))) {}
+
+// CHECK: declare void @foo_decl(i32* nonnull)
+void foo_decl(int *__attribute__((nonnull)));
+
+// CHECK: declare void @bar_decl(i32* nonnull)
+void bar_decl(int *) __attribute__((nonnull(1)));
+
+// CHECK: declare void @bar2_decl(i32*, i32* nonnull)
+void bar2_decl(int *, int *) __attribute__((nonnull(2)));
+
+// CHECK: declare nonnull i32* @bar3_decl()
+int *bar3_decl(void) __attribute__((returns_nonnull));
+
+// CHECK: declare i32 @bar4_decl(i32, i32* nonnull)
+int bar4_decl(int, int *) __attribute__((nonnull));
+
+// CHECK: declare i32 @bar5_decl(i32, i32* nonnull)
+int bar5_decl(int, int *) __attribute__((nonnull(1, 2)));
+
+// CHECK: declare i32 @bar6_decl(i64)
+int bar6_decl(TransparentUnion) __attribute__((nonnull(1)));
+
+// CHECK: declare void @bar7_decl(i32* nonnull, i32* nonnull)
+void bar7_decl(int *, int *)
+__attribute__((nonnull(1))) __attribute__((nonnull(2)));
+
+// CHECK: declare void @bar8_decl(i32* nonnull, i32* nonnull)
+void bar8_decl(int *, int *)
+__attribute__((nonnull)) __attribute__((nonnull(1)));
+
+// Clang specially disables nonnull attributes on some library builtin
+// functions to work around the fact that the standard and some vendors mark
+// them as nonnull even though they are frequently called in practice with null
+// arguments if a corresponding size argument is zero.
+
+// CHECK: declare i8* @memcpy(i8*, i8*, i64)
+void *memcpy(void *, const void *, unsigned long)
+__attribute__((nonnull(1, 2))) __attribute__((returns_nonnull));
+
+// CHECK: declare i32 @memcmp(i8*, i8*, i64)
+int memcmp(const void *, const void *, unsigned long) __attribute__((nonnull(1, 2)));
+
+// CHECK: declare i8* @memmove(i8*, i8*, i64)
+void *memmove(void *, const void *, unsigned long)
+__attribute__((nonnull(1, 2))) __attribute__((returns_nonnull));
+
+// CHECK: declare i8* @strncpy(i8*, i8*, i64)
+char *strncpy(char *, const char *, unsigned long)
+__attribute__((nonnull(1, 2))) __attribute__((returns_nonnull));
+
+// CHECK: declare i32 @strncmp(i8*, i8*, i64)
+int strncmp(const char *, const char *, unsigned long) __attribute__((nonnull(1, 2)));
+
+// CHECK: declare nonnull i8* @strncat(i8* nonnull, i8*, i64)
+char *strncat(char *, const char *, unsigned long)
+__attribute__((nonnull(1, 2))) __attribute__((returns_nonnull));
+
+// CHECK: declare i8* @memchr(i8*, i32, i64)
+void *memchr(const void *__attribute__((nonnull)), int, unsigned long)
+__attribute__((returns_nonnull));
+
+// CHECK: declare i8* @memset(i8*, i32, i64)
+void *memset(void *__attribute__((nonnull)), int, unsigned long)
+__attribute__((returns_nonnull));
+
+void use_declarations(int *p, void *volatile *sink) {
+ foo_decl(p);
+ bar_decl(p);
+ bar2_decl(p, p);
+ (void)bar3_decl();
+ bar4_decl(42, p);
+ bar5_decl(42, p);
+ bar6_decl(p);
+ bar7_decl(p, p);
+ bar8_decl(p, p);
+ *sink = (void *)&memcpy;
+ *sink = (void *)&memcmp;
+ *sink = (void *)&memmove;
+ *sink = (void *)&strncpy;
+ *sink = (void *)&strncmp;
+ *sink = (void *)&strncat;
+ *sink = (void *)&memchr;
+ *sink = (void *)&memset;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -735,
[PATCH] D30776: [coroutines] Fix diagnostics depending on the first coroutine statement.
EricWF added a comment. Good to know. I'll update this tomorrow. https://reviews.llvm.org/D30776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30776: [coroutines] Fix diagnostics depending on the first coroutine statement.
GorNishanov requested changes to this revision. GorNishanov added a comment. This revision now requires changes to proceed. One thought I had with respect to removing array of statements: We need to check during Sema that await_ready returns a type contextually convertible to bool and await_suspend returns either void or bool. If we have this array, I can quickly run through it and verify that all await expressions have expected types during CheckCoroutine body. With the yield/await expression vector removed, I am not sure where would be an appropriate spot to check for that. https://reviews.llvm.org/D30776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30762: [ubsan] Add a nullability sanitizer
vsk added a comment. One question for Anna (inline). I will update the diff with the documentation/code comments/renaming fixes once I hear back. Thanks again for the comments! Comment at: docs/UndefinedBehaviorSanitizer.rst:101 + ``-fsanitize=nullability-assign``, and the argument check with + ``-fsanitize=nullability-arg``. While violating nullability rules does + not result in undefined behavior, it is often unintentional, so UBSan zaks.anna wrote: > Have you investigated if -fsanitize=nullability-arg is too noisy? I think we > should somehow "recommend" return and assignment check to most users. This is > not clear from the help and options provided. The main concern here is that > someone would try -fsanitize=nullability, see a ton of warnings they don't > care about and think that the whole check is too noisy. > > I cannot come up with a great solution. Options that come into mind are: > - Drop "arg" check completely. > - Remove arg from the nullability group. > - Have 2 groups for nullability. > > We can also wait for concrete data before making a decision here and address > this in subsequent commits. The nullability-arg check was not noisy on a small sample (n=4) of Apple-internal projects. I only collected 11 diagnostics in total, all of which seemed actionable. I.e the issues were not isolated to system headers: the project code actually did violate arg nullability. Based on what I've seen so far, I'd like to start off with including the arg check in a "nullability-full" group. If arg check noisiness becomes an issue, then I'd like to create a new "nullability-partial" group with just the return/assignment checks. TL;DR: let's rename -fsanitize=nullability to -fsanitize=nullability-full, keep the arg check, and introduce a nullability-partial group later if we need to. Wdyt? https://reviews.llvm.org/D30762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30675: [clangd] Fix not being able to attach a debugger on macOS
malaperle-ericsson updated this revision to Diff 91246.
malaperle-ericsson added a comment.
Remove ifdexf
https://reviews.llvm.org/D30675
Files:
clangd/ClangDMain.cpp
Index: clangd/ClangDMain.cpp
===
--- clangd/ClangDMain.cpp
+++ clangd/ClangDMain.cpp
@@ -67,6 +67,10 @@
// by \r\n.
std::string Line;
std::getline(std::cin, Line);
+if (!std::cin.good() && errno == EINTR) {
+ std::cin.clear();
+ continue;
+}
// Skip empty lines.
llvm::StringRef LineRef(Line);
Index: clangd/ClangDMain.cpp
===
--- clangd/ClangDMain.cpp
+++ clangd/ClangDMain.cpp
@@ -67,6 +67,10 @@
// by \r\n.
std::string Line;
std::getline(std::cin, Line);
+if (!std::cin.good() && errno == EINTR) {
+ std::cin.clear();
+ continue;
+}
// Skip empty lines.
llvm::StringRef LineRef(Line);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors
hfinkel added a comment. In https://reviews.llvm.org/D12689#515120, @eastig wrote: > The issue is reported as: > https://llvm.org/bugs/show_bug.cgi?id=28954 We do need to fix this bug; I posted to the PR so we can discuss approach there. https://reviews.llvm.org/D12689 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30551: [AMDGPU] Add builtin functions readlane ds_permute mov_dpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297436: [AMDGPU] Add builtin functions readlane ds_permute
mov_dpp (authored by yaxunl).
Changed prior to commit:
https://reviews.llvm.org/D30551?vs=90520&id=91243#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30551
Files:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error.cl
Index: cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error.cl
+++ cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -1,16 +1,17 @@
// REQUIRES: amdgpu-registered-target
// RUN: %clang_cc1 -triple amdgcn-- -target-cpu tahiti -verify -S -o - %s
-// FIXME: We only get one error if the functions are the other order in the
-// file.
-
#pragma OPENCL EXTENSION cl_khr_fp64 : enable
typedef unsigned long ulong;
typedef unsigned int uint;
-ulong test_s_memrealtime()
+// To get all errors for feature checking we need to put them in one function
+// since Clang will stop codegen for the next function if it finds error during
+// codegen of the previous function.
+void test_target_builtin(global int* out, int a)
{
- return __builtin_amdgcn_s_memrealtime(); // expected-error {{'__builtin_amdgcn_s_memrealtime' needs target feature s-memrealtime}}
+ __builtin_amdgcn_s_memrealtime(); // expected-error {{'__builtin_amdgcn_s_memrealtime' needs target feature s-memrealtime}}
+ *out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, false); // expected-error {{'__builtin_amdgcn_mov_dpp' needs target feature dpp}}
}
void test_s_sleep(int x)
@@ -92,3 +93,12 @@
{
*out = __builtin_amdgcn_s_getreg(a); // expected-error {{argument to '__builtin_amdgcn_s_getreg' must be a constant integer}}
}
+
+void test_mov_dpp2(global int* out, int a, int b, int c, int d, bool e)
+{
+ *out = __builtin_amdgcn_mov_dpp(a, b, 0, 0, false); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_mov_dpp(a, 0, c, 0, false); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_mov_dpp(a, 0, 0, d, false); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
+ *out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
+}
+
Index: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
===
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -81,3 +81,11 @@
{
*out = __builtin_amdgcn_s_memrealtime();
}
+
+// CHECK-LABEL: @test_mov_dpp
+// CHECK: call i32 @llvm.amdgcn.mov.dpp.i32(i32 %src, i32 0, i32 0, i32 0, i1 false)
+void test_mov_dpp(global int* out, int src)
+{
+ *out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
+}
+
Index: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -235,6 +235,34 @@
*out = __builtin_amdgcn_ds_swizzle(a, 32);
}
+// CHECK-LABEL: @test_ds_permute
+// CHECK: call i32 @llvm.amdgcn.ds.permute(i32 %a, i32 %b)
+void test_ds_permute(global int* out, int a, int b)
+{
+ out[0] = __builtin_amdgcn_ds_permute(a, b);
+}
+
+// CHECK-LABEL: @test_ds_bpermute
+// CHECK: call i32 @llvm.amdgcn.ds.bpermute(i32 %a, i32 %b)
+void test_ds_bpermute(global int* out, int a, int b)
+{
+ *out = __builtin_amdgcn_ds_bpermute(a, b);
+}
+
+// CHECK-LABEL: @test_readfirstlane
+// CHECK: call i32 @llvm.amdgcn.readfirstlane(i32 %a)
+void test_readfirstlane(global int* out, int a)
+{
+ *out = __builtin_amdgcn_readfirstlane(a);
+}
+
+// CHECK-LABEL: @test_readlane
+// CHECK: call i32 @llvm.amdgcn.readlane(i32 %a, i32 %b)
+void test_readlane(global int* out, int a, int b)
+{
+ *out = __builtin_amdgcn_readlane(a, b);
+}
+
// CHECK-LABEL: @test_fcmp_f32
// CHECK: call i64 @llvm.amdgcn.fcmp.f32(float %a, float %b, i32 5)
void test_fcmp_f32(global ulong* out, float a, float b)
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -8401,6 +8401,14 @@
case AMDGPU::BI__builtin_amdgcn_ds_swizzle:
return emitBinaryBuiltin(*this, E, Intrinsic::amdgcn_ds_swizzle);
+ case AMDGPU::BI__builtin_amdgcn_mov_dpp: {
+llvm::SmallVector Args;
+for (unsigned I = 0; I != 5; ++I)
+ Args.push_back(EmitScalarExpr(E->getArg(I)));
+Value *F = CGM.getIntrinsic(In
r297436 - [AMDGPU] Add builtin functions readlane ds_permute mov_dpp
Author: yaxunl
Date: Thu Mar 9 19:30:46 2017
New Revision: 297436
URL: http://llvm.org/viewvc/llvm-project?rev=297436&view=rev
Log:
[AMDGPU] Add builtin functions readlane ds_permute mov_dpp
Differential Revision: https://reviews.llvm.org/D30551
Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
cfe/trunk/test/SemaOpenCL/builtins-amdgcn-error.cl
Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=297436&r1=297435&r2=297436&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Thu Mar 9 19:30:46 2017
@@ -86,6 +86,10 @@ BUILTIN(__builtin_amdgcn_sicmpl, "LUiLiL
BUILTIN(__builtin_amdgcn_fcmp, "LUiddIi", "nc")
BUILTIN(__builtin_amdgcn_fcmpf, "LUiffIi", "nc")
BUILTIN(__builtin_amdgcn_ds_swizzle, "iiIi", "nc")
+BUILTIN(__builtin_amdgcn_ds_permute, "iii", "nc")
+BUILTIN(__builtin_amdgcn_ds_bpermute, "iii", "nc")
+BUILTIN(__builtin_amdgcn_readfirstlane, "ii", "nc")
+BUILTIN(__builtin_amdgcn_readlane, "iii", "nc")
BUILTIN(__builtin_amdgcn_fmed3f, "", "nc")
//===--===//
@@ -103,6 +107,7 @@ TARGET_BUILTIN(__builtin_amdgcn_frexp_ex
TARGET_BUILTIN(__builtin_amdgcn_fracth, "hh", "nc", "16-bit-insts")
TARGET_BUILTIN(__builtin_amdgcn_classh, "bhi", "nc", "16-bit-insts")
TARGET_BUILTIN(__builtin_amdgcn_s_memrealtime, "LUi", "n", "s-memrealtime")
+TARGET_BUILTIN(__builtin_amdgcn_mov_dpp, "iiIiIiIiIb", "nc", "dpp")
//===--===//
// GFX9+ only builtins.
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=297436&r1=297435&r2=297436&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Mar 9 19:30:46 2017
@@ -2387,6 +2387,7 @@ bool AMDGPUTargetInfo::initFeatureMap(
case GK_GFX8:
Features["s-memrealtime"] = true;
Features["16-bit-insts"] = true;
+ Features["dpp"] = true;
break;
case GK_NONE:
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=297436&r1=297435&r2=297436&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Mar 9 19:30:46 2017
@@ -8401,6 +8401,14 @@ Value *CodeGenFunction::EmitAMDGPUBuilti
case AMDGPU::BI__builtin_amdgcn_ds_swizzle:
return emitBinaryBuiltin(*this, E, Intrinsic::amdgcn_ds_swizzle);
+ case AMDGPU::BI__builtin_amdgcn_mov_dpp: {
+llvm::SmallVector Args;
+for (unsigned I = 0; I != 5; ++I)
+ Args.push_back(EmitScalarExpr(E->getArg(I)));
+Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_mov_dpp,
+Args[0]->getType());
+return Builder.CreateCall(F, Args);
+ }
case AMDGPU::BI__builtin_amdgcn_div_fixup:
case AMDGPU::BI__builtin_amdgcn_div_fixupf:
case AMDGPU::BI__builtin_amdgcn_div_fixuph:
Modified: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl?rev=297436&r1=297435&r2=297436&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-vi.cl Thu Mar 9 19:30:46 2017
@@ -81,3 +81,11 @@ void test_s_memrealtime(global ulong* ou
{
*out = __builtin_amdgcn_s_memrealtime();
}
+
+// CHECK-LABEL: @test_mov_dpp
+// CHECK: call i32 @llvm.amdgcn.mov.dpp.i32(i32 %src, i32 0, i32 0, i32 0, i1
false)
+void test_mov_dpp(global int* out, int src)
+{
+ *out = __builtin_amdgcn_mov_dpp(src, 0, 0, 0, false);
+}
+
Modified: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl?rev=297436&r1=297435&r2=297436&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl Thu Mar 9 19:30:46 2017
@@ -235,6 +235,34 @@ void test_ds_swizzle(global int* out, in
*out = __builtin_amdgcn_ds_swizzle(a, 32);
}
+// CHECK-LABEL: @test_ds_permute
+// CHECK: call i32 @llvm.amdgcn.ds.permute(i32 %a, i32 %b)
+void test_ds_permute(global int* out, int a, int b)
+{
+ out[0]
[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default
zaks.anna added a comment. I've committed the change, but would very much appreciate community feedback here if if there is any! Repository: rL LLVM https://reviews.llvm.org/D30798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297429: [analyzer] Turn suppress-c++-stdlib on by default
(authored by zaks).
Changed prior to commit:
https://reviews.llvm.org/D30798?vs=91236&id=91238#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30798
Files:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
Index: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
===
--- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
+++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -230,7 +230,7 @@
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
Index: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
===
--- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
+++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -230,7 +230,7 @@
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r297429 - [analyzer] Turn suppress-c++-stdlib on by default
Author: zaks
Date: Thu Mar 9 18:33:19 2017
New Revision: 297429
URL: http://llvm.org/viewvc/llvm-project?rev=297429&view=rev
Log:
[analyzer] Turn suppress-c++-stdlib on by default
We have several reports of false positives coming from libc++. For example,
there are reports of false positives in std::regex, std::wcout, and also
a bunch of issues are reported in https://reviews.llvm.org/D30593. In many
cases, the analyzer trips over the complex libc++ code invariants. Let's turn
off the reports coming from these headers until we can re-evalate the support.
We can turn this back on once we individually suppress all known false
positives and perform deeper evaluation on large codebases that use libc++.
We'd also need to commit to doing these evaluations regularly as libc++
headers change.
Differential Revision: https://reviews.llvm.org/D30798
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=297429&r1=297428&r2=297429&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Thu Mar 9 18:33:19
2017
@@ -230,7 +230,7 @@ bool AnalyzerOptions::shouldSuppressInli
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
Modified: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp?rev=297429&r1=297428&r2=297429&view=diff
==
--- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp Thu Mar 9
18:33:19 2017
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30733: [Driver] Add arch-specific rpath for libc++
jroelofs requested changes to this revision. jroelofs added a comment. This revision now requires changes to proceed. As I said on https://reviews.llvm.org/D30214, it is inappropriate to be installing libc++ in the resource directory... please **do not** do that. https://reviews.llvm.org/D30733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. LGTM, https://reviews.llvm.org/D30798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter.
zaks.anna added a comment. @kmarshall, We are going to turn this off by default, see https://reviews.llvm.org/D30798. Please, do file a bug that lists all the issues you are seeing and desirably instructions on how to reproduce. (Please, list even the cases you are not 100% sure are false positives) Thank you! Repository: rL LLVM https://reviews.llvm.org/D30593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.
ributzka added inline comments. Comment at: include/clang/Basic/VirtualFileSystem.h:164 EC = Impl->increment(); -if (EC || !Impl->CurrentEntry.isStatusKnown()) +if (!Impl->CurrentEntry.isStatusKnown()) Impl.reset(); // Normalize the end iterator to Impl == nullptr. bruno wrote: > I would rather we don't drop checks for `EC`s - it's commonly assumed that > whenever they are passed in we wanna check them for errors, can't you just > skip the specific case you want to avoid? EC is an output parameter here. The client is supposed to check it. https://reviews.llvm.org/D30768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default
zaks.anna created this revision.
We have several reports of false positives coming from libc++. For example,
there are reports of false positives in std::regex, std::wcout, and also a
bunch of issues are reported in https://reviews.llvm.org/D30593. In many cases,
the analyzer trips over the complex libc++ code invariants. Let's turn off the
reports coming from these headers until we can re-evalate the support.
We can turn this back on once we individually suppress all known false
positives and perform deeper evaluation on large codebases that use libc++.
We'd also need to commit to doing these evaluations regularly as libc++ headers
change.
https://reviews.llvm.org/D30798
Files:
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
test/Analysis/diagnostics/explicit-suppression.cpp
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -230,7 +230,7 @@
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -230,7 +230,7 @@
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30733: [Driver] Add arch-specific rpath for libc++
pirama added inline comments. Comment at: lib/Driver/ToolChain.cpp:652 +// libc++ may be installed per arch. +addArchSpecificRPath(*this, Args, CmdArgs); break; `addArchSpecificRPath` is a static function in Tools.cpp and isn't visible here. https://reviews.llvm.org/D30733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter.
kmarshall added a comment. OK. I'll make bugs for the false positives I'm confident about. Repository: rL LLVM https://reviews.llvm.org/D30593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30792: Use callback for internalizing linked symbols.
tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.
LGTM with the format fix I pointed out.
Comment at: lib/CodeGen/CodeGenAction.cpp:186
+} else {
+ Err = Linker::linkModules(*getModule(), std::move(LM.Module),
+LM.LinkFlags);
tejohnson wrote:
> Similar question (to what I added to the LLVM patch just now) as to why this
> is needed given the default for that parameter.
As in the other patch, ignore my nonsense comment here.
Repository:
rL LLVM
https://reviews.llvm.org/D30792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.
ributzka added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:1873 vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC); -if (EC) +if (EC && EC != std::errc::no_such_file_or_directory) return *this; bruno wrote: > Can you add a comment explaining why you are doing it? I would prefer if we > reset the `EC` state here than having the callers ignoring `EC` results. If we reset the EC here, then the caller won't know that there was an issue. The idea is that we still want the caller to check EC. It should be up to the caller to decide how to act on this particular error. I guess since the caller has to check for the error anyway I could even remove this check completely and not check EC at all here. https://reviews.llvm.org/D30768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30792: Use callback for internalizing linked symbols.
tejohnson added a comment.
Great, thanks. One question below and a format nit.
Comment at: lib/CodeGen/CodeGenAction.cpp:177
+if (LM.Internalize) {
+ Err = Linker::linkModules(
+ *getModule(), std::move(LM.Module), LM.LinkFlags,
Needs clang-format (you can use git clang-format to change just the files in
this patch if you use git).
Comment at: lib/CodeGen/CodeGenAction.cpp:186
+} else {
+ Err = Linker::linkModules(*getModule(), std::move(LM.Module),
+LM.LinkFlags);
Similar question (to what I added to the LLVM patch just now) as to why this is
needed given the default for that parameter.
Repository:
rL LLVM
https://reviews.llvm.org/D30792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r297412 - Add -cc1 flag -ast-dump-all to perform an AST dump including entities that haven't yet been deserialized.
Author: rsmith Date: Thu Mar 9 16:00:01 2017 New Revision: 297412 URL: http://llvm.org/viewvc/llvm-project?rev=297412&view=rev Log: Add -cc1 flag -ast-dump-all to perform an AST dump including entities that haven't yet been deserialized. Modified: cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Frontend/ASTConsumers.h cfe/trunk/include/clang/Frontend/FrontendOptions.h cfe/trunk/lib/AST/ASTDumper.cpp cfe/trunk/lib/Frontend/ASTConsumers.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/FrontendActions.cpp cfe/trunk/tools/clang-check/ClangCheck.cpp Modified: cfe/trunk/include/clang/AST/DeclBase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=297412&r1=297411&r2=297412&view=diff == --- cfe/trunk/include/clang/AST/DeclBase.h (original) +++ cfe/trunk/include/clang/AST/DeclBase.h Thu Mar 9 16:00:01 2017 @@ -1029,7 +1029,7 @@ public: void dump() const; // Same as dump(), but forces color printing. void dumpColor() const; - void dump(raw_ostream &Out) const; + void dump(raw_ostream &Out, bool Deserialize = false) const; /// \brief Looks through the Decl's underlying type to extract a FunctionType /// when possible. Will return null if the type underlying the Decl does not @@ -1810,7 +1810,8 @@ public: void dumpDeclContext() const; void dumpLookups() const; - void dumpLookups(llvm::raw_ostream &OS, bool DumpDecls = false) const; + void dumpLookups(llvm::raw_ostream &OS, bool DumpDecls = false, + bool Deserialize = false) const; private: void reconcileExternalVisibleStorage() const; Modified: cfe/trunk/include/clang/Driver/CC1Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=297412&r1=297411&r2=297412&view=diff == --- cfe/trunk/include/clang/Driver/CC1Options.td (original) +++ cfe/trunk/include/clang/Driver/CC1Options.td Thu Mar 9 16:00:01 2017 @@ -475,6 +475,8 @@ def ast_list : Flag<["-"], "ast-list">, HelpText<"Build ASTs and print the list of declaration node qualified names">; def ast_dump : Flag<["-"], "ast-dump">, HelpText<"Build ASTs and then debug dump them">; +def ast_dump_all : Flag<["-"], "ast-dump-all">, + HelpText<"Build ASTs and then debug dump them, forcing deserialization">; def ast_dump_lookups : Flag<["-"], "ast-dump-lookups">, HelpText<"Build ASTs and then debug dump their name lookup tables">; def ast_view : Flag<["-"], "ast-view">, Modified: cfe/trunk/include/clang/Frontend/ASTConsumers.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTConsumers.h?rev=297412&r1=297411&r2=297412&view=diff == --- cfe/trunk/include/clang/Frontend/ASTConsumers.h (original) +++ cfe/trunk/include/clang/Frontend/ASTConsumers.h Thu Mar 9 16:00:01 2017 @@ -37,7 +37,7 @@ std::unique_ptr CreateASTPr // AST dumper: dumps the raw AST in human-readable form to stderr; this is // intended for debugging. std::unique_ptr CreateASTDumper(StringRef FilterString, - bool DumpDecls, + bool DumpDecls, bool Deserialize, bool DumpLookups); // AST Decl node lister: prints qualified names of all filterable AST Decl Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=297412&r1=297411&r2=297412&view=diff == --- cfe/trunk/include/clang/Frontend/FrontendOptions.h (original) +++ cfe/trunk/include/clang/Frontend/FrontendOptions.h Thu Mar 9 16:00:01 2017 @@ -157,6 +157,8 @@ public: ///< global module index if needed. unsigned ASTDumpDecls : 1; ///< Whether we include declaration ///< dumps in AST dumps. + unsigned ASTDumpAll : 1; ///< Whether we deserialize all decls + ///< when forming AST dumps. unsigned ASTDumpLookups : 1; ///< Whether we include lookup table ///< dumps in AST dumps. unsigned BuildingImplicitModule : 1; ///< Whether we are performing an Modified: cfe/trunk/lib/AST/ASTDumper.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=297412&r1=297411&r2=297412&view=diff == --- cfe/trunk/lib/AST/ASTDumper.cpp (original) +++ cfe/trunk/lib/AST/ASTDumper.c
[PATCH] D27810: FileManager: mark virtual file entries as valid entries
bruno added a comment. Please attach a testcase! https://reviews.llvm.org/D27810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch for Bug 30413, including test case
Hi David,
The patch looks good to me.
> On Mar 9, 2017, at 1:01 PM, Lobron, David wrote:
>
> Hi Akira,
>
>> My concern is that the patch changes the encoding of @encode(id)
>> on Darwin, which I think isn’t what you are trying to fix. If you compile
>> the following code with command “clang -cc1 -triple x86_64-apple-macosx”,
>> the type encoding changes after applying the patch.
>>
>> const char *foo() {
>> return @encode(id);
>> }
>>
>> It seems like you can fix your problem without affecting Darwin by passing
>> an extra argument to getObjCEncodingForType, just like
>> CGObjCCommonMac::GetMethodVarType does.
>
> Ah, thanks- I understand now. Yes, this change seems a lot safer, and I
> verified that it passes my test. I've attached my new patch file, and I've
> also attached the test again. Please let me know if this works for you or if
> you think it needs any additional work.
>
> --David
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch for Bug 30413, including test case
Hi Akira,
> My concern is that the patch changes the encoding of @encode(id) on
> Darwin, which I think isn’t what you are trying to fix. If you compile the
> following code with command “clang -cc1 -triple x86_64-apple-macosx”, the
> type encoding changes after applying the patch.
>
> const char *foo() {
> return @encode(id);
> }
>
> It seems like you can fix your problem without affecting Darwin by passing an
> extra argument to getObjCEncodingForType, just like
> CGObjCCommonMac::GetMethodVarType does.
Ah, thanks- I understand now. Yes, this change seems a lot safer, and I
verified that it passes my test. I've attached my new patch file, and I've
also attached the test again. Please let me know if this works for you or if
you think it needs any additional work.
--David
CGObjCGNU.cpp.patch
Description: CGObjCGNU.cpp.patch
ivar-type-encoding.m
Description: ivar-type-encoding.m
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.
bruno added a comment.
Hi Juergen,
Thanks for working on this.
Comment at: include/clang/Basic/VirtualFileSystem.h:164
EC = Impl->increment();
-if (EC || !Impl->CurrentEntry.isStatusKnown())
+if (!Impl->CurrentEntry.isStatusKnown())
Impl.reset(); // Normalize the end iterator to Impl == nullptr.
I would rather we don't drop checks for `EC`s - it's commonly assumed that
whenever they are passed in we wanna check them for errors, can't you just skip
the specific case you want to avoid?
Comment at: lib/Basic/VirtualFileSystem.cpp:1860
directory_iterator I = FS->dir_begin(Path, EC);
- if (!EC && I != directory_iterator()) {
+ if (I != directory_iterator()) {
State = std::make_shared();
Same here.
Comment at: lib/Basic/VirtualFileSystem.cpp:1873
vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-if (EC)
+if (EC && EC != std::errc::no_such_file_or_directory)
return *this;
Can you add a comment explaining why you are doing it? I would prefer if we
reset the `EC` state here than having the callers ignoring `EC` results.
https://reviews.llvm.org/D30768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters
bettinson updated this revision to Diff 91204.
bettinson added a comment.
Updated the test name. Sorry for the delay!
https://reviews.llvm.org/D26800
Files:
include/clang/Basic/Attr.td
test/Sema/nonnull.c
Index: test/Sema/nonnull.c
===
--- test/Sema/nonnull.c
+++ test/Sema/nonnull.c
@@ -111,7 +111,7 @@
return 0;
} else {
return *pointer;
- }
+ }
set_param_to_null(&pointer);
if (!pointer)
@@ -167,3 +167,10 @@
int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull
function call 'returns_nonnull_whee()' will evaluate to 'true' on first
encounter}}
and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function
call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
}
+
+void pr30828(char *p __attribute__((nonnull)));
+void pr30828(char *p) {}
+
+void call_pr30828() {
+ pr30828(0); // expected-warning {{null passed to a callee that requires a
non-null argument}}
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1149,7 +1149,7 @@
let Documentation = [NoSplitStackDocs];
}
-def NonNull : InheritableAttr {
+def NonNull : InheritableParamAttr {
let Spellings = [GCC<"nonnull">];
let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
"ExpectedFunctionMethodOrParameter">;
Index: test/Sema/nonnull.c
===
--- test/Sema/nonnull.c
+++ test/Sema/nonnull.c
@@ -111,7 +111,7 @@
return 0;
} else {
return *pointer;
- }
+ }
set_param_to_null(&pointer);
if (!pointer)
@@ -167,3 +167,10 @@
int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}}
}
+
+void pr30828(char *p __attribute__((nonnull)));
+void pr30828(char *p) {}
+
+void call_pr30828() {
+ pr30828(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1149,7 +1149,7 @@
let Documentation = [NoSplitStackDocs];
}
-def NonNull : InheritableAttr {
+def NonNull : InheritableParamAttr {
let Spellings = [GCC<"nonnull">];
let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
"ExpectedFunctionMethodOrParameter">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27627: [WIP] Supporting C++ based kernel languages on AMDGPU Target
kzhuravl added a comment. Once you rebase, can you also update AMDGPUTargetInfo::getDWARFAddressSpace to use AddrSpaceKind? https://reviews.llvm.org/D27627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30777: Added `applyAtomicChanges` function.
djasper added inline comments.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:139
+ // kNone: Don't format anything.
+ // kViolations: Format lines exceeding 80 columns.
+ enum FormatOption { kAll, kNone, kViolations };
Should probably be "exceeding the column limit".
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:91
+// [Start, End].
+bool ViolatesColumnLimit(llvm::StringRef Code, unsigned ColumnLimit,
+ unsigned Start, unsigned End) {
I think LLVM style is lowerCamelCase now, right? Here and below.
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:113
+ const clang::tooling::Replacements &Replaces) {
+ std::vector Ranges;
+ // kNone suppresses formatting entirely.
nit: I'd move this down and just return {} for kNone.
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:127
+R.getOffset() > 0 && R.getOffset() <= Code.size() &&
+Code[R.getOffset() - 1] == '\n') {
+ // If we are inserting at the start of a line and the replacement ends in
Remove braces?
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:151
+ const format::FormatStyle &Style) {
+ llvm::SmallVector InsertedHeaders;
+ llvm::SmallVector RemovedHeaders;
What is copying these vectors buying us?
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:164
+std::string EscapedHeader =
+(Header.startswith("<") || Header.startswith("\""))
+? Header.str()
I'd remove the parentheses.
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:196
+ Replacements Replaces;
+ for (const auto &Change : Changes) {
+for (const auto &R : Change.getReplacements()) {
Remove the braces of the for loops.
https://reviews.llvm.org/D30777
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29901: Modular Codegen: Add/use a bit in serialized function definitions to track whether they are the subject of modular codegen
dblaikie added a comment. Ping I've got a relatively small patch to add support for debug info types after this one & working on a patch on top of that to enable one/the other/both of these (to test their value independently/together). Might be more expedient to sit down & push through them all together side-by-side? Or not, not sure. https://reviews.llvm.org/D29901 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29901: Modular Codegen: Add/use a bit in serialized function definitions to track whether they are the subject of modular codegen
dblaikie updated this revision to Diff 91193.
dblaikie added a comment.
sync/resolve
https://reviews.llvm.org/D29901
Files:
include/clang/AST/ASTContext.h
include/clang/AST/ExternalASTSource.h
include/clang/Sema/MultiplexExternalSemaSource.h
include/clang/Serialization/ASTReader.h
lib/AST/ASTContext.cpp
lib/AST/ExternalASTSource.cpp
lib/Sema/MultiplexExternalSemaSource.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/Modules/Inputs/codegen/foo.h
test/Modules/Inputs/codegen/use.cpp
test/Modules/codegen.test
Index: test/Modules/codegen.test
===
--- test/Modules/codegen.test
+++ test/Modules/codegen.test
@@ -3,8 +3,23 @@
RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++ -fmodules -emit-module -fmodule-name=foo %S/Inputs/codegen/foo.modulemap -o %t/foo.pcm
-RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/foo.pcm | FileCheck %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/foo.pcm | FileCheck --check-prefix=FOO --check-prefix=BOTH %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -fmodules -fmodule-file=%t/foo.pcm %S/Inputs/codegen/use.cpp | FileCheck --check-prefix=BOTH --check-prefix=USE %s
-CHECK: $_Z2f1PKcz = comdat any
-CHECK: define weak_odr void @_Z2f1PKcz(i8* %fmt, ...) #{{[0-9]+}} comdat
-CHECK: call void @llvm.va_start(i8* %{{[a-zA-Z0-9]*}})
+FOO: $_Z2f1PKcz = comdat any
+FOO: $_ZN13implicit_dtorD1Ev = comdat any
+USE: $_Z4instIiEvv = comdat any
+FOO: $_ZN13implicit_dtorD2Ev = comdat any
+FOO: define weak_odr void @_Z2f1PKcz(i8* %fmt, ...) #{{[0-9]+}} comdat
+FOO: call void @llvm.va_start(i8* %{{[a-zA-Z0-9]*}})
+
+Test that implicit special members (like this dtor) are emitted into both module (if they're used there) and user.
+FIXME: Proactively instantiate any valid implicit special members to emit them into the module object.
+
+FOO: define weak_odr void @_ZN13implicit_dtorD1Ev(%struct.implicit_dtor* %this) unnamed_addr #0 comdat align 2 {
+FOO: define weak_odr void @_Z4instIfEvv() #0 comdat {
+FOO: define weak_odr void @_ZN13implicit_dtorD2Ev(%struct.implicit_dtor* %this) unnamed_addr #0 comdat align 2 {
+
+USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD1Ev(%struct.uninst_implicit_dtor* %this) unnamed_addr #0 comdat align 2 {
+USE: define linkonce_odr void @_Z4instIiEvv() #0 comdat {
+USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD2Ev(%struct.uninst_implicit_dtor* %this) unnamed_addr #0 comdat align 2 {
Index: test/Modules/Inputs/codegen/use.cpp
===
--- /dev/null
+++ test/Modules/Inputs/codegen/use.cpp
@@ -0,0 +1,8 @@
+#include "foo.h"
+void non_modular_use_of_implicit_dtor() {
+ implicit_dtor d1;
+ uninst_implicit_dtor d2;
+}
+void use_of_instantiated_declaration_without_definition() {
+ inst();
+}
Index: test/Modules/Inputs/codegen/foo.h
===
--- test/Modules/Inputs/codegen/foo.h
+++ test/Modules/Inputs/codegen/foo.h
@@ -2,3 +2,29 @@
__builtin_va_list args;
__builtin_va_start(args, fmt);
}
+
+struct non_trivial_dtor {
+ ~non_trivial_dtor();
+};
+
+struct implicit_dtor {
+ non_trivial_dtor d;
+};
+
+struct uninst_implicit_dtor {
+ non_trivial_dtor d;
+};
+
+inline void use_implicit_dtor() {
+ implicit_dtor d;
+}
+
+template
+void inst() {
+}
+
+inline void inst_decl() {
+ // cause inst's declaration to be instantiated, without a definition.
+ (void)sizeof(&inst);
+ inst();
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2157,7 +2157,7 @@
/// relatively painless since they would presumably only do it for top-level
/// decls.
static bool isRequiredDecl(const Decl *D, ASTContext &Context,
- bool WritingModule, bool ModularCode) {
+ bool WritingModule) {
// An ObjCMethodDecl is never considered as "required" because its
// implementation container always is.
@@ -2173,7 +2173,7 @@
return false;
}
- return Context.DeclMustBeEmitted(D, ModularCode);
+ return Context.DeclMustBeEmitted(D);
}
void ASTWriter::WriteDecl(ASTContext &Context, Decl *D) {
@@ -2217,18 +2217,20 @@
// Note declarations that should be deserialized eagerly so that we can add
// them to a record in the AST file later.
- if (isRequiredDecl(D, Context, WritingModule, false))
+ if (isRequiredDecl(D, Context, WritingModule))
EagerlyDeserializedDecls.push_back(ID);
- else if (Context.getLangOpts().ModularCodegen && WritingModule &&
- isRequiredDecl(D, Context, true, true))
-ModularCodegenDecls.push_back(ID);
}
void ASTRecordWriter::AddFunctionDefinition(const FunctionDecl *FD) {
// Switch case IDs
[PATCH] D29923: Send UndefMacroDirective to MacroDefined callback
manmanren added a comment.
Please update the patch with context:
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Thanks,
Manman
Comment at: unittests/Basic/SourceManagerTest.cpp:251
std::string Name;
- bool isDefinition; // if false, it is expansion.
-
- MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-: Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+ int Kind; // 0 expansion, 1 definition, 2 undefinition
+
Can we make this an enum?
https://reviews.llvm.org/D29923
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r297397 - [DebugInfo] Append extended dereferencing mechanism to variables' DIExpression for targets that support more than one address space
Author: kzhuravl
Date: Thu Mar 9 12:06:23 2017
New Revision: 297397
URL: http://llvm.org/viewvc/llvm-project?rev=297397&view=rev
Log:
[DebugInfo] Append extended dereferencing mechanism to variables' DIExpression
for targets that support more than one address space
Differential Revision: https://reviews.llvm.org/D29673
Added:
cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=297397&r1=297396&r2=297397&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Mar 9 12:06:23 2017
@@ -3272,6 +3272,20 @@ void CGDebugInfo::CreateLexicalBlock(Sou
getColumnNumber(CurLoc)));
}
+void CGDebugInfo::AppendAddressSpaceXDeref(
+unsigned AddressSpace,
+SmallVectorImpl &Expr) const {
+ Optional DWARFAddressSpace =
+ CGM.getTarget().getDWARFAddressSpace(AddressSpace);
+ if (!DWARFAddressSpace)
+return;
+
+ Expr.push_back(llvm::dwarf::DW_OP_constu);
+ Expr.push_back(DWARFAddressSpace.getValue());
+ Expr.push_back(llvm::dwarf::DW_OP_swap);
+ Expr.push_back(llvm::dwarf::DW_OP_xderef);
+}
+
void CGDebugInfo::EmitLexicalBlockStart(CGBuilderTy &Builder,
SourceLocation Loc) {
// Set our current location.
@@ -3422,13 +3436,16 @@ void CGDebugInfo::EmitDeclare(const VarD
Line = getLineNumber(VD->getLocation());
Column = getColumnNumber(VD->getLocation());
}
- SmallVector Expr;
+ SmallVector Expr;
llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
if (VD->isImplicit())
Flags |= llvm::DINode::FlagArtificial;
auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
+ unsigned AddressSpace =
CGM.getContext().getTargetAddressSpace(VD->getType());
+ AppendAddressSpaceXDeref(AddressSpace, Expr);
+
// If this is the first argument and it is implicit then
// give it an object pointer flag.
// FIXME: There has to be a better way to do this, but for static
@@ -3857,9 +3874,16 @@ void CGDebugInfo::EmitGlobalVariable(llv
GVE = CollectAnonRecordDecls(RD, Unit, LineNo, LinkageName, Var, DContext);
} else {
auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+
+SmallVector Expr;
+unsigned AddressSpace =
+CGM.getContext().getTargetAddressSpace(D->getType());
+AppendAddressSpaceXDeref(AddressSpace, Expr);
+
GVE = DBuilder.createGlobalVariableExpression(
DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T,
Unit),
-Var->hasLocalLinkage(), /*Expr=*/nullptr,
+Var->hasLocalLinkage(),
+Expr.empty() ? nullptr : DBuilder.createExpression(Expr),
getOrCreateStaticDataMemberDeclarationOrNull(D), Align);
Var->addDebugInfo(GVE);
}
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=297397&r1=297396&r2=297397&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Mar 9 12:06:23 2017
@@ -293,6 +293,15 @@ class CGDebugInfo {
/// Create a new lexical block node and push it on the stack.
void CreateLexicalBlock(SourceLocation Loc);
+ /// If target-specific LLVM \p AddressSpace directly maps to target-specific
+ /// DWARF address space, appends extended dereferencing mechanism to complex
+ /// expression \p Expr. Otherwise, does nothing.
+ ///
+ /// Extended dereferencing mechanism is has the following format:
+ /// DW_OP_constu DW_OP_swap DW_OP_xderef
+ void AppendAddressSpaceXDeref(unsigned AddressSpace,
+SmallVectorImpl &Expr) const;
+
public:
CGDebugInfo(CodeGenModule &CGM);
~CGDebugInfo();
Added: cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl?rev=297397&view=auto
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
(added)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl Thu
Mar 9 12:06:23 2017
@@ -0,0 +1,131 @@
+// RUN: %clang -cl-std=CL2.0 -emit-llvm -g -O0 -S -target amdgcn-amd-amdhsa
-mcpu=fiji -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[NONE:[0-9]+]] = !DIExpression()
+// CHECK-DAG: ![[LOCAL:[0-9]+]] = !DIExpression(DW_OP_constu, 2, DW_OP_swap,
DW_OP_xderef)
+// CHECK-DAG: ![[PRIVATE:[0-9]+]] = !DIExpression(DW_OP_constu, 1, DW_OP_swap,
DW_OP_xderef)
+
+// CHECK-DAG: ![[FILEVAR0:[0-9]+]] = distinct !DIGlobalVariable(name:
"File
[PATCH] D30762: [ubsan] Add a nullability sanitizer
filcab added a comment. Please make the tests tighter using `CHECK-NEXT` when possible. Much easier if later anyone needs to debug differences in IR. Comment at: docs/UndefinedBehaviorSanitizer.rst:102 + violating nullability rules does not result in undefined behavior, it + is often unintentional, so UBSan offers to catch it. - ``-fsanitize=object-size``: An attempt to potentially use bytes which This is weird. Please just document each of the `nullability-*` in isolation. None of the other flags is documenting more than one flag per bullet point, and I think splitting them up here is easy. It also makes it easier to read. You already added the `nullability` group below, which mentions it adds the three checks. Comment at: docs/UndefinedBehaviorSanitizer.rst:141 - ``-fsanitize=undefined``: All of the checks listed above other than - ``unsigned-integer-overflow``. + ``unsigned-integer-overflow`` and ``nullability``. - ``-fsanitize=undefined-trap``: Deprecated alias of ```... and the ``nullability`` group.``` Comment at: test/CodeGenObjC/ubsan-nullability.m:114 + // CHECK: [[NONULL]]: + // CHECK: ret i32* +} `CHECK-NEXT`? https://reviews.llvm.org/D30762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 91188.
fgross added a comment.
Diff was missing cppcoreguidelines-special-member-functions-relaxed.cpp...
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+class DefinesCopyConstructor {
+ DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+ DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+ DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+ DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &) = delete;
+ DeletesEverything &operator=(const DeletesEverything &) = delete;
+ DeletesEverything(DeletesEverything &&) = delete;
+ DeletesEverything &operator=(DeletesEverything &&) = delete;
+ ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+ DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsM
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 91183.
fgross added a comment.
Added examples to options doc.
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -3,7 +3,7 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -19,3 +19,31 @@
Guidelines, corresponding to rule C.21. See
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.
+
+Options
+---
+
+.. option:: AllowSoleDefaultDtor
+
+ When set to `1` (default is `0`), this check doesn't flag classes with a sole, explicitly
+ defaulted destructor. An example for such a class is:
+
+ .. code-block:: c++
+
+ struct A {
+ virtual ~A() = default;
+ };
+
+.. option:: AllowMissingMoveFunctions
+
+ When set to `1` (default is `0`), this check doesn't flag classes which define no move
+ operations at all. It still flags classes which define only one of either
+ move constructor or move assignment operator. With this option enabled, the following class won't be flagged:
+
+ .. code-block:: c++
+
+ struct A {
+ A(const A&);
+ A& operator=(const A&);
+ ~A();
+ }
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -25,14 +25,16 @@
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html
class SpecialMemberFunctionsCheck : public ClangTidyCheck {
public:
- SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void onEndOfTranslationUnit() override;
enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
+DefaultDestructor,
+NonDefaultDestructor,
CopyConstructor,
CopyAssignment
[PATCH] D30762: [ubsan] Add a nullability sanitizer
zaks.anna added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:101 + ``-fsanitize=nullability-assign``, and the argument check with + ``-fsanitize=nullability-arg``. While violating nullability rules does + not result in undefined behavior, it is often unintentional, so UBSan Have you investigated if -fsanitize=nullability-arg is too noisy? I think we should somehow "recommend" return and assignment check to most users. This is not clear from the help and options provided. The main concern here is that someone would try -fsanitize=nullability, see a ton of warnings they don't care about and think that the whole check is too noisy. I cannot come up with a great solution. Options that come into mind are: - Drop "arg" check completely. - Remove arg from the nullability group. - Have 2 groups for nullability. We can also wait for concrete data before making a decision here and address this in subsequent commits. Comment at: lib/CodeGen/CGDecl.cpp:1907 + + // Skip the return value nullability check if the nullability preconditions + // are broken. I would add a comment explaining why this is needed, similar to what you included in the commit message: "One point of note is that the nullability-return check is only allowed to kick in if all arguments to the function satisfy their nullability preconditions. This makes it necessary to emit some null checks in the function body itself." Maybe even rename CanCheckRetValNullability into RetValNullabilityPrecondition. I like this more than "CanCheck" because it's just a precondition value. You check if it is satisfied (evaluates to true or false) later when it's used in a branch. https://reviews.llvm.org/D30762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30762: [ubsan] Add a nullability sanitizer
vsk updated this revision to Diff 91180.
vsk added a comment.
- Add a test for the mixed _Nonnull arg + __attribute__((nonnull)) arg case.
- Reword docs per Adrian's comments, fix up doxygen comments, add better code
comments, drop redundant "Nullability" truthiness checks.
https://reviews.llvm.org/D30762
Files:
docs/UndefinedBehaviorSanitizer.rst
include/clang/Basic/Sanitizers.def
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/Driver/SanitizerArgs.cpp
lib/Driver/ToolChain.cpp
test/CodeGenObjC/ubsan-nonnull-and-nullability.m
test/CodeGenObjC/ubsan-nullability.m
Index: test/CodeGenObjC/ubsan-nullability.m
===
--- /dev/null
+++ test/CodeGenObjC/ubsan-nullability.m
@@ -0,0 +1,182 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -x objective-c -emit-llvm -triple x86_64-apple-macosx10.10.0 -fsanitize=nullability-arg,nullability-assign,nullability-return -w %s -o - | FileCheck %s
+
+// CHECK: [[NONNULL_RV_LOC1:@.*]] = private unnamed_addr global {{.*}} i32 109, i32 1 {{.*}} i32 100, i32 6
+// CHECK: [[NONNULL_ARG_LOC:@.*]] = private unnamed_addr global {{.*}} i32 204, i32 15 {{.*}} i32 190, i32 23
+// CHECK: [[NONNULL_ASSIGN1_LOC:@.*]] = private unnamed_addr global {{.*}} i32 305, i32 9
+// CHECK: [[NONNULL_ASSIGN2_LOC:@.*]] = private unnamed_addr global {{.*}} i32 405, i32 10
+// CHECK: [[NONNULL_ASSIGN3_LOC:@.*]] = private unnamed_addr global {{.*}} i32 505, i32 10
+// CHECK: [[NONNULL_INIT1_LOC:@.*]] = private unnamed_addr global {{.*}} i32 604, i32 25
+// CHECK: [[NONNULL_INIT2_LOC1:@.*]] = private unnamed_addr global {{.*}} i32 707, i32 26
+// CHECK: [[NONNULL_INIT2_LOC2:@.*]] = private unnamed_addr global {{.*}} i32 707, i32 29
+// CHECK: [[NONNULL_RV_LOC2:@.*]] = private unnamed_addr global {{.*}} i32 817, i32 1 {{.*}} i32 800, i32 6
+
+#define NULL ((void *)0)
+
+// CHECK-LABEL: define i32* @nonnull_retval1
+#line 100
+int *_Nonnull nonnull_retval1(int *p) {
+ // CHECK: br i1 true, label %[[NULL:.*]], label %[[NONULL:.*]], !nosanitize
+ // CHECK: [[NULL]]:
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* {{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_nonnull_return{{.*}}[[NONNULL_RV_LOC1]]
+ return p;
+ // CHECK: [[NONULL]]:
+ // CHECK: ret i32*
+}
+
+#line 190
+void nonnull_arg(int *_Nonnull p) {}
+
+// CHECK-LABEL: define void @call_func_with_nonnull_arg
+#line 200
+void call_func_with_nonnull_arg(int *_Nonnull p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* {{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_nonnull_arg{{.*}}[[NONNULL_ARG_LOC]]
+ nonnull_arg(p);
+}
+
+// CHECK-LABEL: define void @nonnull_assign1
+#line 300
+void nonnull_assign1(int *p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* {{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_ASSIGN1_LOC]]
+ int *_Nonnull local;
+ local = p;
+}
+
+// CHECK-LABEL: define void @nonnull_assign2
+#line 400
+void nonnull_assign2(int *p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* %{{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_ASSIGN2_LOC]]
+ int *_Nonnull arr[1];
+ arr[0] = p;
+}
+
+struct S1 {
+ int *_Nonnull mptr;
+};
+
+// CHECK-LABEL: define void @nonnull_assign3
+#line 500
+void nonnull_assign3(int *p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* %{{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_ASSIGN3_LOC]]
+ struct S1 s;
+ s.mptr = p;
+}
+
+// CHECK-LABEL: define void @nonnull_init1
+#line 600
+void nonnull_init1(int *p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* %{{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_INIT1_LOC]]
+ int *_Nonnull local = p;
+}
+
+// CHECK-LABEL: define void @nonnull_init2
+#line 700
+void nonnull_init2(int *p) {
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* %{{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_INIT2_LOC1]]
+ // CHECK: [[ICMP:%.*]] = icmp ne i32* %{{.*}}, null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch{{.*}}[[NONNULL_INIT2_LOC2]]
+ int *_Nonnull arr[] = {p, p};
+}
+
+// CHECK-LABEL: define i32* @nonnull_retval2
+#line 800
+int *_Nonnull nonnull_retval2(int *_Nonnull arg1, //< Test this.
+ int *_Nonnull arg2, //< Test this.
+ int *_Nullable arg3, //< Don't test the rest.
+
[PATCH] D30762: [ubsan] Add a nullability sanitizer
vsk marked 8 inline comments as done.
vsk added a comment.
Thanks for your comments, and sorry for jumping the gun earlier with an updated
diff. I'll attach a fixed-up diff shortly.
Comment at: lib/CodeGen/CGDecl.cpp:1911
+if (auto Nullability = Ty->getNullability(getContext())) {
+ if (Nullability && *Nullability == NullabilityKind::NonNull) {
+SanitizerScope SanScope(this);
jroelofs wrote:
> aprantl wrote:
> > Is this a typo, or do you. really want `Nullability` here? Asking because
> > everywhere else there is a check for `! Nullability && *Nullability ==
> > NullabilityKind::NonNull`.
> > Ans should this condition be factored out?
> Could just stick `if (auto Nullability =` in there, and remove the
> `Nullability &&`.
'Nullability' should not have been checked for truthiness twice. By factoring
out, do you mean moving the check's logic out of EmitParmDecl and into a
helper? If so, I'm not sure if that would make the code cleaner, but I'm open
to it.
Comment at: test/CodeGenObjC/ubsan-null-retval.m:14
+ // CHECK-NEXT: [[ICMP:%.*]] = icmp ne i32* [[ARG]], null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], label %[[CONT:.+]], label %[[HANDLE:[^,]+]]
+ // CHECK: [[HANDLE]]:
jroelofs wrote:
> The:
>
> ```
> alloca
> store
> load
> ```
>
> part of this looks like it's just making extra work for mem2reg. Is the
> alloca actually necessary?
>
>
Clang likes its allocas :). This is typical -O0 behavior: IIUC it simplifies
IRGen. E.g for expressions which take the address of an argument, because there
is guaranteed storage for the arg.
https://reviews.llvm.org/D30762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30268: Avoid copy of __atoms when char_type is char
hiraditya added a comment. ping https://reviews.llvm.org/D30268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration
kimgr added a comment. Oops, the flag is called `BreakBeforeInhertianceComma` with a misspelling throughout. May want to search for `inhertiance` throughout and see if there are more cases. Comment at: docs/ClangFormatStyleOptions.rst:531 +**BreakBeforeInhertianceComma** (``bool``) + If ``true``, in the class inheritance expression clang-format will Tyop: Inhertiance Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization
Anastasia added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+ "initialization of atomic variables is restricted to variables in global
address space">;
def err_atomic_init_constant : Error<
Could we combine this error diag with the one below? I guess they are
semantically very similar apart from one is about initialization and another is
about assignment?
Comment at: lib/Sema/SemaInit.cpp:6498
+
+ if (S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200 &&
+ ETy->isAtomicType() && !HasGlobalAS &&
I would remove S.getLangOpts().OpenCL check, it's redundant in my opinion!
Comment at: lib/Sema/SemaInit.cpp:6501
+ Entity.getKind() == InitializedEntity::EK_Variable && Args.size() > 0) {
+const Expr *Init = Args[0];
+S.Diag(Init->getLocStart(), diag::err_atomic_init_addressspace) <<
Even thought it's done above too, I don't really see the point of having this
variable.
https://reviews.llvm.org/D30643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D30773: Make git-clang-format python 3 compatible
Consider landing the obvious bits (print function, mostly) separately and
use this only for the interesting bits.
On Thu, Mar 9, 2017 at 11:10 AM, Michał Górny via Phabricator via
cfe-commits wrote:
> mgorny added inline comments.
>
>
>
> Comment at: tools/clang-format/git-clang-format:293
>
> +def to_bytes(str):
> +# Encode to UTF-8 to get binary data.
>
> Pretty much a nit but using variable names that match type names can be a
> bit confusing here.
>
>
>
> Comment at: tools/clang-format/git-clang-format:302
> +return bytes
> +return to_bytes(bytes)
> +
>
> Shouldn't this be:
>
> return bytes.decode('utf-8')
>
> ?
>
> Otherwise, unless I'm missing something this function will always return
> the parameter [with no modifications], either in the first conditional if
> it's of `str` type, or in the conditional inside `to_bytes()` if it's of
> `bytes` type.
>
>
>
> Comment at: tools/clang-format/git-clang-format:306
> +try:
> +return to_string(bytes.decode('utf-8'))
> +except AttributeError: # 'str' object has no attribute 'decode'.
>
> This logic looks really weird to me. What is the purpose of having both
> `to_string()` and `convert_string()`? Why do `to_bytes()` and `to_string()`
> use `isinstance()` to recognize types, and here you rely on exceptions? Why
> is `to_string()` called after decoding?
>
>
>
> Comment at: tools/clang-format/git-clang-format:310
> +except UnicodeError:
> +return str(bytes)
> +
>
> I don't think this can ever succeed. If the argument is not valid utf8, it
> certainly won't be valid ASCII.
>
>
>
> Comment at: tools/clang-format/git-clang-format:323
>for line in patch_file:
> -match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
> +match = re.search(to_bytes(r'^\+\+\+\ [^/]+/(.*)'), line)
> if match:
>
> Any reason not to use:
>
> br'^\+...'
>
> ? i.e. make it bytes immediately instead of converting.
>
>
>
> Comment at: tools/clang-format/git-clang-format:344
> + keys_to_delete = []
> + for filename in list(dictionary.keys()):
> +filename_cp = convert_string(bytes(filename))
>
> Since you are using `keys_to_delete` now, you can remove the `list()`.
>
>
>
> Comment at: tools/clang-format/git-clang-format:348
> if len(base_ext) == 1 or base_ext[1].lower() not in
> allowed_extensions:
> - del dictionary[filename]
> + keys_to_delete += [filename]
> + for key in keys_to_delete:
>
> Another nit. I think it'd be better to just append a single item instead
> of a list of 1 item ;-).
>
> keys_to_delete.append(filename)
>
>
> https://reviews.llvm.org/D30773
>
>
>
> ___
> 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
[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. A few nits, otherwise looks good. Comment at: include/clang/Format/Format.h:426 + /// \brief If ``true``, in the class inheritance expression clang-format will + /// break before operands ``:`` and ``,`` only if there is multiple + /// inheritance. Please remove "operands" and "only". I think they can be confusing. Comment at: include/clang/Format/Format.h:852 ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas && + BreakBeforeInhertianceComma == + R.BreakBeforeInhertianceComma && Looks like it might fit on one line now :) Comment at: lib/Format/ContinuationIndenter.cpp:355 + if (Current.is(TT_InheritanceColon)) +State.Stack.back().NoLineBreak = true; Can you leave a comment here: // Don't break within the inheritance declaration unless the ":" is on a new line. Comment at: lib/Format/ContinuationIndenter.cpp:750 + if (NextNonComment->isOneOf(TT_InheritanceColon, TT_InheritanceComma)) +return State.FirstIndent + Style.ContinuationIndentWidth; if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() && Please merge these into the one about TT_CtorInitializerComma, i.e.: if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon, TT_InheritanceComma)) return State.FirstIndent + Style.ConstructorInitializerIndentWidth; Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r297389 - Retry: [ubsan] Detect UB loads from bitfields
Author: vedantk
Date: Thu Mar 9 10:06:27 2017
New Revision: 297389
URL: http://llvm.org/viewvc/llvm-project?rev=297389&view=rev
Log:
Retry: [ubsan] Detect UB loads from bitfields
It's possible to load out-of-range values from bitfields backed by a
boolean or an enum. Check for UB loads from bitfields.
This is the motivating example:
struct S {
BOOL b : 1; // Signed ObjC BOOL.
};
S s;
s.b = 1; // This is actually stored as -1.
if (s.b == 1) // Evaluates to false, -1 != 1.
...
Changes since the original commit:
- Single-bit bools are a special case (see CGF::EmitFromMemory), and we
can't avoid dealing with them when loading from a bitfield. Don't try to
insert a check in this case.
Differential Revision: https://reviews.llvm.org/D30423
Added:
cfe/trunk/test/CodeGenCXX/ubsan-bitfields.cpp
Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenObjC/ubsan-bool.m
Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=297389&r1=297388&r2=297389&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Thu Mar 9 10:06:27 2017
@@ -1181,7 +1181,7 @@ RValue AtomicInfo::convertAtomicTempToRV
if (LVal.isBitField())
return CGF.EmitLoadOfBitfieldLValue(
LValue::MakeBitfield(addr, LVal.getBitFieldInfo(), LVal.getType(),
- LVal.getAlignmentSource()));
+ LVal.getAlignmentSource()), loc);
if (LVal.isVectorElt())
return CGF.EmitLoadOfLValue(
LValue::MakeVectorElt(addr, LVal.getVectorIdx(), LVal.getType(),
Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=297389&r1=297388&r2=297389&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu Mar 9 10:06:27 2017
@@ -1327,6 +1327,13 @@ bool CodeGenFunction::EmitScalarRangeChe
if (!NeedsBoolCheck && !NeedsEnumCheck)
return false;
+ // Single-bit booleans don't need to be checked. Special-case this to avoid
+ // a bit width mismatch when handling bitfield values. This is handled by
+ // EmitFromMemory for the non-bitfield case.
+ if (IsBool &&
+ cast(Value->getType())->getBitWidth() == 1)
+return false;
+
llvm::APInt Min, End;
if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool))
return true;
@@ -1549,10 +1556,11 @@ RValue CodeGenFunction::EmitLoadOfLValue
return EmitLoadOfGlobalRegLValue(LV);
assert(LV.isBitField() && "Unknown LValue type!");
- return EmitLoadOfBitfieldLValue(LV);
+ return EmitLoadOfBitfieldLValue(LV, Loc);
}
-RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV) {
+RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
+ SourceLocation Loc) {
const CGBitFieldInfo &Info = LV.getBitFieldInfo();
// Get the output type.
@@ -1577,7 +1585,7 @@ RValue CodeGenFunction::EmitLoadOfBitfie
"bf.clear");
}
Val = Builder.CreateIntCast(Val, ResLTy, Info.IsSigned, "bf.cast");
-
+ EmitScalarRangeCheck(Val, LV.getType(), Loc);
return RValue::get(Val);
}
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=297389&r1=297388&r2=297389&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Mar 9 10:06:27 2017
@@ -2943,7 +2943,7 @@ public:
/// rvalue, returning the rvalue.
RValue EmitLoadOfLValue(LValue V, SourceLocation Loc);
RValue EmitLoadOfExtVectorElementLValue(LValue V);
- RValue EmitLoadOfBitfieldLValue(LValue LV);
+ RValue EmitLoadOfBitfieldLValue(LValue LV, SourceLocation Loc);
RValue EmitLoadOfGlobalRegLValue(LValue LV);
/// EmitStoreThroughLValue - Store the specified rvalue into the specified
Added: cfe/trunk/test/CodeGenCXX/ubsan-bitfields.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-bitfields.cpp?rev=297389&view=auto
==
--- cfe/trunk/test/CodeGenCXX/ubsan-bitfields.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ubsan-bitfields.cpp Thu Mar 9 10:06:27 2017
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s
-fsanitize=bool,enum | FileCheck %s
+
+enum E {
+ a = 1,
+ b = 2,
+ c = 3
+};
+
+struct S {
+ E e1 : 10;
+};
+
+// CHECK-LABEL: define i32 @_Z4loadP1S
+E load(S *s) {
+ // CHECK: [[LOAD:%.*]] = load i16, i
[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration
Abpostelnicu updated this revision to Diff 91174.
Abpostelnicu added a comment.
Yes that's a better name :)
Repository:
rL LLVM
https://reviews.llvm.org/D30487
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1029,6 +1029,17 @@
verifyFormat("class ::A::B {};");
}
+TEST_F(FormatTest, BreakBeforeInhertianceComma) {
+ FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
+ LocalStyle.BreakBeforeInhertianceComma = true;
+
+ verifyFormat("class MyClass : public X {};", LocalStyle);
+ verifyFormat("class MyClass\n"
+ ": public X\n"
+ ", public Y {};",
+ StyleWithInheritanceBreak);
+}
+
TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
verifyFormat("class A {\n} a, b;");
verifyFormat("struct A {\n} a, b;");
@@ -8582,6 +8593,7 @@
CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
CHECK_PARSE_BOOL(BreakStringLiterals);
+ CHECK_PARSE_BOOL(BreakBeforeInhertianceComma)
CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
CHECK_PARSE_BOOL(DerivePointerAlignment);
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -676,6 +676,8 @@
case tok::comma:
if (Contexts.back().InCtorInitializer)
Tok->Type = TT_CtorInitializerComma;
+ else if (Contexts.back().InInhertianceList)
+Tok->Type = TT_InheritanceComma;
else if (Contexts.back().FirstStartOfName &&
(Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
@@ -945,6 +947,7 @@
bool CanBeExpression = true;
bool InTemplateArgument = false;
bool InCtorInitializer = false;
+bool InInhertianceList = false;
bool CaretFound = false;
bool IsForEachMacro = false;
};
@@ -1004,6 +1007,9 @@
Current.Previous->is(TT_CtorInitializerColon)) {
Contexts.back().IsExpression = true;
Contexts.back().InCtorInitializer = true;
+} else if (Current.Previous &&
+ Current.Previous->is(TT_InheritanceColon)) {
+ Contexts.back().InInhertianceList = true;
} else if (Current.isOneOf(tok::r_paren, tok::greater, tok::comma)) {
for (FormatToken *Previous = Current.Previous;
Previous && Previous->isOneOf(tok::star, tok::amp);
@@ -2474,6 +2480,10 @@
Style.BreakConstructorInitializersBeforeComma &&
!Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
return true;
+ // Break only if we have multiple inheritance.
+ if (Style.BreakBeforeInhertianceComma &&
+ Right.is(TT_InheritanceComma))
+ return true;
if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
// Raw string literals are special wrt. line breaks. The author has made a
// deliberate choice and might have aligned the contents of the string
@@ -2653,6 +2663,12 @@
if (Right.is(TT_CtorInitializerComma) &&
Style.BreakConstructorInitializersBeforeComma)
return true;
+ if (Left.is(TT_InheritanceComma) &&
+ Style.BreakBeforeInhertianceComma)
+return false;
+ if (Right.is(TT_InheritanceComma) &&
+ Style.BreakBeforeInhertianceComma)
+return true;
if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
(Left.is(tok::less) && Right.is(tok::less)))
return false;
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -48,6 +48,7 @@
TYPE(FunctionTypeLParen) \
TYPE(ImplicitStringLiteral) \
TYPE(InheritanceColon) \
+ TYPE(InheritanceComma) \
TYPE(InlineASMBrace) \
TYPE(InlineASMColon) \
TYPE(JavaAnnotation) \
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -521,6 +521,7 @@
false, false, false, false, false};
LLVMStyle.BreakAfterJavaFieldAnnotations = false;
LLVMStyle.BreakConstructorInitializersBeforeComma = false;
+ LLVMStyle.BreakBeforeInhertianceComma = false;
LLVMStyle.BreakStringLiterals = true;
LLVMStyle.ColumnLimit = 80;
LLVMStyle.CommentPragmas = "^ IWYU pragma:";
@@ -674,6 +675,7 @@
MozillaStyle.BinPackArguments = false;
MozillaStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
MozillaStyle.BreakConstructor
[PATCH] D30773: Make git-clang-format python 3 compatible
mgorny added inline comments.
Comment at: tools/clang-format/git-clang-format:293
+def to_bytes(str):
+# Encode to UTF-8 to get binary data.
Pretty much a nit but using variable names that match type names can be a bit
confusing here.
Comment at: tools/clang-format/git-clang-format:302
+return bytes
+return to_bytes(bytes)
+
Shouldn't this be:
return bytes.decode('utf-8')
?
Otherwise, unless I'm missing something this function will always return the
parameter [with no modifications], either in the first conditional if it's of
`str` type, or in the conditional inside `to_bytes()` if it's of `bytes` type.
Comment at: tools/clang-format/git-clang-format:306
+try:
+return to_string(bytes.decode('utf-8'))
+except AttributeError: # 'str' object has no attribute 'decode'.
This logic looks really weird to me. What is the purpose of having both
`to_string()` and `convert_string()`? Why do `to_bytes()` and `to_string()` use
`isinstance()` to recognize types, and here you rely on exceptions? Why is
`to_string()` called after decoding?
Comment at: tools/clang-format/git-clang-format:310
+except UnicodeError:
+return str(bytes)
+
I don't think this can ever succeed. If the argument is not valid utf8, it
certainly won't be valid ASCII.
Comment at: tools/clang-format/git-clang-format:323
for line in patch_file:
-match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
+match = re.search(to_bytes(r'^\+\+\+\ [^/]+/(.*)'), line)
if match:
Any reason not to use:
br'^\+...'
? i.e. make it bytes immediately instead of converting.
Comment at: tools/clang-format/git-clang-format:344
+ keys_to_delete = []
+ for filename in list(dictionary.keys()):
+filename_cp = convert_string(bytes(filename))
Since you are using `keys_to_delete` now, you can remove the `list()`.
Comment at: tools/clang-format/git-clang-format:348
if len(base_ext) == 1 or base_ext[1].lower() not in allowed_extensions:
- del dictionary[filename]
+ keys_to_delete += [filename]
+ for key in keys_to_delete:
Another nit. I think it'd be better to just append a single item instead of a
list of 1 item ;-).
keys_to_delete.append(filename)
https://reviews.llvm.org/D30773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration
djasper added a comment. I think the patch is fine, except for the name of the flag. It is not breaking inheritance ;). Maybe BreakBeforeInhertianceColonAndComma, but that's pretty long still. I think maybe we can shorten this to BreakBeforeInhertianceComma, as it never makes sense to break before the comma if we keep the ":" on the old line. What do you think? Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30762: [ubsan] Add a nullability sanitizer
jroelofs added inline comments.
Comment at: lib/CodeGen/CGDecl.cpp:1911
+if (auto Nullability = Ty->getNullability(getContext())) {
+ if (Nullability && *Nullability == NullabilityKind::NonNull) {
+SanitizerScope SanScope(this);
aprantl wrote:
> Is this a typo, or do you. really want `Nullability` here? Asking because
> everywhere else there is a check for `! Nullability && *Nullability ==
> NullabilityKind::NonNull`.
> Ans should this condition be factored out?
Could just stick `if (auto Nullability =` in there, and remove the `Nullability
&&`.
Comment at: lib/CodeGen/CodeGenFunction.cpp:829
+if (auto Nullability = FnRetTy->getNullability(getContext()))
+ if (Nullability && *Nullability == NullabilityKind::NonNull)
+if (!(SanOpts.has(SanitizerKind::ReturnsNonnullAttribute) &&
Hasn't the `if (auto Nullability =` already checked the truthiness of the var,
making the `Nullability &&` part of the second check redundant?
Comment at: test/CodeGenObjC/ubsan-null-retval.m:14
+ // CHECK-NEXT: [[ICMP:%.*]] = icmp ne i32* [[ARG]], null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], label %[[CONT:.+]], label %[[HANDLE:[^,]+]]
+ // CHECK: [[HANDLE]]:
The:
```
alloca
store
load
```
part of this looks like it's just making extra work for mem2reg. Is the alloca
actually necessary?
https://reviews.llvm.org/D30762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30776: [coroutines] Fix diagnostics depending on the first coroutine statement.
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D30776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.
madsravn added a comment. Any updates on this? https://reviews.llvm.org/D30158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration
Abpostelnicu added a comment. Daniel what do you think about the last version of the patch? I don't want to have this stalled since we really need this modifications in order to be able to run clang-format on our repository. Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30777: Added `applyAtomicChanges` function.
ioeric added inline comments.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:147-149
+/// This completely ignores the file path in each change and replaces them with
+/// \p FilePath, i.e. callers are responsible for ensuring all changes are for
+/// the same file.
klimek wrote:
> Why?
Changes might have equivalent paths in different forms (e.g. relative or
absolute), and checking canonical paths are dependent on file systems and the
working directory, so I think this should be checked/handled in the user level.
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:106
+ llvm::SmallVector Lines;
+ Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
+ for (llvm::StringRef Line : Lines) {
klimek wrote:
> Won't this be incorrect if the last line (no \n) is 1 over the column limit
> (due to EndPos = Code.size()-1 above)?
Yeah, that is a bug. Should be `Code.size()` then.
https://reviews.llvm.org/D30777
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30777: Added `applyAtomicChanges` function.
ioeric updated this revision to Diff 91163.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Addressed comments; stylized code.
https://reviews.llvm.org/D30777
Files:
include/clang/Tooling/Refactoring/AtomicChange.h
lib/Tooling/Refactoring/AtomicChange.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -1290,5 +1290,435 @@
Replacement(Context.Sources, SourceLocation(), 0, "b")));
}
+class ApplyAtomicChangesTest : public ::testing::Test {
+protected:
+ ApplyAtomicChangesTest() : FilePath("file.cc") {
+Spec.Cleanup = true;
+Spec.Format = ApplyChangesSpec::kAll;
+Spec.Style = format::getLLVMStyle();
+ }
+
+ ~ApplyAtomicChangesTest() override {}
+
+ void setInput(llvm::StringRef Input) {
+Code = Input;
+FID = Context.createInMemoryFile(FilePath, Code);
+ }
+
+ SourceLocation getLoc(unsigned Offset) const {
+return Context.Sources.getLocForStartOfFile(FID).getLocWithOffset(Offset);
+ }
+
+ AtomicChange replacementToAtomicChange(llvm::StringRef Key, unsigned Offset,
+ unsigned Length,
+ llvm::StringRef Text) {
+AtomicChange Change(FilePath, Key);
+llvm::Error Err =
+Change.replace(Context.Sources, getLoc(Offset), Length, Text);
+EXPECT_FALSE(Err);
+return Change;
+ }
+
+ std::string rewrite(bool FailureExpected = false) {
+llvm::Expected ChangedCode =
+applyAtomicChanges(FilePath, Code, Changes, Spec);
+EXPECT_EQ(FailureExpected, !ChangedCode);
+if (!ChangedCode) {
+ llvm::errs() << "Failed to apply changes: "
+ << llvm::toString(ChangedCode.takeError()) << "\n";
+ return "";
+}
+return *ChangedCode;
+ }
+
+ RewriterTestContext Context;
+ FileID FID;
+ ApplyChangesSpec Spec;
+ std::string Code;
+ std::string FilePath;
+ llvm::SmallVector Changes;
+};
+
+TEST_F(ApplyAtomicChangesTest, BasicRefactoring) {
+ setInput("int a;");
+ AtomicChange Change(FilePath, "key1");
+ Changes.push_back(replacementToAtomicChange("key1", 4, 1, "b"));
+ EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, SeveralRefactorings) {
+ setInput("int a;\n"
+ "int b;");
+ Changes.push_back(replacementToAtomicChange("key1", 0, 3, "float"));
+ Changes.push_back(replacementToAtomicChange("key2", 4, 1, "f"));
+ Changes.push_back(replacementToAtomicChange("key3", 11, 1, "g"));
+ Changes.push_back(replacementToAtomicChange("key4", 7, 3, "float"));
+ EXPECT_EQ("float f;\n"
+"float g;",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, IgnorePathsInRefactorings) {
+ setInput("int a;\n"
+ "int b;");
+ Changes.push_back(replacementToAtomicChange("key1", 4, 1, "aa"));
+
+ FileID ID = Context.createInMemoryFile("AnotherFile", "12345678912345");
+ Changes.emplace_back("AnotherFile", "key2");
+ auto Err = Changes.back().replace(
+ Context.Sources,
+ Context.Sources.getLocForStartOfFile(ID).getLocWithOffset(11), 1, "bb");
+ ASSERT_TRUE(!Err);
+ EXPECT_EQ("int aa;\n"
+"int bb;",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, AppliesDuplicateInsertions) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 0, "b"));
+ Changes.push_back(replacementToAtomicChange("key2", 5, 0, "b"));
+ EXPECT_EQ("int abb;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, BailsOnOverlappingRefactorings) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 0, 5, "float f"));
+ Changes.push_back(replacementToAtomicChange("key2", 4, 1, "b"));
+ EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
+}
+
+TEST_F(ApplyAtomicChangesTest, BasicReformatting) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 1, "b"));
+ EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, OnlyFormatWhenViolateColumnLimits) {
+ Spec.Format = ApplyChangesSpec::kViolations;
+ Spec.Style.ColumnLimit = 8;
+ setInput("int a;\n"
+ "inta;\n"
+ "int ;\n");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 1, "x"));
+ Changes.push_back(replacementToAtomicChange("key2", 15, 1, "x"));
+ Changes.push_back(replacementToAtomicChange("key3", 23, 8, "xx"));
+ EXPECT_EQ("int x;\n"
+"int x;\n"
+"int xx;\n",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, LastLineViolateColumnLimits) {
+ Spec.Format = ApplyChangesSpec::kViolations;
+ Spec.Style.ColumnLimit = 8;
+ setInput("int a;\n"
+ "inta;");
+ Changes.push_back(replacementToAtomicChange("key1", 0, 1, "i"));
+ Changes.push_back(replacementToAtomicChange("key2", 15, 2, "y;"));
+ EXPECT_EQ("int a;\n"
+"int y;",
+
[PATCH] D30693: [mips][msa] Remove range checks for non-immediate sld.[bhwd] instructions
smaksimovic updated this revision to Diff 91158.
smaksimovic added a comment.
Removed whitespace.
https://reviews.llvm.org/D30693
Files:
lib/Sema/SemaChecking.cpp
test/CodeGen/builtins-mips-msa-error.c
test/CodeGen/builtins-mips-msa.c
Index: test/CodeGen/builtins-mips-msa.c
===
--- test/CodeGen/builtins-mips-msa.c
+++ test/CodeGen/builtins-mips-msa.c
@@ -699,6 +699,11 @@
v4i32_r = __msa_sld_w(v4i32_r, v4i32_a, 3); // CHECK: call <4 x i32>
@llvm.mips.sld.w(
v2i64_r = __msa_sld_d(v2i64_r, v2i64_a, 1); // CHECK: call <2 x i64>
@llvm.mips.sld.d(
+ v16i8_r = __msa_sld_b(v16i8_r, v16i8_a, 16); // CHECK: call <16 x i8>
@llvm.mips.sld.b(
+ v8i16_r = __msa_sld_h(v8i16_r, v8i16_a, 8); // CHECK: call <8 x i16>
@llvm.mips.sld.h(
+ v4i32_r = __msa_sld_w(v4i32_r, v4i32_a, 4); // CHECK: call <4 x i32>
@llvm.mips.sld.w(
+ v2i64_r = __msa_sld_d(v2i64_r, v2i64_a, 2); // CHECK: call <2 x i64>
@llvm.mips.sld.d(
+
v16i8_r = __msa_sldi_b(v16i8_r, v16i8_a, 7); // CHECK: call <16 x i8>
@llvm.mips.sldi.b(
v8i16_r = __msa_sldi_h(v8i16_r, v8i16_a, 3); // CHECK: call <8 x i16>
@llvm.mips.sldi.h(
v4i32_r = __msa_sldi_w(v4i32_r, v4i32_a, 2); // CHECK: call <4 x i32>
@llvm.mips.sldi.w(
Index: test/CodeGen/builtins-mips-msa-error.c
===
--- test/CodeGen/builtins-mips-msa-error.c
+++ test/CodeGen/builtins-mips-msa-error.c
@@ -162,11 +162,6 @@
v8i16_r = __msa_shf_h(v8i16_a, 256); // CHECK: warning:
argument should be a value from 0 to 255}}
v4i32_r = __msa_shf_w(v4i32_a, 256); // CHECK: warning:
argument should be a value from 0 to 255}}
- v16i8_r = __msa_sld_b(v16i8_r, v16i8_a, 16); // expected-error
{{argument should be a value from 0 to 15}}
- v8i16_r = __msa_sld_h(v8i16_r, v8i16_a, 8); // expected-error
{{argument should be a value from 0 to 7}}
- v4i32_r = __msa_sld_w(v4i32_r, v4i32_a, 4); // expected-error
{{argument should be a value from 0 to 3}}
- v2i64_r = __msa_sld_d(v2i64_r, v2i64_a, 2); // expected-error
{{argument should be a value from 0 to 1}}
-
v16i8_r = __msa_sldi_b(v16i8_r, v16i8_a, 16); // expected-error
{{argument should be a value from 0 to 15}}
v8i16_r = __msa_sldi_h(v8i16_r, v8i16_a, 8); // expected-error
{{argument should be a value from 0 to 7}}
v4i32_r = __msa_sldi_w(v4i32_r, v4i32_a, 4); // expected-error
{{argument should be a value from 0 to 3}}
@@ -358,11 +353,6 @@
v8i16_r = __msa_shf_h(v8i16_a, -1);// CHECK: warning:
argument should be a value from 0 to 255}}
v4i32_r = __msa_shf_w(v4i32_a, -1);// CHECK: warning:
argument should be a value from 0 to 255}}
- v16i8_r = __msa_sld_b(v16i8_r, v16i8_a, -17); // expected-error
{{argument should be a value from 0 to 15}}
- v8i16_r = __msa_sld_h(v8i16_r, v8i16_a, -8); // expected-error
{{argument should be a value from 0 to 7}}
- v4i32_r = __msa_sld_w(v4i32_r, v4i32_a, -4); // expected-error
{{argument should be a value from 0 to 3}}
- v2i64_r = __msa_sld_d(v2i64_r, v2i64_a, -2); // expected-error
{{argument should be a value from 0 to 1}}
-
v16i8_r = __msa_sldi_b(v16i8_r, v16i8_a, -17); // expected-error
{{argument should be a value from 0 to 15}}
v8i16_r = __msa_sldi_h(v8i16_r, v8i16_a, -8); // expected-error
{{argument should be a value from 0 to 7}}
v4i32_r = __msa_sldi_w(v4i32_r, v4i32_a, -4); // expected-error
{{argument should be a value from 0 to 3}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1619,28 +1619,24 @@
case Mips::BI__builtin_msa_copy_u_b:
case Mips::BI__builtin_msa_insve_b:
case Mips::BI__builtin_msa_splati_b: i = 1; l = 0; u = 15; break;
- case Mips::BI__builtin_msa_sld_b:
case Mips::BI__builtin_msa_sldi_b: i = 2; l = 0; u = 15; break;
// These intrinsics take an unsigned 3 bit immediate.
case Mips::BI__builtin_msa_copy_s_h:
case Mips::BI__builtin_msa_copy_u_h:
case Mips::BI__builtin_msa_insve_h:
case Mips::BI__builtin_msa_splati_h: i = 1; l = 0; u = 7; break;
- case Mips::BI__builtin_msa_sld_h:
case Mips::BI__builtin_msa_sldi_h: i = 2; l = 0; u = 7; break;
// These intrinsics take an unsigned 2 bit immediate.
case Mips::BI__builtin_msa_copy_s_w:
case Mips::BI__builtin_msa_copy_u_w:
case Mips::BI__builtin_msa_insve_w:
case Mips::BI__builtin_msa_splati_w: i = 1; l = 0; u = 3; break;
- case Mips::BI__builtin_msa_sld_w:
case Mips::BI__builtin_msa_sldi_w: i = 2; l = 0; u = 3; break;
// These intrinsics take an unsigned 1 bit immediate.
case Mips::BI__builtin_msa_copy_s_d:
case Mips::BI__builtin_msa_copy_u_d:
case Mips::BI__builtin_msa_insve_d:
case Mips::BI__builtin_msa_splati_d: i = 1; l = 0; u = 1;
[PATCH] D30777: Added `applyAtomicChanges` function.
klimek added inline comments.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:129
+struct ApplyChangesSpec {
+ // If true, cleans up redundant/erroneous around changed code with
+ // clang-format's cleanup functionality, e.g. redundant commas around deleted
That part of the sentence doesn't parse.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:132
+ // parameter or empty namespaces introduced by deletions.
+ bool Cleanup;
+
I'd add default values for everything that will otherwise be uninitialized.
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:147-149
+/// This completely ignores the file path in each change and replaces them with
+/// \p FilePath, i.e. callers are responsible for ensuring all changes are for
+/// the same file.
Why?
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:106
+ llvm::SmallVector Lines;
+ Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
+ for (llvm::StringRef Line : Lines) {
Won't this be incorrect if the last line (no \n) is 1 over the column limit
(due to EndPos = Code.size()-1 above)?
https://reviews.llvm.org/D30777
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30748: [Lexer] Finding beginning of token with escaped new line
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Lex/Lexer.cpp:457
+static bool isNewLineEscaped(const char *BufferStart, const char *Str) {
+ while (Str > BufferStart && isWhitespace(*Str))
+--Str;
We only care about two specific sequences here: `\\\r\n` or `\\\n`, not a
backslash followed by arbitrary whitespace.
Comment at: unittests/Lex/LexerTest.cpp:386
+ // further offset calculation to be more straightforward.
+ const auto IdentifierLength = 8;
+ std::string textToLex =
LLVM doesn't use "almost always auto" style. See
http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Comment at: unittests/Lex/LexerTest.cpp:387
+ const auto IdentifierLength = 8;
+ std::string textToLex =
+"rabarbar\n"
Variable names should start with a capital letter: `TextToLex`. Same elsewhere.
Comment at: unittests/Lex/LexerTest.cpp:402
+
+ auto foundLocation = SourceMgr.getDecomposedExpansionLoc(
+ Lexer::GetBeginningOfToken(
Please clang-format.
https://reviews.llvm.org/D30748
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30777: Added `applyAtomicChanges` function.
ioeric created this revision.
... which applies a set of `AtomicChange`s on code.
https://reviews.llvm.org/D30777
Files:
include/clang/Tooling/Refactoring/AtomicChange.h
lib/Tooling/Refactoring/AtomicChange.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -1290,5 +1290,411 @@
Replacement(Context.Sources, SourceLocation(), 0, "b")));
}
+class ApplyAtomicChangesTest : public ::testing::Test {
+protected:
+ ApplyAtomicChangesTest() : FilePath("file.cc") {
+Spec.Cleanup = true;
+Spec.Format = ApplyChangesSpec::kAll;
+Spec.Style = format::getLLVMStyle();
+ }
+
+ ~ApplyAtomicChangesTest() override {}
+
+ void setInput(llvm::StringRef Input) {
+Code = Input;
+FID = Context.createInMemoryFile(FilePath, Code);
+ }
+
+ SourceLocation getLoc(unsigned Offset) const {
+return Context.Sources.getLocForStartOfFile(FID).getLocWithOffset(Offset);
+ }
+
+ AtomicChange replacementToAtomicChange(llvm::StringRef Key, unsigned Offset,
+ unsigned Length,
+ llvm::StringRef Text) {
+AtomicChange Change(FilePath, Key);
+llvm::Error Err =
+Change.replace(Context.Sources, getLoc(Offset), Length, Text);
+EXPECT_FALSE(Err);
+return Change;
+ }
+
+ std::string rewrite(bool FailureExpected = false) {
+llvm::Expected ChangedCode =
+applyAtomicChanges(FilePath, Code, Changes, Spec);
+EXPECT_EQ(FailureExpected, !ChangedCode);
+if (!ChangedCode) {
+ llvm::errs() << "Failed to apply changes: "
+ << llvm::toString(ChangedCode.takeError()) << "\n";
+ return "";
+}
+return *ChangedCode;
+ }
+
+ RewriterTestContext Context;
+ FileID FID;
+ ApplyChangesSpec Spec;
+ std::string Code;
+ std::string FilePath;
+ llvm::SmallVector Changes;
+};
+
+TEST_F(ApplyAtomicChangesTest, BasicRefactoring) {
+ setInput("int a;");
+ AtomicChange Change(FilePath, "key1");
+ Changes.push_back(replacementToAtomicChange("key1", 4, 1, "b"));
+ EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, SeveralRefactorings) {
+ setInput("int a;\n"
+ "int b;");
+ Changes.push_back(replacementToAtomicChange("key1", 0, 3, "float"));
+ Changes.push_back(replacementToAtomicChange("key2", 4, 1, "f"));
+ Changes.push_back(replacementToAtomicChange("key3", 11, 1, "g"));
+ Changes.push_back(replacementToAtomicChange("key4", 7, 3, "float"));
+ EXPECT_EQ("float f;\n"
+"float g;",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, IgnorePathsInRefactorings) {
+ setInput("int a;\n"
+ "int b;");
+ Changes.push_back(replacementToAtomicChange("key1", 4, 1, "aa"));
+
+ FileID ID = Context.createInMemoryFile("AnotherFile", "12345678912345");
+ Changes.emplace_back("AnotherFile", "key2");
+ auto Err = Changes.back().replace(
+ Context.Sources,
+ Context.Sources.getLocForStartOfFile(ID).getLocWithOffset(11), 1, "bb");
+ ASSERT_TRUE(!Err);
+ EXPECT_EQ("int aa;\n"
+"int bb;",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, AppliesDuplicateInsertions) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 0, "b"));
+ Changes.push_back(replacementToAtomicChange("key2", 5, 0, "b"));
+ EXPECT_EQ("int abb;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, BailsOnOverlappingRefactorings) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 0, 5, "float f"));
+ Changes.push_back(replacementToAtomicChange("key2", 4, 1, "b"));
+ EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
+}
+
+TEST_F(ApplyAtomicChangesTest, BasicReformatting) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 1, "b"));
+ EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, OnlyFormatWhenViolateColumnLimits) {
+ Spec.Format = ApplyChangesSpec::kViolations;
+ Spec.Style.ColumnLimit = 8;
+ setInput("int a;\n"
+ "inta;\n"
+ "int ;\n");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 1, "x"));
+ Changes.push_back(replacementToAtomicChange("key2", 15, 1, "x"));
+ Changes.push_back(replacementToAtomicChange("key3", 23, 8, "xx"));
+ EXPECT_EQ("int x;\n"
+"int x;\n"
+"int xx;\n",
+rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, Longer) {
+ setInput("int a;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 1, "bbb"));
+ EXPECT_EQ("int bbb;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, Shorter) {
+ setInput("int aaa;");
+ Changes.push_back(replacementToAtomicChange("key1", 5, 3, "b"));
+ EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, OnlyFormatChangedLines) {
+ setInput("int aaa;\n"
+
[PATCH] D30685: [include-fixer] Remove line number from Symbol identity
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297371: [include-fixer] Remove line number from Symbol
identity (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D30685?vs=90990&id=91149#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30685
Files:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
clang-tools-extra/trunk/test/include-fixer/Inputs/merge/a.yaml
clang-tools-extra/trunk/test/include-fixer/Inputs/merge/b.yaml
clang-tools-extra/trunk/test/include-fixer/merge.test
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -333,8 +333,7 @@
: "\"" + FilePath + "\""),
SourceManager, HeaderSearch);
SymbolCandidates.emplace_back(Symbol.getName(), Symbol.getSymbolKind(),
- MinimizedFilePath, Symbol.getLineNumber(),
- Symbol.getContexts());
+ MinimizedFilePath, Symbol.getContexts());
}
return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates);
}
Index: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
@@ -178,7 +178,7 @@
for (size_t I = 0, E = CommaSplits.size(); I != E; ++I)
Symbols.push_back(
{SymbolInfo(Split.first.trim(), SymbolInfo::SymbolKind::Unknown,
-CommaSplits[I].trim(), 1, {}),
+CommaSplits[I].trim(), {}),
// Use fake "seen" signal for tests, so first header wins.
SymbolInfo::Signals(/*Seen=*/static_cast(E - I),
/*Used=*/0)});
Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
@@ -20,7 +20,11 @@
namespace clang {
namespace find_all_symbols {
-/// \brief Contains all information for a Symbol.
+/// \brief Describes a named symbol from a header.
+/// Symbols with the same qualified name and type (e.g. function overloads)
+/// that appear in the same header are represented by a single SymbolInfo.
+///
+/// TODO: keep track of instances, e.g. overload locations and signatures.
class SymbolInfo {
public:
/// \brief The SymbolInfo Type.
@@ -66,10 +70,10 @@
// The default constructor is required by YAML traits in
// LLVM_YAML_IS_DOCUMENT_LIST_VECTOR.
- SymbolInfo() : Type(SymbolKind::Unknown), LineNumber(-1) {}
+ SymbolInfo() : Type(SymbolKind::Unknown) {}
SymbolInfo(llvm::StringRef Name, SymbolKind Type, llvm::StringRef FilePath,
- int LineNumber, const std::vector &Contexts);
+ const std::vector &Contexts);
void SetFilePath(llvm::StringRef Path) { FilePath = Path; }
@@ -90,9 +94,6 @@
return Contexts;
}
- /// \brief Get a 1-based line number of the symbol's declaration.
- int getLineNumber() const { return LineNumber; }
-
bool operator<(const SymbolInfo &Symbol) const;
bool operator==(const SymbolInfo &Symbol) const;
@@ -121,9 +122,6 @@
///
/// If the symbol is declared in `TranslationUnitDecl`, it has no context.
std::vector Contexts;
-
- /// \brief The 1-based line number of of the symbol's declaration.
- int LineNumber;
};
struct SymbolAndSignals {
Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -109,8 +109,7 @@
std::string FilePath = getIncludePath(SM, Loc, Collector);
if (FilePath.empty()) return llvm::None;
- return SymbolInfo(ND->getNameAsString(), Type, FilePath,
-SM.getExpansionLineNumbe
[clang-tools-extra] r297371 - [include-fixer] Remove line number from Symbol identity
Author: sammccall
Date: Thu Mar 9 04:47:44 2017
New Revision: 297371
URL: http://llvm.org/viewvc/llvm-project?rev=297371&view=rev
Log:
[include-fixer] Remove line number from Symbol identity
Summary:
Remove line number from Symbol identity.
For our purposes (include-fixer and clangd autocomplete), function overloads
within the same header should mostly be treated as a single combined symbol.
We may want to track individual occurrences (line number, full type info)
and aggregate this during mapreduce, but that's not done here.
Reviewers: hokein, bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D30685
Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
clang-tools-extra/trunk/test/include-fixer/Inputs/merge/a.yaml
clang-tools-extra/trunk/test/include-fixer/Inputs/merge/b.yaml
clang-tools-extra/trunk/test/include-fixer/merge.test
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=297371&r1=297370&r2=297371&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Thu Mar 9 04:47:44
2017
@@ -333,8 +333,7 @@ IncludeFixerContext IncludeFixerSemaSour
: "\"" + FilePath + "\""),
SourceManager, HeaderSearch);
SymbolCandidates.emplace_back(Symbol.getName(), Symbol.getSymbolKind(),
- MinimizedFilePath, Symbol.getLineNumber(),
- Symbol.getContexts());
+ MinimizedFilePath, Symbol.getContexts());
}
return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates);
}
Modified:
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp?rev=297371&r1=297370&r2=297371&view=diff
==
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp
(original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllMacros.cpp
Thu Mar 9 04:47:44 2017
@@ -28,8 +28,7 @@ FindAllMacros::CreateMacroSymbol(const T
if (FilePath.empty())
return llvm::None;
return SymbolInfo(MacroNameTok.getIdentifierInfo()->getName(),
-SymbolInfo::SymbolKind::Macro, FilePath,
-SM->getSpellingLineNumber(info->getDefinitionLoc()), {});
+SymbolInfo::SymbolKind::Macro, FilePath, {});
}
void FindAllMacros::MacroDefined(const Token &MacroNameTok,
Modified:
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp?rev=297371&r1=297370&r2=297371&view=diff
==
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
(original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
Thu Mar 9 04:47:44 2017
@@ -109,8 +109,7 @@ CreateSymbolInfo(const NamedDecl *ND, co
std::string FilePath = getIncludePath(SM, Loc, Collector);
if (FilePath.empty()) return llvm::None;
- return SymbolInfo(ND->getNameAsString(), Type, FilePath,
-SM.getExpansionLineNumber(Loc), GetContexts(ND));
+ return SymbolInfo(ND->getNameAsString(), Type, FilePath, GetContexts(ND));
}
} // namespace
@@ -196,7 +195,7 @@ void FindAllSymbols::registerMatchers(Ma
anyOf(hasDeclContext(enumDecl(HasNSOrTUCtxMatcher)), ExternCMatcher));
// Most of the time we care about all matchable decls, or all types.
- auto Types = namedDecl(anyOf(CRecords, CXXRecords, Enums, Typedefs));
+ auto Types = namedDecl(anyOf(CRecords, CXXRecords, Enums));
auto Decls = namedDecl(anyOf(CRecords, CXXRecords, Enums, Typedefs, Vars,
EnumConstants, Functions));
@@ -219,8 +218,8 @@ void FindAllSymbols::registerMatchers(Ma
typeLoc(isExpansionInMainFile(),
loc(qualT
[PATCH] D27334: [OpenCL] Ambiguous function call.
echuraev added a comment.
In https://reviews.llvm.org/D27334#614826, @Anastasia wrote:
> In https://reviews.llvm.org/D27334#614389, @bader wrote:
>
> > In https://reviews.llvm.org/D27334#613504, @Anastasia wrote:
> >
> > > In https://reviews.llvm.org/D27334#612858, @bader wrote:
> > >
> > > > In https://reviews.llvm.org/D27334#611703, @Anastasia wrote:
> > > >
> > > > > This change seems to modify normal C behavior again. Is there any
> > > > > strong motivation for doing this and if yes could it be done
> > > > > generically with C?
> > > >
> > > >
> > > > Motivation:
> > > >
> > > > // Non-portable OpenCL 1.2 code
> > > > __kernel void foo(global float* out) {
> > > > out[get_global_id(0)] = sin(get_global_id(0));
> > > > }
> > > >
> > > >
> > > > This program compiles fine on OpenCL platform w/o doubles support and
> > > > fails otherwise.
> > > > If OpenCL driver supports doubles it provides at least two versions of
> > > > 'sin' built-in math function and compiler will not be able to choose
> > > > the right one for 'size_t' argument.
> > > > The goal of this warning is to let OpenCL developer know about
> > > > potential issues with code portability.
> > >
> > >
> > > I would argue this improves the portability much as it can also be
> > > misleading in some situations (because it refers to a potentially
> > > hypothetical problem). For example there can be builtin functions that
> > > only have a float parameter (without a double version of it). This is for
> > > example the case with read_image functions that take a float coordinate
> > > value between 0 and 1. Unfortunately this warning won't be triggered on
> > > read_image functions because there is an overload candidate with an int
> > > type of the same parameter too. But we can't exclude this situations to
> > > appear in the future or from some vendor extensions or even custom OpenCL
> > > code.
> >
> >
> > As much as any other warning it's not always means that there is an error
> > in the code. It just means that developer should inspect the construction
> > triggering a warning.
> > Passing integer value to a function with floating point parameters is not
> > always an error, but some times it might be so.
> > Do you suggest dropping the diagnostics at all or changing the diagnostics
> > message?
>
>
> I agree warnings don't always signal a definite issue (even thought it's good
> to make them as precise as we can). We could try to reword the diagnostic
> message. However, the biggest issue I have here is that the message can be
> given in the situations that are unrelated to the problem (i.e. the overload
> candidates that don't have anything to do with the parameter being diagnosed
> or don't overload with the double precision). Therefore, it feels like the
> diagnostic can be confusing in some cases even though they are not very
> probable ones.
@Anastasia, do you have any suggestions how is it better to reword the
diagnostic message? Yes, this message can be given in some situations that are
unrelated to the problem but in this case it will be a notification for
developer that this function call can be potential ambiguous.
Comment at: lib/Sema/SemaChecking.cpp:2479
+// integer values.
+if (FDecl->hasAttr()) {
+ for (const auto* Arg : Args) {
Anastasia wrote:
> How does it check that this is a built-in function?
In OpenCL we can overload only built-in functions. So, I think that we can
recognize that the function is built-in by checking that the language is OpenCL
and the function has Overloadable attribute.
https://reviews.llvm.org/D27334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius added a comment. Hi Alex and sorry for the late reply. The main use case is a more readable `.clang-tidy` configuration checks. Before this correction one can use something like this: --- Checks: ' ,*, ,-cert-dcl03-c, ' ... It works, but is hardly comprehensible to a newbie (the strange use of addtional commas). Since the spaces are ignored (since a recent commit of yours) we can remove the leading comma and hope that no user uses a tab... After applying this patch, we can just write (with tabs or spaces and as many newlines as we want - used for grouping for instance): --- Checks: ' *, -cert-dcl03-c, ' ... Additionaly, you can sometimes accidentally issue a tabulator on the command line and that's just nice to ignore it. https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30693: [mips][msa] Remove range checks for non-immediate sld.[bhwd] instructions
sdardis accepted this revision. sdardis added a comment. This revision is now accepted and ready to land. LGTM. Minor nit: there's some spurious whitespace that can be removed, see my inlined comment. Comment at: test/CodeGen/builtins-mips-msa.c:701 v2i64_r = __msa_sld_d(v2i64_r, v2i64_a, 1); // CHECK: call <2 x i64> @llvm.mips.sld.d( + + v16i8_r = __msa_sld_b(v16i8_r, v16i8_a, 16); // CHECK: call <16 x i8> @llvm.mips.sld.b( I'm seeing some whitespace here in the patch, remove it before committing please. https://reviews.llvm.org/D30693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r297367 - [clang-tidy] Update the doc according to r297311.
Thank you! Completely forgot about this. On Thu, Mar 9, 2017 at 10:15 AM, Haojian Wu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: hokein > Date: Thu Mar 9 03:15:16 2017 > New Revision: 297367 > > URL: http://llvm.org/viewvc/llvm-project?rev=297367&view=rev > Log: > [clang-tidy] Update the doc according to r297311. > > Modified: > clang-tools-extra/trunk/docs/clang-tidy/checks/readability- > function-size.rst > > Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability- > function-size.rst > URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ > trunk/docs/clang-tidy/checks/readability-function-size.rst? > rev=297367&r1=297366&r2=297367&view=diff > > == > --- > clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst > (original) > +++ > clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst > Thu Mar 9 03:15:16 2017 > @@ -29,4 +29,4 @@ Options > .. option:: ParameterThreshold > > Flag functions that exceed a specified number of parameters. The > default > - is 6. > + is `-1` (ignore the number of parameters). > > > ___ > 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
[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors
leanil marked 3 inline comments as done.
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:31
+ ->getName()
+ .find("enable_if") != StringRef::npos;
+ };
aaron.ballman wrote:
> This should be using `equals()` rather than `find()` -- otherwise a name like
> `enable_if_awesome()` would trigger the match. You should also make sure the
> name is in the `::std` namespace so that you can't trigger it with
> `foo::enable_if` or `foo::std::enable_if`.
My intention with using `find()` was that if a user has an
`enable_if_awesome()`, then it seems more likely to be similar to the std
version, than to have completely different purpose.
And as a bonus `find()` also finds `std::enable_if_t` in one check.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> I think a better diagnostic might be: "constructor accepting a universal
> reference hides the %select{copy|move|both the copy and move}0
> %select{constructor{|s}1"
>
> And then provide a note ("%select{copy|move}0 constructor declared here")
> that points to the offending copy and/or move constructor.
Without checking actual constructor calls, I would have to make notes on every
(non disabled) copy / move constructor, any time I produce a warning. And as
the warning already states where the problem lies, the notes would only help
people find the copy and move constructors. Do you think that's necessary?
Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
CheckFactories.registerCheck("misc-misplaced-const");
aaron.ballman wrote:
> I wonder if the name might be slightly misleading -- I've always understood
> these to be universal references rather than forwarding references. I don't
> have the Meyers book in front of me -- what nomenclature does he use?
>
> Once we pick the name, we should use it consistently in the source code (like
> the file name for the check and the check name), the documentation, and the
> release notes.
Meyers calls them universal references, but it's //forwarding reference// in
the standard (14.8.2.1).
Repository:
rL LLVM
https://reviews.llvm.org/D30547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30773: Make git-clang-format python 3 compatible
EricWF updated this revision to Diff 91145.
EricWF added a comment.
This still isn't working in python3 due to more bytes vs str issues, but this
is my progress so far.
https://reviews.llvm.org/D30773
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -23,6 +23,7 @@
Requires Python 2.7
"""
+from __future__ import print_function
import argparse
import collections
import contextlib
@@ -138,15 +139,15 @@
if opts.verbose >= 1:
ignored_files.difference_update(changed_lines)
if ignored_files:
- print 'Ignoring changes in the following files (wrong extension):'
+ print('Ignoring changes in the following files (wrong extension):')
for filename in ignored_files:
-print ' ', filename
+print('%s' % filename)
if changed_lines:
- print 'Running clang-format on the following files:'
+ print('Running clang-format on the following files:')
for filename in changed_lines:
-print ' ', filename
+print('%s' % filename)
if not changed_lines:
-print 'no modified files to format'
+print('no modified files to format')
return
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
@@ -163,20 +164,20 @@
binary=opts.binary,
style=opts.style)
if opts.verbose >= 1:
-print 'old tree:', old_tree
-print 'new tree:', new_tree
+print('old tree:', old_tree)
+print('new tree:', new_tree)
if old_tree == new_tree:
if opts.verbose >= 0:
- print 'clang-format did not modify any files'
+ print('clang-format did not modify any files')
elif opts.diff:
print_diff(old_tree, new_tree)
else:
changed_files = apply_changes(old_tree, new_tree, force=opts.force,
patch_mode=opts.patch)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
- print 'changed files:'
+ print('changed files:')
for filename in changed_files:
-print ' ', filename
+print('%s' % filename)
def load_git_config(non_string_options=None):
@@ -188,9 +189,9 @@
if non_string_options is None:
non_string_options = {}
out = {}
- for entry in run('git', 'config', '--list', '--null').split('\0'):
+ for entry in run('git', 'config', '--list', '--null').split(b'\0'):
if entry:
- name, value = entry.split('\n', 1)
+ name, value = entry.split(b'\n', 1)
if name in non_string_options:
value = run('git', 'config', non_string_options[name], name)
out[name] = value
@@ -289,6 +290,25 @@
return p
+def to_bytes(str):
+# Encode to UTF-8 to get binary data.
+if isinstance(str, bytes):
+return str
+return str.encode('utf-8')
+
+def to_string(bytes):
+if isinstance(bytes, str):
+return bytes
+return to_bytes(bytes)
+
+def convert_string(bytes):
+try:
+return to_string(bytes.decode('utf-8'))
+except AttributeError: # 'str' object has no attribute 'decode'.
+return str(bytes)
+except UnicodeError:
+return str(bytes)
+
def extract_lines(patch_file):
"""Extract the changed lines in `patch_file`.
@@ -300,10 +320,10 @@
list of line `Range`s."""
matches = {}
for line in patch_file:
-match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
+match = re.search(to_bytes(r'^\+\+\+\ [^/]+/(.*)'), line)
if match:
- filename = match.group(1).rstrip('\r\n')
-match = re.search(r'^@@ -[0-9,]+ \+(\d+)(,(\d+))?', line)
+ filename = match.group(1).rstrip(b'\r\n')
+match = re.search(to_bytes(r'^@@ -[0-9,]+ \+(\d+)(,(\d+))?'), line)
if match:
start_line = int(match.group(1))
line_count = 1
@@ -320,10 +340,14 @@
`allowed_extensions` must be a collection of lowercase file extensions,
excluding the period."""
allowed_extensions = frozenset(allowed_extensions)
- for filename in dictionary.keys():
-base_ext = filename.rsplit('.', 1)
+ keys_to_delete = []
+ for filename in list(dictionary.keys()):
+filename_cp = convert_string(bytes(filename))
+base_ext = filename_cp.rsplit('.', 1)
if len(base_ext) == 1 or base_ext[1].lower() not in allowed_extensions:
- del dictionary[filename]
+ keys_to_delete += [filename]
+ for key in keys_to_delete:
+del dictionary[key]
def cd_to_toplevel():
@@ -345,7 +369,7 @@
Returns the object ID (SHA-1) of the created tree."""
def index_info_generator():
-for filename, line_ranges in changed_lines.iteritems():
+for filename, line_ranges in changed_lines.items():
if revision:
git_metadata_c
[clang-tools-extra] r297367 - [clang-tidy] Update the doc according to r297311.
Author: hokein Date: Thu Mar 9 03:15:16 2017 New Revision: 297367 URL: http://llvm.org/viewvc/llvm-project?rev=297367&view=rev Log: [clang-tidy] Update the doc according to r297311. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst?rev=297367&r1=297366&r2=297367&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst Thu Mar 9 03:15:16 2017 @@ -29,4 +29,4 @@ Options .. option:: ParameterThreshold Flag functions that exceed a specified number of parameters. The default - is 6. + is `-1` (ignore the number of parameters). ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors
leanil updated this revision to Diff 91144.
leanil added a comment.
Fix enable_if detection in pointer and reference types.
Improve test coverage for these cases.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
clang-tidy/misc/ForwardingReferenceOverloadCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-forwarding-reference-overload.rst
test/clang-tidy/misc-forwarding-reference-overload.cpp
Index: test/clang-tidy/misc-forwarding-reference-overload.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-forwarding-reference-overload.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s misc-forwarding-reference-overload %t
+
+namespace std {
+template
+struct enable_if {};
+
+template
+struct enable_if { typedef T type; };
+
+template
+using enable_if_t = typename enable_if::type;
+}
+
+template
+constexpr bool just_true = true;
+
+class Person {
+public:
+ template
+ Person(T &&n);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide copy and move constructors [misc-forwarding-reference-overload]
+
+ template
+ Person(T &&n, int I = 5, ...);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide copy and move constructors [misc-forwarding-reference-overload]
+};
+
+template
+class Person2 {
+public:
+ // Two parameters without default value, can't act as copy / move constructor.
+ template
+ Person2(T &&n, V &&m, int i = 5, ...);
+
+ // Guarded with enable_if.
+ template
+ Person2(T &&n, int i = 5, std::enable_if_t a = 5, ...);
+
+ // Guarded with enable_if.
+ template ::type &>
+ Person2(T &&n);
+
+ // Guarded with enable_if.
+ template
+ Person2(T &&n, typename std::enable_if>::type **a = nullptr);
+
+ // Guarded with enable_if.
+ template > *&&>
+ Person2(T &&n, double d = 0.0);
+
+ // Not a forwarding reference parameter.
+ template
+ Person2(const T &&n);
+
+ // Not a forwarding reference parameter.
+ Person2(int &&x);
+
+ // Two parameters without default value, can't act as copy / move constructor.
+ template
+ Person2(T &&n, int x);
+
+ // Not a forwarding reference parameter.
+ template
+ Person2(U &&n);
+};
+
+// Copy and move constructors are both disabled.
+class Person3 {
+public:
+ template
+ Person3(T &&n);
+
+ template
+ Person3(T &&n, int I = 5, ...);
+
+ Person3(const Person3 &rhs) = delete;
+
+private:
+ Person3(Person3 &&rhs);
+};
Index: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-forwarding-reference-overload.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - misc-forwarding-reference-overload
+
+misc-forwarding-reference-overload
+==
+
+The check looks for perfect forwarding constructors that can hide copy or move
+constructors. If a non const lvalue reference is passed to the constructor, the
+forwarding reference parameter will be a better match than the const reference
+parameter of the copy constructor, so the perfect forwarding constructor will be
+called, which can be confusing.
+For detailed description of this issue see: Scott Meyers, Effective Modern C++,
+Item 26.
+
+Consider the following example:
+
+ .. code-block:: c++
+
+class Person {
+public:
+ // C1: perfect forwarding ctor
+ template
+ explicit Person(T&& n) {}
+
+ // C2: perfect forwarding ctor with parameter default value
+ template
+ explicit Person(T&& n, int x = 1) {}
+
+ // C3: perfect forwarding ctor guarded with enable_if
+ template,void>>
+ explicit Person(T&& n) {}
+
+ // (possibly compiler generated) copy ctor
+ Person(const Person& rhs);
+};
+
+The check warns for constructors C1 and C2, because those can hide copy and move
+constructors. We suppress warnings if the copy and the move constructors are both
+disabled (deleted or private), because there is nothing the perfect forwarding
+constructor could hide in this case. We also suppress warnings for constructors
+like C3 that are guarded with an enable_if, assuming the programmer was aware of
+the possible hiding.
+
+Background
+--
+
+For deciding whether a constructor is guarded with enable_if, we consider the
+default values of the type parameters and the types of the contructor
+parameters. If any part of these types contains "enable_if" in its name, we
+assume the constructor is guarded.
+
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -64,6 +64,7 @@
misc-definitions-in-headers
[PATCH] D30773: Make git-clang-format python 3 compatible
kimgr added inline comments.
Comment at: tools/clang-format/git-clang-format:323
allowed_extensions = frozenset(allowed_extensions)
- for filename in dictionary.keys():
+ for filename in list(dictionary.keys()):
base_ext = filename.rsplit('.', 1)
EricWF wrote:
> EricWF wrote:
> > kimgr wrote:
> > > This should not be necessary for iteration -- in Python3 it returns a
> > > generator instead of a list, but generators can be iterated.
> > This was done by the 2to3 tool. I'll have to see what it thinks it was up
> > to. But I agree this seems wrong.
> Ah, so this is needed because in python 3 it's a runtime error to remove
> items from a dictionary while iterating over the keys. So we have to make a
> copy first.
Oh, I see, I didn't catch that. Carry on.
https://reviews.llvm.org/D30773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30773: Make git-clang-format python 3 compatible
EricWF marked 2 inline comments as done.
EricWF added inline comments.
Comment at: tools/clang-format/git-clang-format:323
allowed_extensions = frozenset(allowed_extensions)
- for filename in dictionary.keys():
+ for filename in list(dictionary.keys()):
base_ext = filename.rsplit('.', 1)
EricWF wrote:
> kimgr wrote:
> > This should not be necessary for iteration -- in Python3 it returns a
> > generator instead of a list, but generators can be iterated.
> This was done by the 2to3 tool. I'll have to see what it thinks it was up to.
> But I agree this seems wrong.
Ah, so this is needed because in python 3 it's a runtime error to remove items
from a dictionary while iterating over the keys. So we have to make a copy
first.
https://reviews.llvm.org/D30773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30773: Make git-clang-format python 3 compatible
EricWF marked 4 inline comments as done.
EricWF added inline comments.
Comment at: tools/clang-format/git-clang-format:323
allowed_extensions = frozenset(allowed_extensions)
- for filename in dictionary.keys():
+ for filename in list(dictionary.keys()):
base_ext = filename.rsplit('.', 1)
kimgr wrote:
> This should not be necessary for iteration -- in Python3 it returns a
> generator instead of a list, but generators can be iterated.
This was done by the 2to3 tool. I'll have to see what it thinks it was up to.
But I agree this seems wrong.
Comment at: tools/clang-format/git-clang-format:492
+ print(('The following files would be modified but '
+ 'have unstaged changes:'), file=sys.stderr)
+ print(unstaged_files, file=sys.stderr)
kimgr wrote:
> I wonder if `file=sys.stderr` works with Python 2.7's print statement. Have
> you tested this with 2.7?
>
> You can use `__future__` to get the print function behavior in 2.7 as well:
> http://stackoverflow.com/questions/32032697/how-to-use-from-future-import-print-function
It doesn't. I think getting the new behavior from `__future__` is the only
reasonable way to go.
Comment at: tools/clang-format/git-clang-format:512-515
+def to_string(bytes):
+if isinstance(bytes, str):
+return bytes
+return to_bytes(bytes)
kimgr wrote:
> This looks wrong -- where does `to_bytes` come from?
>
> I can never get my head around Python2's string/bytes philosophy. In Python3
> you would do:
>
> # stdout and stderr are always `bytes`
> stdout = stdout.decode('utf-8')
> stderr = stderr.decode('utf-8')
>
> (assuming the input *is* utf-8)
>
> Not sure how that plays with Python2, but the current code doesn't look right.
>
This is wrong. And it will be removed in the next set of changes.
https://reviews.llvm.org/D30773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30773: Make git-clang-format python 3 compatible
kimgr added a comment.
Some nits from a Python3 hacker.
Comment at: tools/clang-format/git-clang-format:143
for filename in ignored_files:
-print ' ', filename
+print(' ', filename)
if changed_lines:
I find `print('template', tail)` surprising in Python3, but it could be because
we don't use it in our local standards. I'd jump immediately to formatting to
make this 2/3-compatible:
print('%s' % filename)
or in this case maybe just join the strings:
print('' + filename)
Comment at: tools/clang-format/git-clang-format:323
allowed_extensions = frozenset(allowed_extensions)
- for filename in dictionary.keys():
+ for filename in list(dictionary.keys()):
base_ext = filename.rsplit('.', 1)
This should not be necessary for iteration -- in Python3 it returns a generator
instead of a list, but generators can be iterated.
Comment at: tools/clang-format/git-clang-format:491
if unstaged_files:
- print >>sys.stderr, ('The following files would be modified but '
- 'have unstaged changes:')
- print >>sys.stderr, unstaged_files
- print >>sys.stderr, 'Please commit, stage, or stash them first.'
+ print(('The following files would be modified but '
+ 'have unstaged changes:'), file=sys.stderr)
No need for parentheses around the string
Comment at: tools/clang-format/git-clang-format:492
+ print(('The following files would be modified but '
+ 'have unstaged changes:'), file=sys.stderr)
+ print(unstaged_files, file=sys.stderr)
I wonder if `file=sys.stderr` works with Python 2.7's print statement. Have you
tested this with 2.7?
You can use `__future__` to get the print function behavior in 2.7 as well:
http://stackoverflow.com/questions/32032697/how-to-use-from-future-import-print-function
Comment at: tools/clang-format/git-clang-format:512-515
+def to_string(bytes):
+if isinstance(bytes, str):
+return bytes
+return to_bytes(bytes)
This looks wrong -- where does `to_bytes` come from?
I can never get my head around Python2's string/bytes philosophy. In Python3
you would do:
# stdout and stderr are always `bytes`
stdout = stdout.decode('utf-8')
stderr = stderr.decode('utf-8')
(assuming the input *is* utf-8)
Not sure how that plays with Python2, but the current code doesn't look right.
https://reviews.llvm.org/D30773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r297364 - Fix up the places where AddressSpace.hpp is included.
Author: ed
Date: Thu Mar 9 02:04:07 2017
New Revision: 297364
URL: http://llvm.org/viewvc/llvm-project?rev=297364&view=rev
Log:
Fix up the places where AddressSpace.hpp is included.
The AddressSpace.hpp header declares two classes: LocalAddressSpace and
RemoteAddressSpace. These classes are only used in a very small number
of source files, but passed in as template arguments to many other
classes.
Let's go ahead and only include AddressSpace.hpp in source files where
at least one of these two classes is mentioned. This gets rid of a
cyclic header dependency that was already present, but only caused
breakage on macOS until recently.
Reported by:Marshall Clow
Modified:
libunwind/trunk/src/CompactUnwinder.hpp
libunwind/trunk/src/DwarfInstructions.hpp
libunwind/trunk/src/DwarfParser.hpp
libunwind/trunk/src/EHHeaderParser.hpp
libunwind/trunk/src/Unwind_AppleExtras.cpp
libunwind/trunk/src/libunwind.cpp
Modified: libunwind/trunk/src/CompactUnwinder.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CompactUnwinder.hpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/CompactUnwinder.hpp (original)
+++ libunwind/trunk/src/CompactUnwinder.hpp Thu Mar 9 02:04:07 2017
@@ -19,7 +19,6 @@
#include
#include
-#include "AddressSpace.hpp"
#include "Registers.hpp"
#define EXTRACT_BITS(value, mask)
\
Modified: libunwind/trunk/src/DwarfInstructions.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfInstructions.hpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/DwarfInstructions.hpp (original)
+++ libunwind/trunk/src/DwarfInstructions.hpp Thu Mar 9 02:04:07 2017
@@ -18,7 +18,6 @@
#include
#include "dwarf2.h"
-#include "AddressSpace.hpp"
#include "Registers.hpp"
#include "DwarfParser.hpp"
#include "config.h"
Modified: libunwind/trunk/src/DwarfParser.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Thu Mar 9 02:04:07 2017
@@ -21,7 +21,6 @@
#include "libunwind.h"
#include "dwarf2.h"
-#include "AddressSpace.hpp"
#include "config.h"
namespace libunwind {
Modified: libunwind/trunk/src/EHHeaderParser.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/EHHeaderParser.hpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/EHHeaderParser.hpp (original)
+++ libunwind/trunk/src/EHHeaderParser.hpp Thu Mar 9 02:04:07 2017
@@ -15,7 +15,6 @@
#include "libunwind.h"
-#include "AddressSpace.hpp"
#include "DwarfParser.hpp"
namespace libunwind {
Modified: libunwind/trunk/src/Unwind_AppleExtras.cpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind_AppleExtras.cpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/Unwind_AppleExtras.cpp (original)
+++ libunwind/trunk/src/Unwind_AppleExtras.cpp Thu Mar 9 02:04:07 2017
@@ -9,6 +9,7 @@
//===--===//
#include "config.h"
+#include "AddressSpace.hpp"
#include "DwarfParser.hpp"
#include "unwind_ext.h"
Modified: libunwind/trunk/src/libunwind.cpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/libunwind.cpp?rev=297364&r1=297363&r2=297364&view=diff
==
--- libunwind/trunk/src/libunwind.cpp (original)
+++ libunwind/trunk/src/libunwind.cpp Thu Mar 9 02:04:07 2017
@@ -24,6 +24,7 @@
#include
+#include "AddressSpace.hpp"
#include "UnwindCursor.hpp"
using namespace libunwind;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30776: [coroutines] Fix diagnostics depending on the first coroutine statement.
EricWF created this revision.
Herald added a subscriber: mehdi_amini.
Some coroutine diagnostics need to point to the location of the first coroutine
keyword in the function, like when diagnosing a `return` inside a coroutine.
Previously we did this by storing each *valid* coroutine statement in a list
and select the first one to use in diagnostics. However if every coroutine
statement is invalid we would have no location to point to.
This patch fixes the storage of the first coroutine statement location,
ensuring that it gets stored even when the resulting AST node would be invalid.
This patch also removes the `CoroutineStmts` list in `FunctionScopeInfo`
because it was unused.
https://reviews.llvm.org/D30776
Files:
include/clang/Sema/ScopeInfo.h
lib/Sema/ScopeInfo.cpp
lib/Sema/SemaCoroutine.cpp
lib/Sema/TreeTransform.h
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -162,11 +162,59 @@
return; // expected-error {{not allowed in coroutine}}
}
+void mixed_yield_invalid() {
+ co_yield blah; // expected-error {{use of undeclared identifier}}
+ // expected-note@-1 {{function is a coroutine due to use of 'co_yield'}}
+ return; // expected-error {{return statement not allowed in coroutine}}
+}
+
+template
+void mixed_yield_template(T) {
+ co_yield blah; // expected-error {{use of undeclared identifier}}
+ // expected-note@-1 {{function is a coroutine due to use of 'co_yield'}}
+ return; // expected-error {{return statement not allowed in coroutine}}
+}
+
+template
+void mixed_yield_template2(T) {
+ co_yield 42;
+ // expected-note@-1 {{function is a coroutine due to use of 'co_yield'}}
+ return; // expected-error {{return statement not allowed in coroutine}}
+}
+
+template
+void mixed_yield_template3(T v) {
+ co_yield blah(v);
+ // expected-note@-1 {{function is a coroutine due to use of 'co_yield'}}
+ return; // expected-error {{return statement not allowed in coroutine}}
+}
+
void mixed_await() {
co_await a; // expected-note {{use of 'co_await'}}
return; // expected-error {{not allowed in coroutine}}
}
+void mixed_await_invalid() {
+ co_await 42; // expected-error {{'int' is not a structure or union}}
+ // expected-note@-1 {{function is a coroutine due to use of 'co_await'}}
+ return; // expected-error {{not allowed in coroutine}}
+}
+
+template
+void mixed_await_template(T) {
+ co_await 42;
+ // expected-note@-1 {{function is a coroutine due to use of 'co_await'}}
+ return; // expected-error {{not allowed in coroutine}}
+}
+
+template
+void mixed_await_template2(T v) {
+ co_await v; // expected-error {{'long' is not a structure or union}}
+ // expected-note@-1 {{function is a coroutine due to use of 'co_await'}}
+ return; // expected-error {{not allowed in coroutine}}
+}
+template void mixed_await_template2(long); // expected-note {{requested here}}
+
void only_coreturn(void_tag) {
co_return; // OK
}
@@ -178,6 +226,33 @@
return; // expected-error {{not allowed in coroutine}}
}
+void mixed_coreturn_invalid(bool b) {
+ if (b)
+co_return; // expected-note {{use of 'co_return'}}
+// expected-error@-1 {{no member named 'return_void' in 'promise'}}
+ else
+return; // expected-error {{not allowed in coroutine}}
+}
+
+template
+void mixed_coreturn_template(void_tag, bool b, T v) {
+ if (b)
+co_return v; // expected-note {{use of 'co_return'}}
+// expected-error@-1 {{no member named 'return_value' in 'promise_void'}}
+ else
+return; // expected-error {{not allowed in coroutine}}
+}
+template void mixed_coreturn_template(void_tag, bool, int); // expected-note {{requested here}}
+
+template
+void mixed_coreturn_template2(bool b, T) {
+ if (b)
+co_return v; // expected-note {{use of 'co_return'}}
+// expected-error@-1 {{use of undeclared identifier 'v'}}
+ else
+return; // expected-error {{not allowed in coroutine}}
+}
+
struct CtorDtor {
CtorDtor() {
co_yield 0; // expected-error {{'co_yield' cannot be used in a constructor}}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6857,7 +6857,7 @@
ScopeInfo->NeedsCoroutineSuspends &&
ScopeInfo->CoroutineSuspends.first == nullptr &&
ScopeInfo->CoroutineSuspends.second == nullptr &&
- ScopeInfo->CoroutineStmts.empty() && "expected clean scope info");
+ "expected clean scope info");
// Set that we have (possibly-invalid) suspend points before we do anything
// that may fail.
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -399,15 +399,19 @@
/// Check that this is a context in which a coroutine suspension can appear.
stat
