[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
marcel added a comment. In https://reviews.llvm.org/D33393#761679, @erik.pilkington wrote: > Do you agree that this is the right approach here? Sure. As long as it fixes PR32890. You could append my test case as a quick-check. So, I'll go ahead and abandon this revision? https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
erik.pilkington added a comment. Could you please add more context lines in any future patches? Makes it easier to review! I think we fixed the same problem at the same time, I have https://reviews.llvm.org/D33368 that also fixes this! The reason that inserting into db.names.back() sometimes results in a insertion out of bounds is that parse_type() can sometimes append multiple names into `db.names` (ie for a reference to a pack expansion). This results in db.names.back() no longer being the text `'lambda`, which makes inserting at +7 dangerous. I think we should handle this case by appending them one by one into the lambda's parameters, which is what my patch does. Do you agree that this is the right approach here? Comment at: src/cxa_demangle.cpp:3080 + db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); t0 = t1; } We shouldn't make progress here if something invalid happened, if the names.back().size() is less than 7 then we should just bail out and return first. https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
marcel updated this revision to Diff 99850. marcel added a comment. Fixing off-by-one. Passes `make check-cxxabi` for LLVM in trunk on my machine (x86-64, Ubuntu 16.04). https://reviews.llvm.org/D33393 Files: CREDITS.TXT src/cxa_demangle.cpp test/test_demangle.pass.cpp Index: test/test_demangle.pass.cpp === --- test/test_demangle.pass.cpp +++ test/test_demangle.pass.cpp @@ -29665,6 +29665,7 @@ "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37", "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37\x72\x33\x8E\x3A\x29\x8E\x44\x35", "_ZcvCiIJEEDvT__T_vT_v", +"Z1JIJ1_T_EE3o00EUlT_E0", }; const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]); Index: src/cxa_demangle.cpp === --- src/cxa_demangle.cpp +++ src/cxa_demangle.cpp @@ -3075,7 +3075,8 @@ const char* t1 = t0 + 1; while (t1 != last && std::isdigit(*t1)) ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() >= 7) + db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); t0 = t1; } if (t0 == last || *t0 != '_') Index: CREDITS.TXT === --- CREDITS.TXT +++ CREDITS.TXT @@ -12,6 +12,10 @@ E: aa...@aaronballman.com D: Minor patches +N: Marcel Boehme +E: marcel.boe...@acm.org +D: Minor patches + N: Logan Chien E: logan.ch...@mediatek.com D: ARM EHABI Unwind & Exception Handling Index: test/test_demangle.pass.cpp === --- test/test_demangle.pass.cpp +++ test/test_demangle.pass.cpp @@ -29665,6 +29665,7 @@ "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37", "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37\x72\x33\x8E\x3A\x29\x8E\x44\x35", "_ZcvCiIJEEDvT__T_vT_v", +"Z1JIJ1_T_EE3o00EUlT_E0", }; const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]); Index: src/cxa_demangle.cpp === --- src/cxa_demangle.cpp +++ src/cxa_demangle.cpp @@ -3075,7 +3075,8 @@ const char* t1 = t0 + 1; while (t1 != last && std::isdigit(*t1)) ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() >= 7) + db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); t0 = t1; } if (t0 == last || *t0 != '_') Index: CREDITS.TXT === --- CREDITS.TXT +++ CREDITS.TXT @@ -12,6 +12,10 @@ E: aa...@aaronballman.com D: Minor patches +N: Marcel Boehme +E: marcel.boe...@acm.org +D: Minor patches + N: Logan Chien E: logan.ch...@mediatek.com D: ARM EHABI Unwind & Exception Handling ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
marcel added inline comments. Comment at: src/cxa_demangle.cpp:3078-3079 ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() > 7) + db.names.back().first.insert(db.names.back().first.begin() + 7, t0, t1); t0 = t1; dexonsmith wrote: > Should this be `>= 7`? You are right. Thanks for spotting this. Updating the patch. https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
dexonsmith added a reviewer: erik.pilkington. dexonsmith added inline comments. Comment at: src/cxa_demangle.cpp:3078-3079 ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() > 7) + db.names.back().first.insert(db.names.back().first.begin() + 7, t0, t1); t0 = t1; Should this be `>= 7`? https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31627: [coroutines] Skip over passthrough operator co_await
GorNishanov closed this revision. GorNishanov added a comment. r303605 = b026b234d55d6b9fab849389b2085204f530af5d https://reviews.llvm.org/D31627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303605 - [coroutines] Skip over passthrough operator co_await
Author: gornishanov
Date: Tue May 23 00:25:31 2017
New Revision: 303605
URL: http://llvm.org/viewvc/llvm-project?rev=303605&view=rev
Log:
[coroutines] Skip over passthrough operator co_await
https://reviews.llvm.org/D31627
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303605&r1=303604&r2=303605&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Tue May 23 00:25:31 2017
@@ -151,6 +151,20 @@ static RValue emitSuspendExpression(Code
AwaitKind Kind, AggValueSlot aggSlot,
bool ignoreResult) {
auto *E = S.getCommonExpr();
+
+ // FIXME: rsmith 5/22/2017. Does it still make sense for us to have a
+ // UO_Coawait at all? As I recall, the only purpose it ever had was to
+ // represent a dependent co_await expression that couldn't yet be resolved to
+ // a CoawaitExpr. But now we have (and need!) a separate DependentCoawaitExpr
+ // node to store unqualified lookup results, it seems that the UnaryOperator
+ // portion of the representation serves no purpose (and as seen in this
patch,
+ // it's getting in the way). Can we remove it?
+
+ // Skip passthrough operator co_await (present when awaiting on an LValue).
+ if (auto *UO = dyn_cast(E))
+if (UO->getOpcode() == UO_Coawait)
+ E = UO->getSubExpr();
+
auto Binder =
CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(),
E);
auto UnbindOnExit = llvm::make_scope_exit([&] { Binder.unbind(CGF); });
Modified: cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-await.cpp?rev=303605&r1=303604&r2=303605&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-await.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-await.cpp Tue May 23 00:25:31 2017
@@ -271,3 +271,10 @@ extern "C" void EndlessLoop() {
// CHECK-NOT: call void
@_ZNSt12experimental16coroutine_traitsIJvEE12promise_type13final_suspendEv(
// CHECK-NOT: call zeroext i1
@_ZN10final_susp11await_readyEv(%struct.final_susp*
}
+
+// Verifies that we don't crash when awaiting on an lvalue.
+// CHECK-LABEL: @_Z11AwaitLValuev(
+void AwaitLValue() {
+ suspend_always lval;
+ co_await lval;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303604 - Added LLVM_FALLTHROUGH to address gcc warning: this statement may fall through.
Author: gkistanova Date: Tue May 23 00:17:49 2017 New Revision: 303604 URL: http://llvm.org/viewvc/llvm-project?rev=303604&view=rev Log: Added LLVM_FALLTHROUGH to address gcc warning: this statement may fall through. Modified: cfe/trunk/lib/Driver/ToolChains/Myriad.cpp Modified: cfe/trunk/lib/Driver/ToolChains/Myriad.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Myriad.cpp?rev=303604&r1=303603&r2=303604&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Myriad.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Myriad.cpp Tue May 23 00:17:49 2017 @@ -217,6 +217,7 @@ MyriadToolChain::MyriadToolChain(const D default: D.Diag(clang::diag::err_target_unsupported_arch) << Triple.getArchName() << "myriad"; +LLVM_FALLTHROUGH; case llvm::Triple::sparc: case llvm::Triple::sparcel: case llvm::Triple::shave: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
GorNishanov closed this revision. GorNishanov added a comment. r303603 = 12cdab350043e7ff931af5f13d3e3afe85b76628 https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303603 - [coroutines] Add emission of initial and final suspends
Author: gornishanov
Date: Tue May 23 00:04:01 2017
New Revision: 303603
URL: http://llvm.org/viewvc/llvm-project?rev=303603&view=rev
Log:
[coroutines] Add emission of initial and final suspends
https://reviews.llvm.org/D31608
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303603&r1=303602&r2=303603&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Tue May 23 00:04:01 2017
@@ -388,7 +388,10 @@ void CodeGenFunction::EmitCoroutineBody(
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
-// FIXME: Emit initial suspend and more before the body.
+// FIXME: Emit param moves.
+
+CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
+EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
@@ -410,7 +413,8 @@ void CodeGenFunction::EmitCoroutineBody(
const bool HasCoreturns = CurCoro.Data->CoreturnCount > 0;
if (CanFallthrough || HasCoreturns) {
EmitBlock(FinalBB);
- // FIXME: Emit final suspend.
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Final;
+ EmitStmt(S.getFinalSuspendStmt());
}
}
Modified: cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-await.cpp?rev=303603&r1=303602&r2=303603&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-await.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-await.cpp Tue May 23 00:04:01 2017
@@ -21,6 +21,17 @@ struct coroutine_handle : coroutine_hand
}
}
+struct init_susp {
+ bool await_ready();
+ void await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+struct final_susp {
+ bool await_ready();
+ void await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+
struct suspend_always {
int stuff;
bool await_ready();
@@ -32,8 +43,8 @@ template<>
struct std::experimental::coroutine_traits {
struct promise_type {
void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
+init_susp initial_suspend();
+final_susp final_suspend();
void return_void();
};
};
@@ -42,6 +53,13 @@ struct std::experimental::coroutine_trai
extern "C" void f0() {
// CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(
+ // See if initial_suspend was issued:
+ // --
+ // CHECK: call void
@_ZNSt12experimental16coroutine_traitsIJvEE12promise_type15initial_suspendEv(
+ // CHECK-NEXT: call zeroext i1
@_ZN9init_susp11await_readyEv(%struct.init_susp*
+ // CHECK: %[[INITSP_ID:.+]] = call token @llvm.coro.save(
+ // CHECK: call i8 @llvm.coro.suspend(token %[[INITSP_ID]], i1 false)
+
co_await suspend_always{};
// See if we need to suspend:
// --
@@ -76,6 +94,13 @@ extern "C" void f0() {
// --
// CHECK: [[READY_BB]]:
// CHECK: call void
@_ZN14suspend_always12await_resumeEv(%struct.suspend_always* %[[AWAITABLE]])
+
+ // See if final_suspend was issued:
+ // --
+ // CHECK: call void
@_ZNSt12experimental16coroutine_traitsIJvEE12promise_type13final_suspendEv(
+ // CHECK-NEXT: call zeroext i1
@_ZN10final_susp11await_readyEv(%struct.final_susp*
+ // CHECK: %[[FINALSP_ID:.+]] = call token @llvm.coro.save(
+ // CHECK: call i8 @llvm.coro.suspend(token %[[FINALSP_ID]], i1 true)
}
struct suspend_maybe {
@@ -91,8 +116,8 @@ template<>
struct std::experimental::coroutine_traits {
struct promise_type {
void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
+init_susp initial_suspend();
+final_susp final_suspend();
void return_void();
suspend_maybe yield_value(int);
};
@@ -228,3 +253,21 @@ extern "C" void TestOpAwait() {
// CHECK: call void @_ZN5MyAggawEv(%struct.MyAgg* %
// CHECK: call void @_ZN11AggrAwaiter12await_resumeEv(%struct.Aggr* sret %
}
+
+// CHECK-LABEL: EndlessLoop(
+extern "C" void EndlessLoop() {
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(
+
+ // See if initial_suspend was issued:
+ // --
+ // CHECK: call void
@_ZNSt12experimental16coroutine_traitsIJvEE12promise_type15initial_suspendEv(
+ // CHECK-NEXT: call zeroext i1
@_ZN9init_susp11await_readyEv(%struct.init_susp*
+
+ for (;;)
+co_await suspend_always{};
+
+ // Verify that final_suspend was NOT issued:
+ // --
+ // CHECK-NOT: call void
@_ZNSt12experimental16coroutine_
[PATCH] D31590: [coroutines] Add support for deallocation elision
GorNishanov added a comment. Commited: r303599 = abbd4f85db96b0ed1478e2ee52e9f1eec7404131 https://reviews.llvm.org/D31590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303599 - [coroutines] Add support for deallocation elision
Author: gornishanov
Date: Mon May 22 23:21:27 2017
New Revision: 303599
URL: http://llvm.org/viewvc/llvm-project?rev=303599&view=rev
Log:
[coroutines] Add support for deallocation elision
Wrap deallocation code with:
if (auto *mem = coro.free()) Deallocate
When backend decides to elide allocations it will replace coro.free with
nullptr to suppress deallocation code.
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303599&r1=303598&r2=303599&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Mon May 22 23:21:27 2017
@@ -62,6 +62,10 @@ struct clang::CodeGen::CGCoroData {
// the address of the coroutine frame of the current coroutine.
llvm::CallInst *CoroBegin = nullptr;
+ // Stores the last emitted coro.free for the deallocate expressions, we use
it
+ // to wrap dealloc code with if(auto mem = coro.free) dealloc(mem).
+ llvm::CallInst *LastCoroFree = nullptr;
+
// If coro.id came from the builtin, remember the expression to give better
// diagnostic. If CoroIdExpr is nullptr, the coro.id was created by
// EmitCoroutineBody.
@@ -262,14 +266,47 @@ namespace {
struct CallCoroDelete final : public EHScopeStack::Cleanup {
Stmt *Deallocate;
- // TODO: Wrap deallocate in if(coro.free(...)) Deallocate.
+ // Emit "if (coro.free(CoroId, CoroBegin)) Deallocate;"
+
+ // Note: That deallocation will be emitted twice: once for a normal exit and
+ // once for exceptional exit. This usage is safe because Deallocate does not
+ // contain any declarations. The SubStmtBuilder::makeNewAndDeleteExpr()
+ // builds a single call to a deallocation function which is safe to emit
+ // multiple times.
void Emit(CodeGenFunction &CGF, Flags) override {
-// Note: That deallocation will be emitted twice: once for a normal exit
and
-// once for exceptional exit. This usage is safe because Deallocate does
not
-// contain any declarations. The SubStmtBuilder::makeNewAndDeleteExpr()
-// builds a single call to a deallocation function which is safe to emit
-// multiple times.
+// Remember the current point, as we are going to emit deallocation code
+// first to get to coro.free instruction that is an argument to a delete
+// call.
+BasicBlock *SaveInsertBlock = CGF.Builder.GetInsertBlock();
+
+auto *FreeBB = CGF.createBasicBlock("coro.free");
+CGF.EmitBlock(FreeBB);
CGF.EmitStmt(Deallocate);
+
+auto *AfterFreeBB = CGF.createBasicBlock("after.coro.free");
+CGF.EmitBlock(AfterFreeBB);
+
+// We should have captured coro.free from the emission of deallocate.
+auto *CoroFree = CGF.CurCoro.Data->LastCoroFree;
+if (!CoroFree) {
+ CGF.CGM.Error(Deallocate->getLocStart(),
+"Deallocation expressoin does not refer to coro.free");
+ return;
+}
+
+// Get back to the block we were originally and move coro.free there.
+auto *InsertPt = SaveInsertBlock->getTerminator();
+CoroFree->moveBefore(InsertPt);
+CGF.Builder.SetInsertPoint(InsertPt);
+
+// Add if (auto *mem = coro.free) Deallocate;
+auto *NullPtr = llvm::ConstantPointerNull::get(CGF.Int8PtrTy);
+auto *Cond = CGF.Builder.CreateICmpNE(CoroFree, NullPtr);
+CGF.Builder.CreateCondBr(Cond, FreeBB, AfterFreeBB);
+
+// No longer need old terminator.
+InsertPt->eraseFromParent();
+CGF.Builder.SetInsertPoint(AfterFreeBB);
}
explicit CallCoroDelete(Stmt *DeallocStmt) : Deallocate(DeallocStmt) {}
};
@@ -431,7 +468,7 @@ RValue CodeGenFunction::EmitCoroutineInt
llvm::Value *F = CGM.getIntrinsic(IID);
llvm::CallInst *Call = Builder.CreateCall(F, Args);
- // Note: The following code is to enable to emit coroutine intrinsics by
+ // Note: The following code is to enable to emit coro.id and coro.begin by
// hand to experiment with coroutines in C.
// If we see @llvm.coro.id remember it in the CoroData. We will update
// coro.alloc, coro.begin and coro.free intrinsics to refer to it.
@@ -442,5 +479,10 @@ RValue CodeGenFunction::EmitCoroutineInt
if (CurCoro.Data)
CurCoro.Data->CoroBegin = Call;
}
- return RValue::get(Call);
+ else if (IID == llvm::Intrinsic::coro_free) {
+// Remember the last coro_free as we need it to build the conditional
+// deletion of the coroutine frame.
+if (CurCoro.Data)
+ CurCoro.Data->LastCoroFree = Call;
+ } return RValue::get(Call);
}
Modified: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp?rev=303599&r1=303598&r2=303599&view=diff
==
--- cfe/trunk/test/CodeGenCo
[PATCH] D31590: [coroutines] Add support for deallocation elision
GorNishanov accepted this revision. GorNishanov added a comment. LGTM https://reviews.llvm.org/D31590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
compnerd added inline comments.
Comment at: src/cxa_demangle.cpp:3036
break;
-if (db.names.size() < 2)
+if (k1 <= k0)
return first;
erik.pilkington wrote:
> compnerd wrote:
> > I'm not sure how `k1` can be `<` than `k0`. Isn't this effectively always
> > going down the `==` path? If I'm just mistaken, then I think that this
> > needs a comment.
> I agree that it should be impossible, but the previous version handled this
> case with the `if (db.names.size() < 2)` check, when really `db.names.size()`
> can only be 1 or greater. I think it makes more sense to be paranoid here, I
> trust this file about as far as I can throw it (And I can't throw it very
> far, its not a tangible object).
Hmm, how about being paranoid, and converting that to an assertion and then
doing the `==` check? This code is already incredibly complicated, so adding
additional things like this only makes it harder to understand whats going on.
Comment at: src/cxa_demangle.cpp:3042-3051
+for (size_t k = k0; k < k1; ++k) {
+auto tmp = db.names[k].move_full();
+if (!tmp.empty())
+{
+if (!is_first_it)
+db.names[lambda_pos].first.append(", ");
+is_first_it = false;
dexonsmith wrote:
> compnerd wrote:
> > I think that using a bit of algorithm here might be nice.
> >
> > std::ostringstream oss(db.names[lambda_pos]);
> > std::copy_if(db.names[k0], db.names[k1],
> > std::ostream_iterator(oss, ", "),
> > [](const std::string &s) -> bool { return !s.empty(); });
> Introducing a dependency on `std::ostream` would be awkward. libc++abi.dylib
> cannot link against libc++.dylib, and it would bloat libc++abi.dylib to use
> custom traits to pull in a full copy.
Ugh, thats a good point. Very well. However, there doesnt seem to be any
iterator invalidation here, so we can still just iterate over a slice here,
which would be nicer.
https://reviews.llvm.org/D33368
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal
compnerd added inline comments.
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "lib", "baremetal");
+ return Dir.str();
jroelofs wrote:
> compnerd wrote:
> > jroelofs wrote:
> > > compnerd wrote:
> > > > Why not just the standard `arm` directory?
> > > There are a few differences between the stuff in the existing ones, and
> > > what is needed on baremetal. For example __enable_execute_stack, emutls,
> > > as well as anything else that assumes existence of pthreads support
> > > shouldn't be there.
> > Well, I think that "baremetal" here is a bad idea. How about using the
> > android approach? Use `clang_rt.builtins-arm-baremetal.a` ?
> Why? Given the way the cmake goop works in lib/runtimes + compiler-rt, the
> folder name there has to be the same as the CMAKE_SYSTEM_NAME. The
> alternative, I guess, is to call it 'generic', but I'm not convinced that's
> better than 'baremetal'.
Because I can have a baremetal environment that uses a different architecture.
How do you differentiate between the MIPS and ARM bare metal runtimes? The way
that the compiler actually looks up the builtins is that it uses
`clang_rt.[component]-[arch][-variant]`
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+ const char *getDefaultLinker() const override { return "ld.lld"; }
+
ruiu wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > jroelofs wrote:
> > > > compnerd wrote:
> > > > > I think that this really should be `ld` still, as that is the
> > > > > canonical name for the linker.
> > > > You mean `lld`?
> > > >
> > > > ```
> > > > $ lld
> > > > lld is a generic driver.
> > > > Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.
> > > > ```
> > > >
> > > > Or are you saying: "make binutils ld the default, not llvm's lld"?
> > > Im saying use the name "ld". ld is a symlink on most linux distributions
> > > these days. ld -> ld.gold, ld.bfd, ld.lld
> > I don't think this makes sense. I don't care about "most linux
> > distributions", but rather about putting together an LLVM baremetal
> > distribution based as much as possible on LLVM components. If someone wants
> > a different linker, they can use `-fuse-ld=` and/or `-B`.
> >
> > Also, doesn't using a symlink named `ld` cause `lld` to behave like a
> > mach-o linker? We really want the elf one here.
> If you invoke lld as ld, it behaves as a Mach-O linker on macOS. On other
> OSes, it behaves as an ELF linker.
`ld` really is just the unix name for the linker. I think that at some point
we are going to have to consider cross-linking with lld. Adding a `-flavor
` with `-fuse-ld=lld` makes sense I think.
https://reviews.llvm.org/D33259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31586: [coroutines] Replace all coro.frame builtins with an SSA value of coro.begin
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. Committed: r303598 = cd8cc3ea48a887f846b3cbfc6dcb546a7b4de3af https://reviews.llvm.org/D31586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303598 - [coroutines] Replace all coro.frame builtins with an SSA value of coro.begin
Author: gornishanov
Date: Mon May 22 22:46:59 2017
New Revision: 303598
URL: http://llvm.org/viewvc/llvm-project?rev=303598&view=rev
Log:
[coroutines] Replace all coro.frame builtins with an SSA value of coro.begin
SemaCoroutine forms expressions referring to the coroutine frame of the
enclosing coroutine using coro.frame builtin.
During codegen, we emit llvm.coro.begin intrinsic that returns the address of
the coroutine frame.
When coro.frame is emitted, we replace it with SSA value of coro.begin.
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
cfe/trunk/test/CodeGenCoroutines/coro-builtins.c
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303598&r1=303597&r2=303598&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Mon May 22 22:46:59 2017
@@ -57,6 +57,11 @@ struct clang::CodeGen::CGCoroData {
// builtin.
llvm::CallInst *CoroId = nullptr;
+ // Stores the llvm.coro.begin emitted in the function so that we can replace
+ // all coro.frame intrinsics with direct SSA value of coro.begin that returns
+ // the address of the coroutine frame of the current coroutine.
+ llvm::CallInst *CoroBegin = nullptr;
+
// If coro.id came from the builtin, remember the expression to give better
// diagnostic. If CoroIdExpr is nullptr, the coro.id was created by
// EmitCoroutineBody.
@@ -330,8 +335,9 @@ void CodeGenFunction::EmitCoroutineBody(
auto *Phi = Builder.CreatePHI(VoidPtrTy, 2);
Phi->addIncoming(NullPtr, EntryBB);
Phi->addIncoming(AllocateCall, AllocOrInvokeContBB);
- Builder.CreateCall(
+ auto *CoroBegin = Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
+ CurCoro.Data->CoroBegin = CoroBegin;
CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
{
@@ -388,6 +394,17 @@ RValue CodeGenFunction::EmitCoroutineInt
switch (IID) {
default:
break;
+ // The coro.frame builtin is replaced with an SSA value of the coro.begin
+ // intrinsic.
+ case llvm::Intrinsic::coro_frame: {
+if (CurCoro.Data && CurCoro.Data->CoroBegin) {
+ return RValue::get(CurCoro.Data->CoroBegin);
+}
+CGM.Error(E->getLocStart(), "this builtin expect that __builtin_coro_begin
"
+ "has been used earlier in this function");
+auto NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
+return RValue::get(NullPtr);
+ }
// The following three intrinsics take a token parameter referring to a token
// returned by earlier call to @llvm.coro.id. Since we cannot represent it in
// builtins, we patch it up here.
@@ -414,10 +431,16 @@ RValue CodeGenFunction::EmitCoroutineInt
llvm::Value *F = CGM.getIntrinsic(IID);
llvm::CallInst *Call = Builder.CreateCall(F, Args);
+ // Note: The following code is to enable to emit coroutine intrinsics by
+ // hand to experiment with coroutines in C.
// If we see @llvm.coro.id remember it in the CoroData. We will update
// coro.alloc, coro.begin and coro.free intrinsics to refer to it.
if (IID == llvm::Intrinsic::coro_id) {
createCoroData(*this, CurCoro, Call, E);
}
+ else if (IID == llvm::Intrinsic::coro_begin) {
+if (CurCoro.Data)
+ CurCoro.Data->CoroBegin = Call;
+ }
return RValue::get(Call);
}
Modified: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp?rev=303598&r1=303597&r2=303598&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp Mon May 22 22:46:59 2017
@@ -66,9 +66,8 @@ extern "C" void f0(global_new_delete_tag
// CHECK: [[InitBB]]:
// CHECK: %[[PHI:.+]] = phi i8* [ null, %{{.+}} ], [ %call, %[[AllocBB]] ]
- // CHECK: call i8* @llvm.coro.begin(token %[[ID]], i8* %[[PHI]])
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[ID]], i8*
%[[PHI]])
- // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
// CHECK: %[[MEM:.+]] = call i8* @llvm.coro.free(token %[[ID]], i8*
%[[FRAME]])
// CHECK: call void @_ZdlPv(i8* %[[MEM]])
co_return;
@@ -93,7 +92,7 @@ extern "C" void f1(promise_new_tag ) {
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
// CHECK: call i8*
@_ZNSt12experimental16coroutine_traitsIJv15promise_new_tagEE12promise_typenwEm(i64
%[[SIZE]])
- // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(
// CHECK: %[[MEM:.+]] = call i8* @llvm.coro.free(token %[[ID]], i8*
%[[FRAME]])
// CHECK: call void @_ZdlPv(i8* %[[MEM]])
co_return;
@@ -118,7 +117,7 @@ ext
[PATCH] D31586: [coroutines] Replace all coro.frame builtins with an SSA value of coro.begin
GorNishanov updated this revision to Diff 99843.
GorNishanov added a comment.
merged with top of the trunk. preparing to land
https://reviews.llvm.org/D31586
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/CodeGenCoroutines/coro-await.cpp
test/CodeGenCoroutines/coro-builtins.c
Index: test/CodeGenCoroutines/coro-builtins.c
===
--- test/CodeGenCoroutines/coro-builtins.c
+++ test/CodeGenCoroutines/coro-builtins.c
@@ -2,7 +2,7 @@
void *myAlloc(long long);
-// CHECK-LABEL: f(
+// CHECK-LABEL: f(
void f(int n) {
// CHECK: %n.addr = alloca i32
// CHECK: %n_copy = alloca i32
@@ -19,31 +19,25 @@
// CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
// CHECK-NEXT: %[[MEM:.+]] = call i8* @myAlloc(i64 %[[SIZE]])
- // CHECK-NEXT: %[[BEG:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], i8* %[[MEM]])
+ // CHECK-NEXT: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], i8* %[[MEM]])
__builtin_coro_begin(myAlloc(__builtin_coro_size()));
- // CHECK-NEXT: %[[FRAME1:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call void @llvm.coro.resume(i8* %[[FRAME1]])
+ // CHECK-NEXT: call void @llvm.coro.resume(i8* %[[FRAME]])
__builtin_coro_resume(__builtin_coro_frame());
- // CHECK-NEXT: %[[FRAME2:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call void @llvm.coro.destroy(i8* %[[FRAME2]])
+ // CHECK-NEXT: call void @llvm.coro.destroy(i8* %[[FRAME]])
__builtin_coro_destroy(__builtin_coro_frame());
- // CHECK-NEXT: %[[FRAME3:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call i1 @llvm.coro.done(i8* %[[FRAME3]])
+ // CHECK-NEXT: call i1 @llvm.coro.done(i8* %[[FRAME]])
__builtin_coro_done(__builtin_coro_frame());
- // CHECK-NEXT: %[[FRAME4:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call i8* @llvm.coro.promise(i8* %[[FRAME4]], i32 48, i1 false)
+ // CHECK-NEXT: call i8* @llvm.coro.promise(i8* %[[FRAME]], i32 48, i1 false)
__builtin_coro_promise(__builtin_coro_frame(), 48, 0);
- // CHECK-NEXT: %[[FRAME5:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call i8* @llvm.coro.free(token %[[COROID]], i8* %[[FRAME5]])
+ // CHECK-NEXT: call i8* @llvm.coro.free(token %[[COROID]], i8* %[[FRAME]])
__builtin_coro_free(__builtin_coro_frame());
- // CHECK-NEXT: %[[FRAME6:.+]] = call i8* @llvm.coro.frame()
- // CHECK-NEXT: call i1 @llvm.coro.end(i8* %[[FRAME6]], i1 false)
+ // CHECK-NEXT: call i1 @llvm.coro.end(i8* %[[FRAME]], i1 false)
__builtin_coro_end(__builtin_coro_frame(), 0);
// CHECK-NEXT: call i8 @llvm.coro.suspend(token none, i1 true)
Index: test/CodeGenCoroutines/coro-await.cpp
===
--- test/CodeGenCoroutines/coro-await.cpp
+++ test/CodeGenCoroutines/coro-await.cpp
@@ -40,6 +40,7 @@
// CHECK-LABEL: f0(
extern "C" void f0() {
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(
co_await suspend_always{};
// See if we need to suspend:
@@ -54,7 +55,6 @@
// ---
// 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** %{{.+}}
@@ -100,8 +100,9 @@
// CHECK-LABEL: f1(
extern "C" void f1(int) {
- co_yield 42;
// CHECK: %[[PROMISE:.+]] = alloca %"struct.std::experimental::coroutine_traits::promise_type"
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(
+ co_yield 42;
// 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:
@@ -116,7 +117,6 @@
// ---
// Build the coroutine handle and pass it to await_suspend
// ---
- // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
// CHECK: call i8* @_ZNSt12experimental16coroutine_handleINS_16coroutine_traitsIJviEE12promise_typeEE12from_addressEPv(i8* %[[FRAME]])
// ... many lines of code to coerce coroutine_handle into an i8* scalar
// CHECK: %[[CH:.+]] = load i8*, i8** %{{.+}}
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -66,9 +66,8 @@
// CHECK: [[InitBB]]:
// CHECK: %[[PHI:.+]] = phi i8* [ null, %{{.+}} ], [ %call, %[[AllocBB]] ]
- // CHECK: call i8* @llvm.coro.begin(token %[[ID]], i8* %[[PHI]])
+ // CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[ID]], i8* %[[PHI]])
- // CHEC
[PATCH] D33430: [clang-tidy] Do not dereference a null BaseType
chh created this revision.
Herald added a subscriber: xazax.hun.
Check BaseType before dereference.
Simplified test case is derived from Android Open Source code.
https://reviews.llvm.org/D33430
Files:
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
test/clang-tidy/misc-forwarding-reference-overload.cpp
Index: test/clang-tidy/misc-forwarding-reference-overload.cpp
===
--- test/clang-tidy/misc-forwarding-reference-overload.cpp
+++ test/clang-tidy/misc-forwarding-reference-overload.cpp
@@ -121,3 +121,25 @@
private:
Test6(const Test6 &rhs);
};
+
+// Do not dereference a null BaseType.
+template class result_of;
+template class result_of<_Fp(_Args...)> { };
+template using result_of_t = typename result_of<_Tp>::type;
+
+template struct __overload;
+template
+struct __overload<_Tp, _Types...> : __overload<_Types...> {
+ using __overload<_Types...>::operator();
+};
+
+template
+using __best_match_t = typename
result_of_t<__overload<_Types...>(_Tp&&)>::type;
+
+template
+class variant {
+public:
+ template >
+ constexpr variant(_Arg&& __arg) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: constructor accepting a
forwarding reference can hide the copy and move constructors
+};
Index: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
===
--- clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
+++ clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
@@ -40,6 +40,8 @@
if (const auto *Dependent = BaseType->getAs()) {
BaseType = Dependent->getQualifier()->getAsType();
}
+ if (!BaseType)
+return false;
if (CheckTemplate(BaseType->getAs())) {
return true; // Case: enable_if_t< >.
} else if (const auto *Elaborated = BaseType->getAs()) {
Index: test/clang-tidy/misc-forwarding-reference-overload.cpp
===
--- test/clang-tidy/misc-forwarding-reference-overload.cpp
+++ test/clang-tidy/misc-forwarding-reference-overload.cpp
@@ -121,3 +121,25 @@
private:
Test6(const Test6 &rhs);
};
+
+// Do not dereference a null BaseType.
+template class result_of;
+template class result_of<_Fp(_Args...)> { };
+template using result_of_t = typename result_of<_Tp>::type;
+
+template struct __overload;
+template
+struct __overload<_Tp, _Types...> : __overload<_Types...> {
+ using __overload<_Types...>::operator();
+};
+
+template
+using __best_match_t = typename result_of_t<__overload<_Types...>(_Tp&&)>::type;
+
+template
+class variant {
+public:
+ template >
+ constexpr variant(_Arg&& __arg) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+};
Index: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
===
--- clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
+++ clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
@@ -40,6 +40,8 @@
if (const auto *Dependent = BaseType->getAs()) {
BaseType = Dependent->getQualifier()->getAsType();
}
+ if (!BaseType)
+return false;
if (CheckTemplate(BaseType->getAs())) {
return true; // Case: enable_if_t< >.
} else if (const auto *Elaborated = BaseType->getAs()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31584: [coroutines] Add support for allocation elision
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303596: [coroutines] Add support for allocation elision
(authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D31584?vs=93805&id=99838#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31584
Files:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -284,6 +284,9 @@
auto &TI = CGM.getContext().getTargetInfo();
unsigned NewAlign = TI.getNewAlign() / TI.getCharWidth();
+ auto *EntryBB = Builder.GetInsertBlock();
+ auto *AllocBB = createBasicBlock("coro.alloc");
+ auto *InitBB = createBasicBlock("coro.init");
auto *FinalBB = createBasicBlock("coro.final");
auto *RetBB = createBasicBlock("coro.ret");
@@ -293,12 +296,20 @@
createCoroData(*this, CurCoro, CoroId);
CurCoro.Data->SuspendBB = RetBB;
+ // Backend is allowed to elide memory allocations, to help it, emit
+ // auto mem = coro.alloc() ? 0 : ... allocation code ...;
+ auto *CoroAlloc = Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_alloc), {CoroId});
+
+ Builder.CreateCondBr(CoroAlloc, AllocBB, InitBB);
+
+ EmitBlock(AllocBB);
auto *AllocateCall = EmitScalarExpr(S.getAllocate());
+ auto *AllocOrInvokeContBB = Builder.GetInsertBlock();
// Handle allocation failure if 'ReturnStmtOnAllocFailure' was provided.
if (auto *RetOnAllocFailure = S.getReturnStmtOnAllocFailure()) {
auto *RetOnFailureBB = createBasicBlock("coro.ret.on.failure");
-auto *InitBB = createBasicBlock("coro.init");
// See if allocation was successful.
auto *NullPtr = llvm::ConstantPointerNull::get(Int8PtrTy);
@@ -308,9 +319,19 @@
// If not, return OnAllocFailure object.
EmitBlock(RetOnFailureBB);
EmitStmt(RetOnAllocFailure);
-
-EmitBlock(InitBB);
}
+ else {
+Builder.CreateBr(InitBB);
+ }
+
+ EmitBlock(InitBB);
+
+ // Pass the result of the allocation to coro.begin.
+ auto *Phi = Builder.CreatePHI(VoidPtrTy, 2);
+ Phi->addIncoming(NullPtr, EntryBB);
+ Phi->addIncoming(AllocateCall, AllocOrInvokeContBB);
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
{
Index: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
@@ -56,8 +56,17 @@
// CHECK-LABEL: f0(
extern "C" void f0(global_new_delete_tag) {
// CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
+ // CHECK: %[[NeedAlloc:.+]] = call i1 @llvm.coro.alloc(token %[[ID]])
+ // CHECK: br i1 %[[NeedAlloc]], label %[[AllocBB:.+]], label %[[InitBB:.+]]
+
+ // CHECK: [[AllocBB]]:
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
- // CHECK: call i8* @_Znwm(i64 %[[SIZE]])
+ // CHECK: %[[MEM:.+]] = call i8* @_Znwm(i64 %[[SIZE]])
+ // CHECK: br label %[[InitBB]]
+
+ // CHECK: [[InitBB]]:
+ // CHECK: %[[PHI:.+]] = phi i8* [ null, %{{.+}} ], [ %call, %[[AllocBB]] ]
+ // CHECK: call i8* @llvm.coro.begin(token %[[ID]], i8* %[[PHI]])
// CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
// CHECK: %[[MEM:.+]] = call i8* @llvm.coro.free(token %[[ID]], i8*
%[[FRAME]])
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -284,6 +284,9 @@
auto &TI = CGM.getContext().getTargetInfo();
unsigned NewAlign = TI.getNewAlign() / TI.getCharWidth();
+ auto *EntryBB = Builder.GetInsertBlock();
+ auto *AllocBB = createBasicBlock("coro.alloc");
+ auto *InitBB = createBasicBlock("coro.init");
auto *FinalBB = createBasicBlock("coro.final");
auto *RetBB = createBasicBlock("coro.ret");
@@ -293,12 +296,20 @@
createCoroData(*this, CurCoro, CoroId);
CurCoro.Data->SuspendBB = RetBB;
+ // Backend is allowed to elide memory allocations, to help it, emit
+ // auto mem = coro.alloc() ? 0 : ... allocation code ...;
+ auto *CoroAlloc = Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_alloc), {CoroId});
+
+ Builder.CreateCondBr(CoroAlloc, AllocBB, InitBB);
+
+ EmitBlock(AllocBB);
auto *AllocateCall = EmitScalarExpr(S.getAllocate());
+ auto *AllocOrInvokeContBB = Builder.GetInsertBlock();
// Handle allocation failure if 'ReturnStmtOnAllocFailure' was provided.
if (auto *RetOnAllocFailure = S.getReturnStmtOnAllocFailure()) {
auto *RetOnFailureBB = createBasicBlock("coro.ret.on.failure");
-auto *InitBB = createBasicBlock("coro.init");
// See if allocation was successful.
auto *NullPtr = llv
r303596 - [coroutines] Add support for allocation elision
Author: gornishanov
Date: Mon May 22 20:13:17 2017
New Revision: 303596
URL: http://llvm.org/viewvc/llvm-project?rev=303596&view=rev
Log:
[coroutines] Add support for allocation elision
Summary:
We wrap allocation code so that backend can elide it if necessary.
llvm.coro.alloc intrinsic returns true, when allocation is needed and false
otherwise.
```
%NeedAlloc = call i1 @llvm.coro.alloc(token %2)
br i1 %NeedAlloc, label %AllocBB, label %InitBB
AllocBB:
%5 = call i64 @llvm.coro.size.i64()
%call = call i8* @_Znwm(i64 %5) ; operator new
br label %InitBB
InitBB:
%Phi = phi i8* [ null, %0 ], [ %call, %4 ]
call i8* @llvm.coro.begin(token %2, i8* %Phi)
```
Reviewers: majnemer, EricWF
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D31584
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303596&r1=303595&r2=303596&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Mon May 22 20:13:17 2017
@@ -284,6 +284,9 @@ void CodeGenFunction::EmitCoroutineBody(
auto &TI = CGM.getContext().getTargetInfo();
unsigned NewAlign = TI.getNewAlign() / TI.getCharWidth();
+ auto *EntryBB = Builder.GetInsertBlock();
+ auto *AllocBB = createBasicBlock("coro.alloc");
+ auto *InitBB = createBasicBlock("coro.init");
auto *FinalBB = createBasicBlock("coro.final");
auto *RetBB = createBasicBlock("coro.ret");
@@ -293,12 +296,20 @@ void CodeGenFunction::EmitCoroutineBody(
createCoroData(*this, CurCoro, CoroId);
CurCoro.Data->SuspendBB = RetBB;
+ // Backend is allowed to elide memory allocations, to help it, emit
+ // auto mem = coro.alloc() ? 0 : ... allocation code ...;
+ auto *CoroAlloc = Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_alloc), {CoroId});
+
+ Builder.CreateCondBr(CoroAlloc, AllocBB, InitBB);
+
+ EmitBlock(AllocBB);
auto *AllocateCall = EmitScalarExpr(S.getAllocate());
+ auto *AllocOrInvokeContBB = Builder.GetInsertBlock();
// Handle allocation failure if 'ReturnStmtOnAllocFailure' was provided.
if (auto *RetOnAllocFailure = S.getReturnStmtOnAllocFailure()) {
auto *RetOnFailureBB = createBasicBlock("coro.ret.on.failure");
-auto *InitBB = createBasicBlock("coro.init");
// See if allocation was successful.
auto *NullPtr = llvm::ConstantPointerNull::get(Int8PtrTy);
@@ -308,9 +319,19 @@ void CodeGenFunction::EmitCoroutineBody(
// If not, return OnAllocFailure object.
EmitBlock(RetOnFailureBB);
EmitStmt(RetOnAllocFailure);
-
-EmitBlock(InitBB);
}
+ else {
+Builder.CreateBr(InitBB);
+ }
+
+ EmitBlock(InitBB);
+
+ // Pass the result of the allocation to coro.begin.
+ auto *Phi = Builder.CreatePHI(VoidPtrTy, 2);
+ Phi->addIncoming(NullPtr, EntryBB);
+ Phi->addIncoming(AllocateCall, AllocOrInvokeContBB);
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
{
Modified: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp?rev=303596&r1=303595&r2=303596&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp Mon May 22 20:13:17 2017
@@ -56,8 +56,17 @@ struct std::experimental::coroutine_trai
// CHECK-LABEL: f0(
extern "C" void f0(global_new_delete_tag) {
// CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
+ // CHECK: %[[NeedAlloc:.+]] = call i1 @llvm.coro.alloc(token %[[ID]])
+ // CHECK: br i1 %[[NeedAlloc]], label %[[AllocBB:.+]], label %[[InitBB:.+]]
+
+ // CHECK: [[AllocBB]]:
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
- // CHECK: call i8* @_Znwm(i64 %[[SIZE]])
+ // CHECK: %[[MEM:.+]] = call i8* @_Znwm(i64 %[[SIZE]])
+ // CHECK: br label %[[InitBB]]
+
+ // CHECK: [[InitBB]]:
+ // CHECK: %[[PHI:.+]] = phi i8* [ null, %{{.+}} ], [ %call, %[[AllocBB]] ]
+ // CHECK: call i8* @llvm.coro.begin(token %[[ID]], i8* %[[PHI]])
// CHECK: %[[FRAME:.+]] = call i8* @llvm.coro.frame()
// CHECK: %[[MEM:.+]] = call i8* @llvm.coro.free(token %[[ID]], i8*
%[[FRAME]])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
faisalv closed this revision. faisalv added a comment. Committed as https://reviews.llvm.org/rL303594 https://reviews.llvm.org/D31414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303594 - [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
Author: faisalv
Date: Mon May 22 20:07:12 2017
New Revision: 303594
URL: http://llvm.org/viewvc/llvm-project?rev=303594&view=rev
Log:
[NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
A refactoring of TemplateIdAnnotation that uses TrailingObjects to create a
variably-sized object on the heap.
https://reviews.llvm.org/D31414
Thanks to Aaron B for the review!
Modified:
cfe/trunk/include/clang/Basic/TemplateKinds.h
cfe/trunk/include/clang/Sema/ParsedTemplate.h
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Parse/ParseTemplate.cpp
Modified: cfe/trunk/include/clang/Basic/TemplateKinds.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TemplateKinds.h?rev=303594&r1=303593&r2=303594&view=diff
==
--- cfe/trunk/include/clang/Basic/TemplateKinds.h (original)
+++ cfe/trunk/include/clang/Basic/TemplateKinds.h Mon May 22 20:07:12 2017
@@ -26,13 +26,21 @@ enum TemplateNameKind {
TNK_Function_template,
/// The name refers to a template whose specialization produces a
/// type. The template itself could be a class template, template
- /// template parameter, or C++0x template alias.
+ /// template parameter, or template alias.
TNK_Type_template,
/// The name refers to a variable template whose specialization produces a
/// variable.
TNK_Var_template,
- /// The name refers to a dependent template name. Whether the
- /// template name is assumed to refer to a type template or a
+ /// The name refers to a dependent template name:
+ /// \code
+ /// template struct apply2 {
+ /// typedef typename MetaFun::template apply::type type;
+ /// };
+ /// \endcode
+ ///
+ /// Here, "apply" is a dependent template name within the typename
+ /// specifier in the typedef. "apply" is a nested template, and
+ /// whether the template name is assumed to refer to a type template or a
/// function template depends on the context in which the template
/// name occurs.
TNK_Dependent_template_name
Modified: cfe/trunk/include/clang/Sema/ParsedTemplate.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ParsedTemplate.h?rev=303594&r1=303593&r2=303594&view=diff
==
--- cfe/trunk/include/clang/Sema/ParsedTemplate.h (original)
+++ cfe/trunk/include/clang/Sema/ParsedTemplate.h Mon May 22 20:07:12 2017
@@ -145,12 +145,15 @@ namespace clang {
/// expressions, or template names, and the source locations for important
/// tokens. All of the information about template arguments is allocated
/// directly after this structure.
- struct TemplateIdAnnotation {
+ struct TemplateIdAnnotation final
+ : private llvm::TrailingObjects {
+friend TrailingObjects;
/// \brief The nested-name-specifier that precedes the template name.
CXXScopeSpec SS;
-/// TemplateKWLoc - The location of the template keyword within the
-/// source.
+/// TemplateKWLoc - The location of the template keyword.
+/// For e.g. typename T::template Y
SourceLocation TemplateKWLoc;
/// TemplateNameLoc - The location of the template name within the
@@ -183,34 +186,56 @@ namespace clang {
/// \brief Retrieves a pointer to the template arguments
ParsedTemplateArgument *getTemplateArgs() {
- return reinterpret_cast(this + 1);
+ return getTrailingObjects();
}
/// \brief Creates a new TemplateIdAnnotation with NumArgs arguments and
/// appends it to List.
static TemplateIdAnnotation *
-Allocate(unsigned NumArgs, SmallVectorImpl &List) {
- TemplateIdAnnotation *TemplateId
-= (TemplateIdAnnotation *)std::malloc(sizeof(TemplateIdAnnotation) +
- sizeof(ParsedTemplateArgument) *
NumArgs);
- TemplateId->NumArgs = NumArgs;
-
- // Default-construct nested-name-specifier.
- new (&TemplateId->SS) CXXScopeSpec();
-
- // Default-construct parsed template arguments.
- ParsedTemplateArgument *TemplateArgs = TemplateId->getTemplateArgs();
- for (unsigned I = 0; I != NumArgs; ++I)
-new (TemplateArgs + I) ParsedTemplateArgument();
-
- List.push_back(TemplateId);
+Create(CXXScopeSpec SS, SourceLocation TemplateKWLoc,
+ SourceLocation TemplateNameLoc, IdentifierInfo *Name,
+ OverloadedOperatorKind OperatorKind,
+ ParsedTemplateTy OpaqueTemplateName, TemplateNameKind TemplateKind,
+ SourceLocation LAngleLoc, SourceLocation RAngleLoc,
+ ArrayRef TemplateArgs,
+ SmallVectorImpl &CleanupList) {
+
+ TemplateIdAnnotation *TemplateId = new (std::malloc(
+ totalSizeToAlloc(TemplateArgs.size(
+ TemplateIdAnnotation(SS, TemplateKWLoc, TemplateNameLoc, Name,
+ OperatorKind, OpaqueTemplateName, TemplateKi
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov marked an inline comment as done. GorNishanov added a comment. @rsmith, better now? :) https://reviews.llvm.org/D31670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov updated this revision to Diff 99837.
GorNishanov added a comment.
Remember old_top before emitting the cleanup and walk set Active on all emitted
cleanups.
I tried to add Active flag to Emission, but it ended up being very hairy, so I
went with the first option suggested.
https://reviews.llvm.org/D31670
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-gro.cpp
Index: test/CodeGenCoroutines/coro-gro.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-gro.cpp
@@ -0,0 +1,86 @@
+// Verifies lifetime of __gro local variable
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct GroType {
+ ~GroType();
+ operator int() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+GroType get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type();
+void unhandled_exception() noexcept;
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void doSomething() noexcept;
+
+// CHECK: define i32 @_Z1fv(
+int f() {
+ // CHECK: %[[RetVal:.+]] = alloca i32
+ // CHECK: %[[GroActive:.+]] = alloca i1
+
+ // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: call i8* @_Znwm(i64 %[[Size]])
+ // CHECK: store i1 false, i1* %[[GroActive]]
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+ // CHECK: store i1 true, i1* %[[GroActive]]
+
+ Cleanup cleanup;
+ doSomething();
+ co_return;
+
+ // CHECK: call void @_Z11doSomethingv(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv(
+ // CHECK: call void @_ZN7CleanupD1Ev(
+
+ // Destroy promise and free the memory.
+
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev(
+ // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free(
+ // CHECK: call void @_ZdlPv(i8* %[[Mem]])
+
+ // Initialize retval from Gro and destroy Gro
+
+ // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv(
+ // CHECK: store i32 %[[Conv]], i32* %[[RetVal]]
+ // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]]
+ // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+ // CHECK: [[CleanupGro]]:
+ // CHECK: call void @_ZN7GroTypeD1Ev(
+ // CHECK: br label %[[Done]]
+
+ // CHECK: [[Done]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]]
+ // CHECK: ret i32 %[[LoadRet]]
+}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -11,6 +11,7 @@
//
//===--===//
+#include "CGCleanup.h"
#include "CodeGenFunction.h"
#include "llvm/ADT/ScopeExit.h"
#include "clang/AST/StmtCXX.h"
@@ -270,6 +271,72 @@
};
}
+namespace {
+struct GetReturnObjectManager {
+ CodeGenFunction &CGF;
+ CGBuilderTy &Builder;
+ const CoroutineBodyStmt &S;
+
+ Address GroActiveFlag;
+ CodeGenFunction::AutoVarEmission GroEmission;
+
+ GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+ : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+ // The gro variable has to outlive coroutine frame and coroutine promise, but,
+ // it can only be initialized after coroutine promise was created, thus, we
+ // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+ // cleanups. Later when coroutine promise is available we initialize the gro
+ // and sets the flag that the cleanup is now active.
+
+ void EmitGroAlloca() {
+auto *GroDeclStmt = dyn_cast(S.getResultDecl());
+if (!GroDeclStmt) {
+ // If get_return_object returns void, no need to do an alloca.
+ return;
+}
+
+auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl());
+
+// Set GRO flag that it is not initialized yet
+GroActiveFlag =
+ CGF.CreateTe
r303593 - Adjust clang test for r303590
Author: tejohnson
Date: Mon May 22 19:35:09 2017
New Revision: 303593
URL: http://llvm.org/viewvc/llvm-project?rev=303593&view=rev
Log:
Adjust clang test for r303590
Forgot to commit this separately from the llvm change to use a new
module flag type for pic and pie levels. Should fix the bot errors
Modified:
cfe/trunk/test/CodeGen/piclevels.c
Modified: cfe/trunk/test/CodeGen/piclevels.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/piclevels.c?rev=303593&r1=303592&r2=303593&view=diff
==
--- cfe/trunk/test/CodeGen/piclevels.c (original)
+++ cfe/trunk/test/CodeGen/piclevels.c Mon May 22 19:35:09 2017
@@ -1,7 +1,12 @@
-// RUN: %clang_cc1 -emit-llvm -pic-level 2 %s -o - | FileCheck %s
-check-prefix=CHECK-BIGPIC
-// RUN: %clang_cc1 -emit-llvm -pic-level 1 %s -o - | FileCheck %s
-check-prefix=CHECK-SMALLPIC
+// RUN: %clang_cc1 -emit-llvm -pic-level 2 %s -o - | FileCheck %s
-check-prefix=CHECK-BIGPIC -check-prefix=CHECK-NOPIE
+// RUN: %clang_cc1 -emit-llvm -pic-level 1 %s -o - | FileCheck %s
-check-prefix=CHECK-SMALLPIC -check-prefix=CHECK-NOPIE
+// RUN: %clang_cc1 -emit-llvm -pic-level 2 -pic-is-pie %s -o - | FileCheck %s
-check-prefix=CHECK-BIGPIC -check-prefix=CHECK-BIGPIE
+// RUN: %clang_cc1 -emit-llvm -pic-level 1 -pic-is-pie %s -o - | FileCheck %s
-check-prefix=CHECK-SMALLPIC -check-prefix=CHECK-SMALLPIE
// CHECK-BIGPIC: !llvm.module.flags = !{{{.*}}}
-// CHECK-BIGPIC: !{{[0-9]+}} = !{i32 1, !"PIC Level", i32 2}
+// CHECK-BIGPIC: !{{[0-9]+}} = !{i32 7, !"PIC Level", i32 2}
// CHECK-SMALLPIC: !llvm.module.flags = !{{{.*}}}
-// CHECK-SMALLPIC: !{{[0-9]+}} = !{i32 1, !"PIC Level", i32 1}
+// CHECK-SMALLPIC: !{{[0-9]+}} = !{i32 7, !"PIC Level", i32 1}
+// CHECK-NOPIE-NOT: PIE Level
+// CHECK-BIGPIE: !{{[0-9]+}} = !{i32 7, !"PIE Level", i32 2}
+// CHECK-SMALLPIE: !{{[0-9]+}} = !{i32 7, !"PIE Level", i32 1}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).
rsmith added a comment. Counterproposal: in `-std=*++14` onwards, treat this construct as a user-defined literal, but fall back on the built-in interpretation if name lookup doesn't find an `operator""i` function. (The two interpretations only conflict if the source code explicitly does something like `using namespace std::complex_literals;`, which seems like a pretty big clue that the user expected `1.0i`to be `std::complex` rather than `_Complex double`.) If we find that's not enough for GCC compatibility in practice (perhaps in old code using `using namespace std;`), we can consider whether we want to make `-std=gnu++*` cause the built-in interpretation of the `i*` suffixes to be preferred over the overloaded interpretations. Comment at: clang/lib/Lex/LiteralSupport.cpp:656 // "i", "if", and "il" are user-defined suffixes in C++1y. - if (*s == 'i' && PP.getLangOpts().CPlusPlus14) + if (*s == 'i' && !PP.getLangOpts().GNUMode) break; I'm unconvinced this is sufficient justification to remove this previously-supported extension from `-std=c*` and `-std=c++98` modes. https://reviews.llvm.org/D33424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303589 - Add option to include multiple lines in snippets.
Author: rsmith
Date: Mon May 22 18:51:40 2017
New Revision: 303589
URL: http://llvm.org/viewvc/llvm-project?rev=303589&view=rev
Log:
Add option to include multiple lines in snippets.
When a diagnostic includes a highlighted range spanning multiple lines, clang
now supports printing out multiple lines of context if necessary to show the
highlighted ranges. This is not yet exposed in the driver, but can be enabled
by "-Xclang -fcaret-diagnostics-max-lines -Xclang N".
This is experimental until we can find out whether it works well in practice,
and if so, what a good default for the maximum number of lines is.
Added:
cfe/trunk/test/Misc/caret-diags-multiline.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticOptions.def
cfe/trunk/include/clang/Basic/DiagnosticOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/TextDiagnostic.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticOptions.def?rev=303589&r1=303588&r2=303589&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticOptions.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticOptions.def Mon May 22 18:51:40 2017
@@ -87,6 +87,8 @@ VALUE_DIAGOPT(TemplateBacktraceLimit, 32
VALUE_DIAGOPT(ConstexprBacktraceLimit, 32, DefaultConstexprBacktraceLimit)
/// Limit number of times to perform spell checking.
VALUE_DIAGOPT(SpellCheckingLimit, 32, DefaultSpellCheckingLimit)
+/// Limit number of lines shown in a snippet.
+VALUE_DIAGOPT(SnippetLineLimit, 32, DefaultSnippetLineLimit)
VALUE_DIAGOPT(TabStop, 32, DefaultTabStop) /// The distance between tab stops.
/// Column limit for formatting message diagnostics, or 0 if unused.
Modified: cfe/trunk/include/clang/Basic/DiagnosticOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticOptions.h?rev=303589&r1=303588&r2=303589&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticOptions.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticOptions.h Mon May 22 18:51:40 2017
@@ -63,11 +63,15 @@ public:
enum TextDiagnosticFormat { Clang, MSVC, Vi };
// Default values.
- enum { DefaultTabStop = 8, MaxTabStop = 100,
+ enum {
+DefaultTabStop = 8,
+MaxTabStop = 100,
DefaultMacroBacktraceLimit = 6,
DefaultTemplateBacktraceLimit = 10,
DefaultConstexprBacktraceLimit = 10,
-DefaultSpellCheckingLimit = 50 };
+DefaultSpellCheckingLimit = 50,
+DefaultSnippetLineLimit = 1,
+ };
// Define simple diagnostic options (with no accessors).
#define DIAGOPT(Name, Bits, Default) unsigned Name : Bits;
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=303589&r1=303588&r2=303589&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon May 22 18:51:40 2017
@@ -359,6 +359,9 @@ def fconstexpr_backtrace_limit : Separat
HelpText<"Set the maximum number of entries to print in a constexpr
evaluation backtrace (0 = no limit).">;
def fspell_checking_limit : Separate<["-"], "fspell-checking-limit">,
MetaVarName<"">,
HelpText<"Set the maximum number of times to perform spell checking on
unrecognized identifiers (0 = no limit).">;
+def fcaret_diagnostics_max_lines :
+ Separate<["-"], "fcaret-diagnostics-max-lines">, MetaVarName<"">,
+ HelpText<"Set the maximum number of source lines to show in a caret
diagnostic">;
def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"">,
HelpText<"Format message diagnostics so that they fit within N columns or
fewer, when possible.">;
def verify : Flag<["-"], "verify">,
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=303589&r1=303588&r2=303589&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon May 22 18:51:40 2017
@@ -1087,6 +1087,9 @@ bool clang::ParseDiagnosticArgs(Diagnost
Opts.SpellCheckingLimit = getLastArgIntValue(
Args, OPT_fspell_checking_limit,
DiagnosticOptions::DefaultSpellCheckingLimit, Diags);
+ Opts.SnippetLineLimit = getLastArgIntValue(
+ Args, OPT_fcaret_diagnostics_max_lines,
+ DiagnosticOptions::DefaultSnippetLineLimit, Diags);
Opts.TabStop = getLastArgIntValue(Args, OPT_ftabstop,
DiagnosticOptions::DefaultTabStop, Diags);
if (Opts.TabStop == 0 || Opts.TabStop > Di
Re: r303582 - Give files from #line the characteristics of the current file
On 22 May 2017 at 15:22, Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On Mon, May 22, 2017 at 3:17 PM, Richard Smith
> wrote:
>
>>// If the StrTok is "eod", then it wasn't present. Otherwise, it must
>>> be a
>>>// string followed by eod.
>>> - if (StrTok.is(tok::eod))
>>> -; // ok
>>> - else if (StrTok.isNot(tok::string_literal)) {
>>> + if (StrTok.is(tok::eod)) {
>>> +// Treat this like "#line NN", which doesn't change file
>>> characteristics.
>>> +FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
>>>
>>
>> This change for "# " handling makes sense (and I've checked and
>> it matches GCC), but it looks like we don't have test coverage for either
>> the old or new behavior. Can I interest you in adding some? :)
>>
>
> We do have coverage for it, see test/Preprocessor/line-directive.c:
> """
> # 192 "glomp.h" 3 // System header.
> typedef int y; // ok
> typedef int y; // ok
>
> typedef int q; // q is in system header.
>
> #line 42 "blonk.h" // doesn't change system headerness.
>
> typedef int z; // ok
> typedef int z; // ok
>
> # 97 // doesn't change system headerness.
>
> typedef int z1; // ok
> typedef int z1; // ok
> """
>
> We were just getting similar behavior because of the code I removed that
> sends us to the #line directive handling when the filename isn't present.
>
Ah, I see. It looks like, prior to your change, we got self-inconsistent
results. "# " worked properly when compiling (and we do have
coverage for that, per the above test), but not when preprocessing (for
which we don't have test coverage). So for instance, this input:
# 1 "foo" 3
# 2
x
preprocesses as
# 1 "foo" 3
# 2 "foo"
x
before your change and as
# 1 "foo" 3
# 2 "foo" 3
x
after your change.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30837: [libcxx] Support for shared_ptr
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D30837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303584 - [coroutines] Fix coro-eh-cleanup.cpp test
Author: gornishanov
Date: Mon May 22 17:41:28 2017
New Revision: 303584
URL: http://llvm.org/viewvc/llvm-project?rev=303584&view=rev
Log:
[coroutines] Fix coro-eh-cleanup.cpp test
Modified:
cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp
Modified: cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp?rev=303584&r1=303583&r2=303584&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp Mon May 22 17:41:28
2017
@@ -51,7 +51,7 @@ coro_t f() {
// CHECK: [[EHCLEANUP]]:
// CHECK: %[[INNERPAD:.+]] = cleanuppad within none []
// CHECK: call void @"\01??_DCleanup@@QEAAXXZ"(
-// CHECK: cleanupret from %4 unwind label %[[CATCHDISPATCH:.+]]
+// CHECK: cleanupret from %{{.+}} unwind label %[[CATCHDISPATCH:.+]]
// CHECK: [[CATCHDISPATCH]]:
// CHECK: catchswitch within none [label %[[CATCHPAD:.+]]] unwind label
%[[COROENDBB:.+]]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303583: [coroutines] Wrap the body of the coroutine in
try-catch (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D31692?vs=99815&id=99827#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31692
Files:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp
cfe/trunk/test/SemaCXX/coroutine-seh.cpp
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8960,6 +8960,8 @@
InGroup;
def err_coroutine_promise_get_return_object_on_allocation_failure : Error<
"%0: 'get_return_object_on_allocation_failure()' must be a static member function">;
+def err_seh_in_a_coroutine_with_cxx_exceptions : Error<
+ "cannot use SEH '__try' in a coroutine when C++ exceptions are enabled">;
def err_coroutine_promise_new_requires_nothrow : Error<
"%0 is required to have a non-throwing noexcept specification when the promise "
"type declares 'get_return_object_on_allocation_failure()'">;
Index: cfe/trunk/test/SemaCXX/coroutine-seh.cpp
===
--- cfe/trunk/test/SemaCXX/coroutine-seh.cpp
+++ cfe/trunk/test/SemaCXX/coroutine-seh.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+void SEH_used() {
+ __try { // expected-error {{cannot use SEH '__try' in a coroutine when C++ exceptions are enabled}}
+co_return; // expected-note {{function is a coroutine due to use of 'co_return' here}}
+ } __except(0) {}
+}
Index: cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
@@ -6,38 +6,38 @@
template struct coroutine_handle {
coroutine_handle() = default;
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
};
template <> struct coroutine_handle {
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
coroutine_handle() = default;
template
- coroutine_handle(coroutine_handle) {}
+ coroutine_handle(coroutine_handle) noexcept;
};
}
struct suspend_always {
- bool await_ready();
- void await_suspend(std::experimental::coroutine_handle<>);
- void await_resume();
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
};
template <> struct std::experimental::coroutine_traits {
struct promise_type {
-void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
-void return_void();
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
promise_type();
~promise_type();
-void unhandled_exception();
+void unhandled_exception() noexcept;
};
};
struct Cleanup { ~Cleanup(); };
void may_throw();
-// CHECK: define void @_Z1fv(
+// CHECK-LABEL: define void @_Z1fv(
void f() {
// CHECK: call i8* @_Znwm(i64
@@ -52,23 +52,46 @@
// if may_throw throws, check that we destroy the promise and free the memory.
// CHECK: invoke void @_Z9may_throwv(
- // CHECK-NEXT: to label %{{.+}} unwind label %[[PromDtorPad:.+]]
+ // CHECK-NEXT: to label %{{.+}} unwind label %[[CatchPad:.+]]
// CHECK: [[DeallocPad]]:
// CHECK-NEXT: landingpad
// CHECK-NEXT: cleanup
// CHECK: br label %[[Dealloc:.+]]
- // CHECK: [[PromD
r303583 - [coroutines] Wrap the body of the coroutine in try-catch
Author: gornishanov
Date: Mon May 22 17:33:17 2017
New Revision: 303583
URL: http://llvm.org/viewvc/llvm-project?rev=303583&view=rev
Log:
[coroutines] Wrap the body of the coroutine in try-catch
Summary:
If unhandled_exception member function is present in the coroutine promise,
wrap the body of the coroutine in:
```
try {
body
} catch(...) { promise.unhandled_exception(); }
```
Reviewers: EricWF, rnk, rsmith
Reviewed By: rsmith
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D31692
Added:
cfe/trunk/test/SemaCXX/coroutine-seh.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
cfe/trunk/test/CodeGenCoroutines/coro-eh-cleanup.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303583&r1=303582&r2=303583&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 22 17:33:17
2017
@@ -8960,6 +8960,8 @@ def warn_coroutine_promise_unhandled_exc
InGroup;
def err_coroutine_promise_get_return_object_on_allocation_failure : Error<
"%0: 'get_return_object_on_allocation_failure()' must be a static member
function">;
+def err_seh_in_a_coroutine_with_cxx_exceptions : Error<
+ "cannot use SEH '__try' in a coroutine when C++ exceptions are enabled">;
def err_coroutine_promise_new_requires_nothrow : Error<
"%0 is required to have a non-throwing noexcept specification when the
promise "
"type declares 'get_return_object_on_allocation_failure()'">;
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303583&r1=303582&r2=303583&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Mon May 22 17:33:17 2017
@@ -270,6 +270,15 @@ struct CallCoroDelete final : public EHS
};
}
+static void emitBodyAndFallthrough(CodeGenFunction &CGF,
+ const CoroutineBodyStmt &S, Stmt *Body) {
+ CGF.EmitStmt(Body);
+ const bool CanFallthrough = CGF.Builder.GetInsertBlock();
+ if (CanFallthrough)
+if (Stmt *OnFallthrough = S.getFallthroughHandler())
+ CGF.EmitStmt(OnFallthrough);
+}
+
void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
auto *NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
auto &TI = CGM.getContext().getTargetInfo();
@@ -318,7 +327,19 @@ void CodeGenFunction::EmitCoroutineBody(
// FIXME: Emit initial suspend and more before the body.
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-EmitStmt(S.getBody());
+
+if (auto *OnException = S.getExceptionHandler()) {
+ auto Loc = S.getLocStart();
+ CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr, OnException);
+ auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, S.getBody(),
&Catch);
+
+ EnterCXXTryStmt(*TryStmt);
+ emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
+ ExitCXXTryStmt(*TryStmt);
+}
+else {
+ emitBodyAndFallthrough(*this, S, S.getBody());
+}
// See if we need to generate final suspend.
const bool CanFallthrough = Builder.GetInsertBlock();
Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=303583&r1=303582&r2=303583&view=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Mon May 22 17:33:17 2017
@@ -1043,6 +1043,15 @@ bool CoroutineStmtBuilder::makeOnExcepti
if (UnhandledException.isInvalid())
return false;
+ // Since the body of the coroutine will be wrapped in try-catch, it will
+ // be incompatible with SEH __try if present in a function.
+ if (!S.getLangOpts().Borland && Fn.FirstSEHTryLoc.isValid()) {
+S.Diag(Fn.FirstSEHTryLoc,
diag::err_seh_in_a_coroutine_with_cxx_exceptions);
+S.Diag(Fn.FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
+<< Fn.getFirstCoroutineStmtKeyword();
+return false;
+ }
+
this->OnException = UnhandledException.get();
return true;
}
Modified: cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp?rev=303583&r1=303582&r2=303583&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-cleanup.cpp Mon May 22 17
[PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).
t.p.northover created this revision.
Herald added a subscriber: mcrosier.
While looking into the regression tests' compatibility with C++14, many of the
failures were because that specification defined UDLs for imaginary constants,
specifically ones ending in 'i' and 'il'. This conflicted with the previous GNU
extension that allowed them purely as a builtin literal syntax so they'd been
disabled in C++14 mode.
GCC's behaviour is to use the builtin constants in all "-std=gnu++N" modes but
disable them for "-std=c++11" and "-std=c++14" (they are still permitted in
"-std=c++98"). This seemed inconsistent (and likely an implementation detail
rather than intended behaviour) so my patch makes the decision purely on
whether we're in GNU mode.
Does it look reasonable?
Tim.
https://reviews.llvm.org/D33424
Files:
clang/lib/Lex/LiteralSupport.cpp
clang/test/SemaCXX/constexpr-printing.cpp
clang/unittests/AST/DeclTest.cpp
Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -26,7 +26,7 @@
// This is a regression test for a memory leak in APValues for structs that
// allocate memory. This test only fails if run under valgrind with full leak
// checking enabled.
- std::vector Args(1, "-std=c++11");
+ std::vector Args(1, "-std=gnu++11");
Args.push_back("-fno-ms-extensions");
ASSERT_TRUE(runToolOnCodeWithArgs(
Factory->create(),
Index: clang/test/SemaCXX/constexpr-printing.cpp
===
--- clang/test/SemaCXX/constexpr-printing.cpp
+++ clang/test/SemaCXX/constexpr-printing.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -std=c++11 -fsyntax-only -verify -triple x86_64-linux-gnu
+// RUN: %clang_cc1 %s -std=gnu++11 -fsyntax-only -verify -triple
x86_64-linux-gnu
struct S;
constexpr int extract(const S &s);
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -653,7 +653,7 @@
}
}
// "i", "if", and "il" are user-defined suffixes in C++1y.
- if (*s == 'i' && PP.getLangOpts().CPlusPlus14)
+ if (*s == 'i' && !PP.getLangOpts().GNUMode)
break;
// fall through.
case 'j':
Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -26,7 +26,7 @@
// This is a regression test for a memory leak in APValues for structs that
// allocate memory. This test only fails if run under valgrind with full leak
// checking enabled.
- std::vector Args(1, "-std=c++11");
+ std::vector Args(1, "-std=gnu++11");
Args.push_back("-fno-ms-extensions");
ASSERT_TRUE(runToolOnCodeWithArgs(
Factory->create(),
Index: clang/test/SemaCXX/constexpr-printing.cpp
===
--- clang/test/SemaCXX/constexpr-printing.cpp
+++ clang/test/SemaCXX/constexpr-printing.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -std=c++11 -fsyntax-only -verify -triple x86_64-linux-gnu
+// RUN: %clang_cc1 %s -std=gnu++11 -fsyntax-only -verify -triple x86_64-linux-gnu
struct S;
constexpr int extract(const S &s);
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -653,7 +653,7 @@
}
}
// "i", "if", and "il" are user-defined suffixes in C++1y.
- if (*s == 'i' && PP.getLangOpts().CPlusPlus14)
+ if (*s == 'i' && !PP.getLangOpts().GNUMode)
break;
// fall through.
case 'j':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r303582 - Give files from #line the characteristics of the current file
On Mon, May 22, 2017 at 3:17 PM, Richard Smith
wrote:
>// If the StrTok is "eod", then it wasn't present. Otherwise, it must
>> be a
>>// string followed by eod.
>> - if (StrTok.is(tok::eod))
>> -; // ok
>> - else if (StrTok.isNot(tok::string_literal)) {
>> + if (StrTok.is(tok::eod)) {
>> +// Treat this like "#line NN", which doesn't change file
>> characteristics.
>> +FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
>>
>
> This change for "# " handling makes sense (and I've checked and it
> matches GCC), but it looks like we don't have test coverage for either the
> old or new behavior. Can I interest you in adding some? :)
>
We do have coverage for it, see test/Preprocessor/line-directive.c:
"""
# 192 "glomp.h" 3 // System header.
typedef int y; // ok
typedef int y; // ok
typedef int q; // q is in system header.
#line 42 "blonk.h" // doesn't change system headerness.
typedef int z; // ok
typedef int z; // ok
# 97 // doesn't change system headerness.
typedef int z1; // ok
typedef int z1; // ok
"""
We were just getting similar behavior because of the code I removed that
sends us to the #line directive handling when the filename isn't present.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r303582 - Give files from #line the characteristics of the current file
On 22 May 2017 at 14:42, Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rnk
> Date: Mon May 22 16:42:58 2017
> New Revision: 303582
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303582&view=rev
> Log:
> Give files from #line the characteristics of the current file
>
> This allows #line directives to appear in system headers that have code
> that clang would normally warn on. This is compatible with GCC, which is
> easy to test by running `gcc -E`.
>
> Fixes PR30752
>
> Added:
> cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h
> cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h
> cfe/trunk/test/Frontend/system-header-line-directive.c
> Modified:
> cfe/trunk/include/clang/Basic/SourceManager.h
> cfe/trunk/include/clang/Basic/SourceManagerInternals.h
> cfe/trunk/lib/Basic/SourceManager.cpp
> cfe/trunk/lib/Frontend/FrontendAction.cpp
> cfe/trunk/lib/Lex/PPDirectives.cpp
> cfe/trunk/lib/Lex/Pragma.cpp
>
> Modified: cfe/trunk/include/clang/Basic/SourceManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/SourceManager.h?rev=303582&r1=303581&r2=303582&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/SourceManager.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManager.h Mon May 22 16:42:58 2017
> @@ -1399,10 +1399,9 @@ public:
>/// specified by Loc.
>///
>/// If FilenameID is -1, it is considered to be unspecified.
> - void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID);
>void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID,
> bool IsFileEntry, bool IsFileExit,
> - bool IsSystemHeader, bool IsExternCHeader);
> + SrcMgr::CharacteristicKind FileKind);
>
>/// \brief Determine if the source manager has a line table.
>bool hasLineTable() const { return LineTable != nullptr; }
>
> Modified: cfe/trunk/include/clang/Basic/SourceManagerInternals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> SourceManagerInternals.h?rev=303582&r1=303581&r2=303582&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/SourceManagerInternals.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManagerInternals.h Mon May 22
> 16:42:58 2017
> @@ -102,8 +102,6 @@ public:
>unsigned getNumFilenames() const { return FilenamesByID.size(); }
>
>void AddLineNote(FileID FID, unsigned Offset,
> - unsigned LineNo, int FilenameID);
> - void AddLineNote(FileID FID, unsigned Offset,
> unsigned LineNo, int FilenameID,
> unsigned EntryExit, SrcMgr::CharacteristicKind
> FileKind);
>
>
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
> SourceManager.cpp?rev=303582&r1=303581&r2=303582&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Mon May 22 16:42:58 2017
> @@ -183,48 +183,22 @@ unsigned LineTableInfo::getLineTableFile
>return IterBool.first->second;
> }
>
> -/// AddLineNote - Add a line note to the line table that indicates that
> there
> -/// is a \#line at the specified FID/Offset location which changes the
> presumed
> -/// location to LineNo/FilenameID.
> -void LineTableInfo::AddLineNote(FileID FID, unsigned Offset,
> -unsigned LineNo, int FilenameID) {
> - std::vector &Entries = LineEntries[FID];
> -
> - assert((Entries.empty() || Entries.back().FileOffset < Offset) &&
> - "Adding line entries out of order!");
> -
> - SrcMgr::CharacteristicKind Kind = SrcMgr::C_User;
> - unsigned IncludeOffset = 0;
> -
> - if (!Entries.empty()) {
> -// If this is a '#line 4' after '#line 42 "foo.h"', make sure to
> remember
> -// that we are still in "foo.h".
> -if (FilenameID == -1)
> - FilenameID = Entries.back().FilenameID;
> -
> -// If we are after a line marker that switched us to system header
> mode, or
> -// that set #include information, preserve it.
> -Kind = Entries.back().FileKind;
> -IncludeOffset = Entries.back().IncludeOffset;
> - }
> -
> - Entries.push_back(LineEntry::get(Offset, LineNo, FilenameID, Kind,
> - IncludeOffset));
> -}
> -
> -/// AddLineNote This is the same as the previous version of AddLineNote,
> but is
> -/// used for GNU line markers. If EntryExit is 0, then this doesn't
> change the
> -/// presumed \#include stack. If it is 1, this is a file entry, if it is
> 2 then
> -/// this is a file exit. FileKind specifies whether this is a system
> header or
> -/// extern C system header.
> -void LineTableInfo::AddLineNote(FileID FID
[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D31692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33398: Remove __unaligned preventively when mangling types in Itanium ABI
rsmith added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:2329-2333
+ // __unaligned is not currently mangled in any way. This implies that it is
+ // not a relevant qualifier for substitutions (while CVR and maybe others
+ // are). This triggers an assertion when this is the only qualifier and the
+ // unqualified type is a class. So let's remove it preventively here.
+ quals.removeUnaligned();
I don't think this is the right place/way to handle this: given
```
void f(struct X __unaligned *p, struct X *q) {}
```
it looks like we'll mangle as `_Z1fP1XP1X` with this patch, which seems wrong:
this should presumably instead be `_Z1fP1XS0_`.
But regardless, I think the right thing to do is to invent a mangling for
`__unaligned`, since we support overloading on it; the most appropriate
mangling would be `U11__unaligned`, per
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-type. (You'll need
to extend `mangleQualifiers` to emit this and
`hasMangledSubstitutionQualifiers` to ignore it.)
https://reviews.llvm.org/D33398
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303582 - Give files from #line the characteristics of the current file
Author: rnk
Date: Mon May 22 16:42:58 2017
New Revision: 303582
URL: http://llvm.org/viewvc/llvm-project?rev=303582&view=rev
Log:
Give files from #line the characteristics of the current file
This allows #line directives to appear in system headers that have code
that clang would normally warn on. This is compatible with GCC, which is
easy to test by running `gcc -E`.
Fixes PR30752
Added:
cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h
cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h
cfe/trunk/test/Frontend/system-header-line-directive.c
Modified:
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/include/clang/Basic/SourceManagerInternals.h
cfe/trunk/lib/Basic/SourceManager.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/Pragma.cpp
Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=303582&r1=303581&r2=303582&view=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Mon May 22 16:42:58 2017
@@ -1399,10 +1399,9 @@ public:
/// specified by Loc.
///
/// If FilenameID is -1, it is considered to be unspecified.
- void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID);
void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID,
bool IsFileEntry, bool IsFileExit,
- bool IsSystemHeader, bool IsExternCHeader);
+ SrcMgr::CharacteristicKind FileKind);
/// \brief Determine if the source manager has a line table.
bool hasLineTable() const { return LineTable != nullptr; }
Modified: cfe/trunk/include/clang/Basic/SourceManagerInternals.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManagerInternals.h?rev=303582&r1=303581&r2=303582&view=diff
==
--- cfe/trunk/include/clang/Basic/SourceManagerInternals.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManagerInternals.h Mon May 22 16:42:58
2017
@@ -102,8 +102,6 @@ public:
unsigned getNumFilenames() const { return FilenamesByID.size(); }
void AddLineNote(FileID FID, unsigned Offset,
- unsigned LineNo, int FilenameID);
- void AddLineNote(FileID FID, unsigned Offset,
unsigned LineNo, int FilenameID,
unsigned EntryExit, SrcMgr::CharacteristicKind FileKind);
Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=303582&r1=303581&r2=303582&view=diff
==
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Mon May 22 16:42:58 2017
@@ -183,48 +183,22 @@ unsigned LineTableInfo::getLineTableFile
return IterBool.first->second;
}
-/// AddLineNote - Add a line note to the line table that indicates that there
-/// is a \#line at the specified FID/Offset location which changes the presumed
-/// location to LineNo/FilenameID.
-void LineTableInfo::AddLineNote(FileID FID, unsigned Offset,
-unsigned LineNo, int FilenameID) {
- std::vector &Entries = LineEntries[FID];
-
- assert((Entries.empty() || Entries.back().FileOffset < Offset) &&
- "Adding line entries out of order!");
-
- SrcMgr::CharacteristicKind Kind = SrcMgr::C_User;
- unsigned IncludeOffset = 0;
-
- if (!Entries.empty()) {
-// If this is a '#line 4' after '#line 42 "foo.h"', make sure to remember
-// that we are still in "foo.h".
-if (FilenameID == -1)
- FilenameID = Entries.back().FilenameID;
-
-// If we are after a line marker that switched us to system header mode, or
-// that set #include information, preserve it.
-Kind = Entries.back().FileKind;
-IncludeOffset = Entries.back().IncludeOffset;
- }
-
- Entries.push_back(LineEntry::get(Offset, LineNo, FilenameID, Kind,
- IncludeOffset));
-}
-
-/// AddLineNote This is the same as the previous version of AddLineNote, but is
-/// used for GNU line markers. If EntryExit is 0, then this doesn't change the
-/// presumed \#include stack. If it is 1, this is a file entry, if it is 2
then
-/// this is a file exit. FileKind specifies whether this is a system header or
-/// extern C system header.
-void LineTableInfo::AddLineNote(FileID FID, unsigned Offset,
-unsigned LineNo, int FilenameID,
-unsigned EntryExit,
+/// Add a line note to the line table that indicates that there is a \#line or
+/// GNU line marker at the specified FID/Offset location which changes the
+/// presumed location
[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch
GorNishanov updated this revision to Diff 99815.
GorNishanov added a comment.
Fix misspelling in a comment.
@rsmith, Looks good now?
https://reviews.llvm.org/D31692
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-cleanup.cpp
test/CodeGenCoroutines/coro-eh-cleanup.cpp
test/SemaCXX/coroutine-seh.cpp
Index: test/SemaCXX/coroutine-seh.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-seh.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+void SEH_used() {
+ __try { // expected-error {{cannot use SEH '__try' in a coroutine when C++ exceptions are enabled}}
+co_return; // expected-note {{function is a coroutine due to use of 'co_return' here}}
+ } __except(0) {}
+}
Index: test/CodeGenCoroutines/coro-eh-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-eh-cleanup.cpp
+++ test/CodeGenCoroutines/coro-eh-cleanup.cpp
@@ -51,7 +51,13 @@
// CHECK: [[EHCLEANUP]]:
// CHECK: %[[INNERPAD:.+]] = cleanuppad within none []
// CHECK: call void @"\01??_DCleanup@@QEAAXXZ"(
-// CHECK: cleanupret from %[[INNERPAD]] unwind label %[[COROENDBB:.+]]
+// CHECK: cleanupret from %4 unwind label %[[CATCHDISPATCH:.+]]
+
+// CHECK: [[CATCHDISPATCH]]:
+// CHECK: catchswitch within none [label %[[CATCHPAD:.+]]] unwind label %[[COROENDBB:.+]]
+// CHECK: [[CATCHPAD]]:
+// CHECK: call void @"\01?unhandled_exception@promise_type@coro_t@@QEAAXXZ"
+
// CHECK: [[COROENDBB]]:
// CHECK-NEXT: %[[CLPAD:.+]] = cleanuppad within none
// CHECK-NEXT: call i1 @llvm.coro.end(i8* null, i1 true) [ "funclet"(token %[[CLPAD]]) ]
@@ -62,8 +68,14 @@
// CHECK-LPAD: to label %[[CONT:.+]] unwind label %[[EHCLEANUP:.+]]
// CHECK-LPAD: [[EHCLEANUP]]:
// CHECK-LPAD:landingpad { i8*, i32 }
-// CHECK-LPAD: cleanup
+// CHECK-LPAD: catch
// CHECK-LPAD: call void @_ZN7CleanupD1Ev(
+// CHECK-LPAD: call i8* @__cxa_begin_catch
+// CHECK-LPAD: call void @_ZN6coro_t12promise_type19unhandled_exceptionEv
+// CHECK-LPAD: invoke void @__cxa_end_catch()
+// CHECK-LPAD: to label %{{.+}} unwind label %[[UNWINDBB:.+]]
+
+// CHECK-LPAD: [[UNWINDBB]]:
// CHECK-LPAD: %[[I1RESUME:.+]] = call i1 @llvm.coro.end(i8* null, i1 true)
// CHECK-LPAD: br i1 %[[I1RESUME]], label %[[EHRESUME:.+]], label
// CHECK-LPAD: [[EHRESUME]]:
Index: test/CodeGenCoroutines/coro-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-cleanup.cpp
+++ test/CodeGenCoroutines/coro-cleanup.cpp
@@ -6,38 +6,38 @@
template struct coroutine_handle {
coroutine_handle() = default;
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
};
template <> struct coroutine_handle {
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
coroutine_handle() = default;
template
- coroutine_handle(coroutine_handle) {}
+ coroutine_handle(coroutine_handle) noexcept;
};
}
struct suspend_always {
- bool await_ready();
- void await_suspend(std::experimental::coroutine_handle<>);
- void await_resume();
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
};
template <> struct std::experimental::coroutine_traits {
struct promise_type {
-void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
-void return_void();
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
promise_type();
~promise_type();
-void unhandled_exception();
+void unhandled_exception() noexcept;
};
};
struct Cleanup { ~Cleanup()
[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch
GorNishanov updated this revision to Diff 99814.
GorNishanov added a comment.
1. Heap allocate CxxTryStmt in CGCoroutine.
2. Merge with trunk
https://reviews.llvm.org/D31692
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-cleanup.cpp
test/CodeGenCoroutines/coro-eh-cleanup.cpp
test/SemaCXX/coroutine-seh.cpp
Index: test/SemaCXX/coroutine-seh.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-seh.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+void SEH_used() {
+ __try { // expected-error {{cannot use SEH '__try' in a coroutine when C++ exceptions are enabled}}
+co_return; // expected-note {{function is a coroutine due to use of 'co_return' here}}
+ } __except(0) {}
+}
Index: test/CodeGenCoroutines/coro-eh-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-eh-cleanup.cpp
+++ test/CodeGenCoroutines/coro-eh-cleanup.cpp
@@ -51,7 +51,13 @@
// CHECK: [[EHCLEANUP]]:
// CHECK: %[[INNERPAD:.+]] = cleanuppad within none []
// CHECK: call void @"\01??_DCleanup@@QEAAXXZ"(
-// CHECK: cleanupret from %[[INNERPAD]] unwind label %[[COROENDBB:.+]]
+// CHECK: cleanupret from %4 unwind label %[[CATCHDISPATCH:.+]]
+
+// CHECK: [[CATCHDISPATCH]]:
+// CHECK: catchswitch within none [label %[[CATCHPAD:.+]]] unwind label %[[COROENDBB:.+]]
+// CHECK: [[CATCHPAD]]:
+// CHECK: call void @"\01?unhandled_exception@promise_type@coro_t@@QEAAXXZ"
+
// CHECK: [[COROENDBB]]:
// CHECK-NEXT: %[[CLPAD:.+]] = cleanuppad within none
// CHECK-NEXT: call i1 @llvm.coro.end(i8* null, i1 true) [ "funclet"(token %[[CLPAD]]) ]
@@ -62,8 +68,14 @@
// CHECK-LPAD: to label %[[CONT:.+]] unwind label %[[EHCLEANUP:.+]]
// CHECK-LPAD: [[EHCLEANUP]]:
// CHECK-LPAD:landingpad { i8*, i32 }
-// CHECK-LPAD: cleanup
+// CHECK-LPAD: catch
// CHECK-LPAD: call void @_ZN7CleanupD1Ev(
+// CHECK-LPAD: call i8* @__cxa_begin_catch
+// CHECK-LPAD: call void @_ZN6coro_t12promise_type19unhandled_exceptionEv
+// CHECK-LPAD: invoke void @__cxa_end_catch()
+// CHECK-LPAD: to label %{{.+}} unwind label %[[UNWINDBB:.+]]
+
+// CHECK-LPAD: [[UNWINDBB]]:
// CHECK-LPAD: %[[I1RESUME:.+]] = call i1 @llvm.coro.end(i8* null, i1 true)
// CHECK-LPAD: br i1 %[[I1RESUME]], label %[[EHRESUME:.+]], label
// CHECK-LPAD: [[EHRESUME]]:
Index: test/CodeGenCoroutines/coro-cleanup.cpp
===
--- test/CodeGenCoroutines/coro-cleanup.cpp
+++ test/CodeGenCoroutines/coro-cleanup.cpp
@@ -6,38 +6,38 @@
template struct coroutine_handle {
coroutine_handle() = default;
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
};
template <> struct coroutine_handle {
- static coroutine_handle from_address(void *) { return {}; }
+ static coroutine_handle from_address(void *) noexcept;
coroutine_handle() = default;
template
- coroutine_handle(coroutine_handle) {}
+ coroutine_handle(coroutine_handle) noexcept;
};
}
struct suspend_always {
- bool await_ready();
- void await_suspend(std::experimental::coroutine_handle<>);
- void await_resume();
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
};
template <> struct std::experimental::coroutine_traits {
struct promise_type {
-void get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend();
-void return_void();
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
promise_type();
~promise_type();
-void unhandled_exception();
+void unhandled_exception() noexcept;
};
};
struct Cleanup { ~
[PATCH] D33357: Avoid calling report_fatal_error in the destructor of raw_fd_ostream when saving a module timestamp file
bruno added a comment. Any idea why we're hitting this issue in the first place? The error that gets cleaned up is reported at some point before? Seems to me that we're going to fail to update the timestamp but continue as nothing happened, I wonder how many other issues this might trigger... > I couldn't think of a way to test this, do you think it's possible to have a > test for this? If you can come up with a testcase it would be awesome, but for this to trigger you'd have to reproduce the concurrent scenario in the testcase, which I don't see how. Repository: rL LLVM https://reviews.llvm.org/D33357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal
ruiu added inline comments.
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+ const char *getDefaultLinker() const override { return "ld.lld"; }
+
jroelofs wrote:
> compnerd wrote:
> > jroelofs wrote:
> > > compnerd wrote:
> > > > I think that this really should be `ld` still, as that is the canonical
> > > > name for the linker.
> > > You mean `lld`?
> > >
> > > ```
> > > $ lld
> > > lld is a generic driver.
> > > Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.
> > > ```
> > >
> > > Or are you saying: "make binutils ld the default, not llvm's lld"?
> > Im saying use the name "ld". ld is a symlink on most linux distributions
> > these days. ld -> ld.gold, ld.bfd, ld.lld
> I don't think this makes sense. I don't care about "most linux
> distributions", but rather about putting together an LLVM baremetal
> distribution based as much as possible on LLVM components. If someone wants a
> different linker, they can use `-fuse-ld=` and/or `-B`.
>
> Also, doesn't using a symlink named `ld` cause `lld` to behave like a mach-o
> linker? We really want the elf one here.
If you invoke lld as ld, it behaves as a Mach-O linker on macOS. On other OSes,
it behaves as an ELF linker.
https://reviews.llvm.org/D33259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 updated this revision to Diff 99802.
jyu2 added a comment.
This is new version should address all @Aaron's commands, but CFG part.
I would not think we should go that far in the compiler for this.
Thanks.
https://reviews.llvm.org/D3
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaExprCXX.cpp
lib/Sema/TreeTransform.h
test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p1.cpp
test/CXX/except/except.spec/p11.cpp
test/SemaCXX/warn-throw-out_noexcept_func.cpp
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -687,6 +687,47 @@
return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope);
}
+static bool isNoexcept(const FunctionDecl * FD)
+{
+ if (const FunctionProtoType *FPT = FD->getType()->castAs())
+if (FPT->getExceptionSpecType() != EST_None &&
+FPT->getNoexceptSpec(FD->getASTContext()) ==
+ FunctionProtoType::NR_Nothrow)
+ return true;
+ return false;
+}
+
+static bool isNoexceptTrue(const FunctionDecl * FD) {
+ // Avoid emitting error twice.
+ if (const auto * TempFD = FD->getTemplateInstantiationPattern())
+if (isNoexcept(TempFD))
+ return false;
+ return isNoexcept(FD);
+}
+
+
+void Sema::CheckCXXThrowInNonThrowingFunc(SourceLocation OpLoc) {
+ bool isInCXXTryBlock = false;
+ for (Scope *S = getCurScope(); S; S = S->getParent())
+if (S->isTryScope()) {
+ isInCXXTryBlock = true;
+ break;
+} else if (S->isFunctionScope())
+ break;
+ if (const FunctionDecl *FD = getCurFunctionDecl())
+if (!isInCXXTryBlock && !getSourceManager().isInSystemHeader(OpLoc))
+ if (isNoexceptTrue(FD)) {
+Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD;
+if (getLangOpts().CPlusPlus11 &&
+(isa(FD) ||
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Delete ||
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete))
+ Diag(FD->getLocation(), diag::note_throw_in_dtor);
+else
+ Diag(FD->getLocation(), diag::note_throw_in_function);
+ }
+}
+
ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
bool IsThrownVarInScope) {
// Don't report an error if 'throw' is used in system headers.
@@ -702,6 +743,8 @@
if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope())
Diag(OpLoc, diag::err_omp_simd_region_cannot_use_stmt) << "throw";
+ CheckCXXThrowInNonThrowingFunc(OpLoc);
+
if (Ex && !Ex->isTypeDependent()) {
QualType ExceptionObjectTy = Context.getExceptionObjectType(Ex->getType());
if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -9873,9 +9873,10 @@
if (SubExpr.isInvalid())
return ExprError();
- if (!getDerived().AlwaysRebuild() &&
- SubExpr.get() == E->getSubExpr())
+ if (!getDerived().AlwaysRebuild() && SubExpr.get() == E->getSubExpr()) {
+getSema().CheckCXXThrowInNonThrowingFunc(E->getThrowLoc());
return E;
+ }
return getDerived().RebuildCXXThrowExpr(E->getThrowLoc(), SubExpr.get(),
E->isThrownVariableInScope());
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -4967,6 +4967,9 @@
ExprResult BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
bool IsThrownVarInScope);
bool CheckCXXThrowOperand(SourceLocation ThrowLoc, QualType ThrowTy, Expr *E);
+ /// Check if throw is used in function with a non-throwing noexcept
+ /// specifier.
+ void CheckCXXThrowInNonThrowingFunc(SourceLocation OpLoc);
/// ActOnCXXTypeConstructExpr - Parse construction of a specified type.
/// Can be interpreted either as function-style casting ("int(x)")
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6331,6 +6331,15 @@
"cannot use '%0' with exceptions disabled">;
def err_objc_exceptions_disabled : Error<
"cannot use '%0' with Objective-C exceptions disabled">;
+def warn_throw_in_noexcept_func
+: Warning<"%0 has a non-throwing exception specification but can still "
+ "throw, may result in unexpected program termination.">,
+ InGroup;
+def note_throw_in_dtor
+: Note<"destructor or deallocator has a (possible implicit) non-throwing "
+ "excepton specification">;
+def note_throw_in_function
+: Note<"nonthrowing function declare here">;
def err_seh_try_outside_functions : Error<
"cannot use SEH '__try' in blocks, capt
[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal
jroelofs added inline comments.
Comment at: cmake/caches/BaremetalARM.cmake:1
+set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "")
+
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > Please rename this file to `BareMetalARMv6.cmake`. (I'm interested in
> > > the suffix primarily).
> > My plan is to eventually add multilibs to this configuration, so while that
> > makes sense short term, I don't think it makes sense long term.
> >
> > With that in mind, do you still want me to rename it?
> I think it makes sense longer term still. ARMv6 vs ARMv7. Even if you do
> multilib, that wouldnt take care of that right?
It's not limited to just ARMv6m multilibs... imagine it had:
```
set(LLVM_BUILTIN_TARGETS "armv6m-none-eabi;armv7m-none-eabi" CACHE STRING
"Builtin Targets")
```
Then naming the file `BaremetalARMv6m.cmake` will be inappropriately specific.
Granted, the way clangrt is built/used now isn't very conducive to multilibs
that differ by more than just the arch, since the name currently has to be of
the form: `libclang_rt-builtins-${triple's subarch}.a`, which for example
doesn't allow for differentiating between:
```
v7m;@mthumb@march=armv7-m
v7m-PIC;@mthumb@march=armv7-m@fPIC
```
or
```
v7a-neon;@march=armv7-a@mfloat-abi=softfp@mfpu=neon
v7a-vfpv3-d16;@march=armv7-a@mfloat-abi=softfp@mfpu=vfpv3-d16
```
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "lib", "baremetal");
+ return Dir.str();
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > Why not just the standard `arm` directory?
> > There are a few differences between the stuff in the existing ones, and
> > what is needed on baremetal. For example __enable_execute_stack, emutls, as
> > well as anything else that assumes existence of pthreads support shouldn't
> > be there.
> Well, I think that "baremetal" here is a bad idea. How about using the
> android approach? Use `clang_rt.builtins-arm-baremetal.a` ?
Why? Given the way the cmake goop works in lib/runtimes + compiler-rt, the
folder name there has to be the same as the CMAKE_SYSTEM_NAME. The alternative,
I guess, is to call it 'generic', but I'm not convinced that's better than
'baremetal'.
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+ const char *getDefaultLinker() const override { return "ld.lld"; }
+
compnerd wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > I think that this really should be `ld` still, as that is the canonical
> > > name for the linker.
> > You mean `lld`?
> >
> > ```
> > $ lld
> > lld is a generic driver.
> > Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.
> > ```
> >
> > Or are you saying: "make binutils ld the default, not llvm's lld"?
> Im saying use the name "ld". ld is a symlink on most linux distributions
> these days. ld -> ld.gold, ld.bfd, ld.lld
I don't think this makes sense. I don't care about "most linux distributions",
but rather about putting together an LLVM baremetal distribution based as much
as possible on LLVM components. If someone wants a different linker, they can
use `-fuse-ld=` and/or `-B`.
Also, doesn't using a symlink named `ld` cause `lld` to behave like a mach-o
linker? We really want the elf one here.
https://reviews.llvm.org/D33259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30170: Function definition may have uninstantiated body
rsmith added a comment.
Do we really need two different notions of "definition" and "odr definition"
here? What goes wrong if we always treat the "instantiation of a friend
function definition" case as being a definition?
Comment at: include/clang/AST/Decl.h:1789
+ // function is not used yet, but the declaration is considered a
+ // definition, it does not allow other definition of this function.
+ // - it does not have a user specified body, but it does not allow
it -> and
Comment at: include/clang/AST/Decl.h:1829-1830
+ /// Returns true if the function is defined and other definitions are not
+ /// allowed in the redeclaration chain.
+ ///
This will not be true in the presence of modules: we will ensure that at most
one declaration has a body, but multiple declarations can be instantiated from
a defined member function of a class template, for instance. So I think the
constraint is that you're not allowed to add *more* such definitions.
Comment at: include/clang/AST/Decl.h:1842
+
+ /// Returns true if this declaration is a definition is the sense of ODR
+ /// checks.
is the sense -> in the sense
Comment at: include/clang/AST/Decl.h:1848
+ return true;
+if (FunctionDecl *Original = getInstantiatedFromMemberFunction())
+ return Original->isOdrDefined();
I think this will do the wrong thing for an explicit specialization of a member
of a class template:
```
template struct A { void f() {}; };
template<> void A::f();
```
Here, `A::f()` is "instantiated from a member function" but it's not a
definition, not even in the ODR sense. I think it would suffice to also check
whether `Original` is a friend declaration here.
Comment at: lib/AST/Decl.cpp:2538
+if (I->isThisDeclarationADefinition()) {
+ Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
+ return true;
It seems incorrect to me for `isThisDeclarationADefinition()` to return `true`
on a redeclaration of a deleted function: only the first declaration should be
considered to be a definition in that case. (This special case should be in
`isThisDeclarationADefinition()`, not in `isDefined()`.)
Comment at: lib/Sema/SemaDecl.cpp:11882
+
+ if (FD == Definition)
+return;
This looks wrong to me: `CheckForFunctionRedefinition` is intended to be called
*before* marking the function in question as a definition. If you call it
afterwards, then the `isOdrDefined` call won't work properly because it will
always check to see whether `FD` is a definition before checking any other
declaration. So any case that reaches here with `FD` being a definition seems
like a bug. I expect we're only getting here in that case due to the missing
check for friend in `isThisDeclarationAnOdrDefinition`.
https://reviews.llvm.org/D30170
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement
This revision was automatically updated to reflect the committed changes.
GorNishanov marked an inline comment as done.
Closed by commit rL303573: [coroutines] Build GRO declaration and return GRO
statement (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D31646?vs=99796&id=99798#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31646
Files:
cfe/trunk/include/clang/AST/StmtCXX.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/StmtCXX.cpp
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/CoroutineStmtBuilder.h
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Index: cfe/trunk/include/clang/AST/StmtCXX.h
===
--- cfe/trunk/include/clang/AST/StmtCXX.h
+++ cfe/trunk/include/clang/AST/StmtCXX.h
@@ -308,7 +308,9 @@
OnFallthrough, ///< Handler for control flow falling off the body.
Allocate, ///< Coroutine frame memory allocation.
Deallocate,///< Coroutine frame memory deallocation.
-ReturnValue, ///< Return value for thunk function.
+ReturnValue, ///< Return value for thunk function: p.get_return_object().
+ResultDecl,///< Declaration holding the result of get_return_object.
+ReturnStmt,///< Return statement for the thunk function.
ReturnStmtOnAllocFailure, ///< Return statement if allocation failed.
FirstParamMove ///< First offset for move construction of parameter copies.
};
@@ -332,7 +334,9 @@
Stmt *OnFallthrough = nullptr;
Expr *Allocate = nullptr;
Expr *Deallocate = nullptr;
-Stmt *ReturnValue = nullptr;
+Expr *ReturnValue = nullptr;
+Stmt *ResultDecl = nullptr;
+Stmt *ReturnStmt = nullptr;
Stmt *ReturnStmtOnAllocFailure = nullptr;
ArrayRef ParamMoves;
};
@@ -381,10 +385,11 @@
Expr *getDeallocate() const {
return cast_or_null(getStoredStmts()[SubStmt::Deallocate]);
}
-
Expr *getReturnValueInit() const {
-return cast_or_null(getStoredStmts()[SubStmt::ReturnValue]);
+return cast(getStoredStmts()[SubStmt::ReturnValue]);
}
+ Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; }
+ Stmt *getReturnStmt() const { return getStoredStmts()[SubStmt::ReturnStmt]; }
Stmt *getReturnStmtOnAllocFailure() const {
return getStoredStmts()[SubStmt::ReturnStmtOnAllocFailure];
}
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8910,6 +8910,8 @@
"return statement not allowed in coroutine; did you mean 'co_return'?">;
def note_declared_coroutine_here : Note<
"function is a coroutine due to use of '%0' here">;
+def note_promise_member_declared_here : Note<
+ "'%0' is declared here">;
def err_coroutine_objc_method : Error<
"Objective-C methods as coroutines are not yet supported">;
def err_coroutine_unevaluated_context : Error<
Index: cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
@@ -156,14 +156,24 @@
// CHECK-LABEL: f4(
extern "C" int f4(promise_on_alloc_failure_tag) {
+ // CHECK: %[[RetVal:.+]] = alloca i32
// CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
// CHECK: %[[MEM:.+]] = call i8* @_ZnwmRKSt9nothrow_t(i64 %[[SIZE]], %"struct.std::nothrow_t"* dereferenceable(1) @_ZStL7nothrow)
// CHECK: %[[OK:.+]] = icmp ne i8* %[[MEM]], null
// CHECK: br i1 %[[OK]], label %[[OKBB:.+]], label %[[ERRBB:.+]]
// CHECK: [[ERRBB]]:
- // CHECK: %[[RETVAL:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type39get_return_object_on_allocation_failureEv(
- // CHECK: ret i32 %[[RETVAL]]
+ // CHECK: %[[FailRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type39get_return_object_on_allocation_failureEv(
+ // CHECK: store i32 %[[FailRet]], i32* %[[RetVal]]
+ // CHECK: br label %[[RetBB:.+]]
+
+ // CHECK: [[OKBB]]:
+ // CHECK: %[[OkRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type17get_return_objectEv(
+ // CHECK: store i32 %[[OkRet]], i32* %[[RetVal]]
+
+ // CHECK: [[RetBB]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]], align 4
+ // CHECK: ret i32 %[[LoadRet]]
co_return;
}
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.c
r303573 - [coroutines] Build GRO declaration and return GRO statement
Author: gornishanov
Date: Mon May 22 15:22:23 2017
New Revision: 303573
URL: http://llvm.org/viewvc/llvm-project?rev=303573&view=rev
Log:
[coroutines] Build GRO declaration and return GRO statement
Summary:
1. build declaration of the gro local variable that keeps the result of
get_return_object.
2. build return statement returning the gro variable
3. emit them during CodeGen
4. sema and CodeGen tests updated
Reviewers: EricWF, rsmith
Reviewed By: rsmith
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D31646
Modified:
cfe/trunk/include/clang/AST/StmtCXX.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/StmtCXX.cpp
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/CoroutineStmtBuilder.h
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Modified: cfe/trunk/include/clang/AST/StmtCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtCXX.h?rev=303573&r1=303572&r2=303573&view=diff
==
--- cfe/trunk/include/clang/AST/StmtCXX.h (original)
+++ cfe/trunk/include/clang/AST/StmtCXX.h Mon May 22 15:22:23 2017
@@ -308,7 +308,9 @@ class CoroutineBodyStmt final
OnFallthrough, ///< Handler for control flow falling off the body.
Allocate, ///< Coroutine frame memory allocation.
Deallocate,///< Coroutine frame memory deallocation.
-ReturnValue, ///< Return value for thunk function.
+ReturnValue, ///< Return value for thunk function: p.get_return_object().
+ResultDecl,///< Declaration holding the result of get_return_object.
+ReturnStmt,///< Return statement for the thunk function.
ReturnStmtOnAllocFailure, ///< Return statement if allocation failed.
FirstParamMove ///< First offset for move construction of parameter copies.
};
@@ -332,7 +334,9 @@ public:
Stmt *OnFallthrough = nullptr;
Expr *Allocate = nullptr;
Expr *Deallocate = nullptr;
-Stmt *ReturnValue = nullptr;
+Expr *ReturnValue = nullptr;
+Stmt *ResultDecl = nullptr;
+Stmt *ReturnStmt = nullptr;
Stmt *ReturnStmtOnAllocFailure = nullptr;
ArrayRef ParamMoves;
};
@@ -381,10 +385,11 @@ public:
Expr *getDeallocate() const {
return cast_or_null(getStoredStmts()[SubStmt::Deallocate]);
}
-
Expr *getReturnValueInit() const {
-return cast_or_null(getStoredStmts()[SubStmt::ReturnValue]);
+return cast(getStoredStmts()[SubStmt::ReturnValue]);
}
+ Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; }
+ Stmt *getReturnStmt() const { return getStoredStmts()[SubStmt::ReturnStmt]; }
Stmt *getReturnStmtOnAllocFailure() const {
return getStoredStmts()[SubStmt::ReturnStmtOnAllocFailure];
}
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303573&r1=303572&r2=303573&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May 22 15:22:23
2017
@@ -8910,6 +8910,8 @@ def err_return_in_coroutine : Error<
"return statement not allowed in coroutine; did you mean 'co_return'?">;
def note_declared_coroutine_here : Note<
"function is a coroutine due to use of '%0' here">;
+def note_promise_member_declared_here : Note<
+ "'%0' is declared here">;
def err_coroutine_objc_method : Error<
"Objective-C methods as coroutines are not yet supported">;
def err_coroutine_unevaluated_context : Error<
Modified: cfe/trunk/lib/AST/StmtCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtCXX.cpp?rev=303573&r1=303572&r2=303573&view=diff
==
--- cfe/trunk/lib/AST/StmtCXX.cpp (original)
+++ cfe/trunk/lib/AST/StmtCXX.cpp Mon May 22 15:22:23 2017
@@ -88,7 +88,7 @@ const VarDecl *CXXForRangeStmt::getLoopV
}
CoroutineBodyStmt *CoroutineBodyStmt::Create(
-const ASTContext &C, CoroutineBodyStmt::CtorArgs const& Args) {
+const ASTContext &C, CoroutineBodyStmt::CtorArgs const &Args) {
std::size_t Size = totalSizeToAlloc(
CoroutineBodyStmt::FirstParamMove + Args.ParamMoves.size());
@@ -108,6 +108,8 @@ CoroutineBodyStmt::CoroutineBodyStmt(Cor
SubStmts[CoroutineBodyStmt::Allocate] = Args.Allocate;
SubStmts[CoroutineBodyStmt::Deallocate] = Args.Deallocate;
SubStmts[CoroutineBodyStmt::ReturnValue] = Args.ReturnValue;
+ SubStmts[CoroutineBodyStmt::ResultDecl] = Args.ResultDecl;
+ SubStmts[CoroutineBodyStmt::ReturnStmt] = Args.ReturnStmt;
SubStmts[CoroutineBodyStmt::ReturnStmtOnAllocFailure] =
Args.ReturnStmtOnAllocFailure;
std
[PATCH] D31627: [coroutines] Skip over passthrough operator co_await
GorNishanov added a comment. In https://reviews.llvm.org/D31627#761320, @rsmith wrote: > Does it still make sense for us to have a `UO_Coawait` at all? As I recall, > the only purpose it ever had was to represent a dependent `co_await` > expression that couldn't yet be resolved to a `CoawaitExpr`. But now we have > (and need!) a separate `DependentCoawaitExpr` node to store unqualified > lookup results, it seems that the `UnaryOperator` portion of the > representation serves no purpose (and as seen in this patch, it's getting in > the way). Can we remove it? > > Anyway, this change LGTM for now. Thank you for the review! I'll see if I can remove UO_Coawait in a separate patch https://reviews.llvm.org/D31627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement
GorNishanov updated this revision to Diff 99796.
GorNishanov added a comment.
Thank you very much for the review!
Merged with top of the trunk, implemented suggested changes, preparing to land
https://reviews.llvm.org/D31646
Files:
include/clang/AST/StmtCXX.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/StmtCXX.cpp
lib/CodeGen/CGCoroutine.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/CoroutineStmtBuilder.h
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-alloc.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -746,3 +746,75 @@
co_return;
}
template coro dependent_uses_nothrow_new(good_promise_13);
+
+struct mismatch_gro_type_tag1 {};
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() {} //expected-note {{'get_return_object' is declared here}}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+void unhandled_exception();
+ };
+};
+
+extern "C" int f(mismatch_gro_type_tag1) {
+ // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
+ co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
+}
+
+struct mismatch_gro_type_tag2 {};
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+void* get_return_object() {} //expected-note {{'get_return_object' is declared here}}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+void unhandled_exception();
+ };
+};
+
+extern "C" int f(mismatch_gro_type_tag2) {
+ // expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
+ co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
+}
+
+struct mismatch_gro_type_tag3 {};
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+int get_return_object() {}
+static void get_return_object_on_allocation_failure() {} //expected-note {{'get_return_object_on_allocation_failure' is declared here}}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+void unhandled_exception();
+ };
+};
+
+extern "C" int f(mismatch_gro_type_tag3) {
+ // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
+ co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
+}
+
+
+struct mismatch_gro_type_tag4 {};
+template<>
+struct std::experimental::coroutine_traits {
+ struct promise_type {
+int get_return_object() {}
+static char* get_return_object_on_allocation_failure() {} //expected-note {{'get_return_object_on_allocation_failure' is declared here}}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+void unhandled_exception();
+ };
+};
+
+extern "C" int f(mismatch_gro_type_tag4) {
+ // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
+ co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
+}
+
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -156,14 +156,24 @@
// CHECK-LABEL: f4(
extern "C" int f4(promise_on_alloc_failure_tag) {
+ // CHECK: %[[RetVal:.+]] = alloca i32
// CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
// CHECK: %[[MEM:.+]] = call i8* @_ZnwmRKSt9nothrow_t(i64 %[[SIZE]], %"struct.std::nothrow_t"* dereferenceable(1) @_ZStL7nothrow)
// CHECK: %[[OK:.+]] = icmp ne i8* %[[MEM]], null
// CHECK: br i1 %[[OK]], label %[[OKBB:.+]], label %[[ERRBB:.+]]
// CHECK: [[ERRBB]]:
- // CHECK: %[[RETVAL:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type39get_return_object_on_allocation_failureEv(
- // CHECK: ret i32 %[[RETVAL]]
+ // CHECK: %[[FailRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type39get_return_object_on_allocation_failureEv(
+ // CHECK: store i32 %[[FailRet]], i32* %[[RetVal]]
+ // CHECK: br label %[[RetBB:.+]]
+
+ // CHECK: [[OKBB]]:
+ // CHECK: %[[OkRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type17get_return_objectEv(
+ // CHECK: store i32 %[[OkRet]], i32* %[[RetVal]]
+
+ // CHECK: [[RetBB]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVa
[PATCH] D31627: [coroutines] Skip over passthrough operator co_await
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Does it still make sense for us to have a `UO_Coawait` at all? As I recall, the only purpose it ever had was to represent a dependent `co_await` expression that couldn't yet be resolved to a `CoawaitExpr`. But now we have (and need!) a separate `DependentCoawaitExpr` node to store unqualified lookup results, it seems that the `UnaryOperator` portion of the representation serves no purpose (and as seen in this patch, it's getting in the way). Can we remove it? Anyway, this change LGTM for now. Comment at: lib/CodeGen/CGCoroutine.cpp:146 + + // Skip paththrough operator co_await (present when awaiting on an LValue). + if (auto *UO = dyn_cast(E)) paththrough -> passthrough Comment at: lib/CodeGen/CGCoroutine.cpp:148-149 + if (auto *UO = dyn_cast(E)) + if (UO->getOpcode() == UO_Coawait) +E = UO->getSubExpr(); + Indent. https://reviews.llvm.org/D31627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.
NoQ added a comment. Thanks, this is great! Two more things: - You have touched other code, unrelated to your patch, with clang-format; we're usually trying to avoid that, because it creates merge conflicts out of nowhere, and because some of that code actually seems formatted by hand intentionally. It's better to revert these changes; you can use the `git clang-format` thing to format only actual changes. - Updating .gitignore sounds like the right thing to do (llvm's .gitignore already has this), but i guess we'd better make a separate commit for that. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:143-146 +if(lstate->isSchrodingerUntouched()) + state = state->remove(lockR); +else if(lstate->isSchrodingerUnlocked()) + state = state->set(lockR, LockState::getUnlocked()); malhar1995 wrote: > NoQ wrote: > > I think we can be certain that the lock is in one of these states, and > > assert that. > We can be certain that the lock state will be either of the two only if I add > the following statement before returning from this function. > ``` > state = state->remove(lockR); > ``` > If I don't add the above statement, a return value symbol for the region > specified by lockR will still be in DestroyRetVal and it may have an actual > lock state (locked, unlocked or destroyed). Yep, that's a great thing to do. I didn't notice this. Generally, it's great to keep the program state free from stuff that would no longer be necessary. Repository: rL LLVM https://reviews.llvm.org/D32449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31692: [coroutines] Wrap the body of the coroutine in try-catch
rsmith added inline comments.
Comment at: include/clang/AST/StmtCXX.h:128
+struct CXXTryStmt::OnStack : CXXTryStmt {
+ alignas(CXXTryStmt) Stmt *Stmts[2];
+ OnStack(SourceLocation tryLoc, Stmt *tryBlock, Stmt *handler)
This makes more assumptions about record layout and the ability to reach
derived-class members through pointer arithmetic than I'd prefer. We're only
going to have at most one of these per coroutine for which we emit an IR
definition, so it seems fine for CodeGen to just go ahead and allocate one in
the normal way.
Alternatively, it looks like it'd be straightforward to refactor `CodeGen`'s
`EnterCXXTryStmt` / `ExitCXXTryStmt` to take an `ArrayRef` instead
of a `CXXTryStmt`, which would let you avoid this machinery entirely.
https://reviews.llvm.org/D31692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
rsmith added inline comments.
Comment at: lib/CodeGen/CGCoroutine.cpp:273
+
+// FIXME: There must be a cleaner way to do this!
+if (auto *Cleanup =
dyn_cast_or_null(&*CGF.EHStack.begin())) {
It doesn't seem safe to assume that a prior `EHCleanupScope` would be for the
GRO variable, nor that it can only have one such cleanup. How about this: grab
a stable EHStack iterator before you emit cleanups, grab another afterwards,
and iterate over the cleanups in that range modifying them as appropriate.
Even that seems a bit fragile, though. Would it be feasible to thread the
'active' flag through `EmitAutoVarCleanups` (perhaps add it to
`AutoVarEmission`)?
https://reviews.llvm.org/D31670
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21508: Make friend function template definition available if class is instantiated.
sepavloff updated this revision to Diff 99789.
sepavloff edited the summary of this revision.
sepavloff added a comment.
Updated patch
https://reviews.llvm.org/D21508
Files:
include/clang/AST/DeclTemplate.h
lib/AST/Decl.cpp
lib/AST/DeclTemplate.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/SemaCXX/friend2.cpp
Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -129,6 +129,83 @@
void func_22() {} // expected-error{{redefinition of 'func_22'}}
+// Case of template friend functions.
+
+template void func_31(T *x);
+template
+struct C31a {
+ template friend void func_31(T *x) {}
+};
+template
+struct C31b {
+ template friend void func_31(T *x) {}
+};
+
+
+template inline void func_32(T *x) {}
+template
+struct C32a {
+ template friend void func_32(T *x) {}
+};
+template
+struct C32b {
+ template friend void func_32(T *x) {}
+};
+
+
+template
+struct C33a {
+ template friend void func_33(T *x) {}
+};
+template
+struct C33b {
+ template friend void func_33(T *x) {}
+};
+
+
+template inline void func_34(T *x) {} // expected-note{{previous definition is here}}
+template
+struct C34 {
+ template friend void func_34(T *x) {} // expected-error{{redefinition of 'func_34'}}
+};
+
+C34 v34; // expected-note{{in instantiation of template class 'C34' requested here}}
+
+
+template inline void func_35(T *x);
+template
+struct C35a {
+ template friend void func_35(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C35b {
+ template friend void func_35(T *x) {} // expected-error{{redefinition of 'func_35'}}
+};
+
+C35a v35a;
+C35b v35b; // expected-note{{in instantiation of template class 'C35b' requested here}}
+
+
+template void func_36(T *x);
+template
+struct C36 {
+ template friend void func_36(T *x) {} // expected-error{{redefinition of 'func_36'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C36 v36a;
+C36 v36b; //expected-note{{in instantiation of template class 'C36' requested here}}
+
+
+template void func_37(T *x);
+template
+struct C37 {
+ template friend void func_37(T *x) {} // expected-note{{previous definition is here}}
+};
+
+C37 v37;
+template void func_37(T *x) {} // expected-error{{redefinition of 'func_37'}}
+
namespace pr22307 {
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1792,7 +1792,9 @@
// If the original function was part of a friend declaration,
// inherit its namespace state and add it to the owner.
if (isFriend) {
-PrincipalDecl->setObjectOfFriendDecl();
+Function->setObjectOfFriendDecl();
+if (FunctionTemplate)
+ FunctionTemplate->setObjectOfFriendDecl();
DC->makeDeclVisibleInContext(PrincipalDecl);
bool QueuedInstantiation = false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11874,9 +11874,35 @@
const FunctionDecl *EffectiveDefinition,
SkipBodyInfo *SkipBody) {
const FunctionDecl *Definition = EffectiveDefinition;
- if (!Definition)
-if (!FD->isOdrDefined(Definition))
+ if (!Definition) {
+if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate()) {
+ FunctionTemplateDecl *DefTD = nullptr;
+ if (!FTD->isDefined(DefTD))
+return;
+ const FunctionDecl *Def = DefTD->getTemplatedDecl();
+
+ // If the found definition is a template with uninstantiated body, it
+ // can be replaced in specialization:
+ //
+ //template struct X {
+ // template void f(T, U) { }
+ //};
+ //template<> template void X::f(int x, U y) {
+ // x = y;
+ //}
+ //
+ // In this example the specialization 'X' contains declaration of
+ // 'f', which is considered a definition by 'isDefined' because it is
+ // obtained by instantiation of 'X::f', which has a body.
+ //
+ if (isa(Def->getLexicalDeclContext()) &&
+ !Def->isThisDeclarationADefinition() &&
+ Def->getFriendObjectKind() == Decl::FOK_None)
+return;
+ Definition = Def;
+} else if (!FD->isOdrDefined(Definition))
return;
+ }
assert(Definition);
if (FD == Definition)
Index: lib/AST/DeclTemplate.cpp
===
--- lib/AST/DeclTemplate.cpp
+++ lib/AST/DeclTemplate.cpp
@@ -231,6 +231,30 @@
return CommonPtr;
}
+bool FunctionTemplateDecl::isDefined(FunctionTemplateDecl *&Def) const {
+ FunctionTemplateDecl *UninstantiatedDef = nullptr;
+ for (auto I : redecls()) {
+auto FTD =
[PATCH] D21508: Make friend function template definition available if class is instantiated.
sepavloff marked an inline comment as done.
sepavloff added inline comments.
Comment at: lib/AST/DeclTemplate.cpp:292-311
+FunctionTemplateDecl *FunctionTemplateDecl::getDefinition() const {
+ for (auto *R : redecls()) {
+FunctionTemplateDecl *F = cast(R);
+if (F->isThisDeclarationADefinition())
+ return F;
+
+// If template does not have a body, probably it is instantiated from
rsmith wrote:
> It seems to me that we're getting something fundamentally wrong in the way we
> model friend function definitions where we've instantiated the declaration
> but not the definition. Here's a case we get wrong on the
> non-function-template side:
>
> template struct X { friend void f() {} };
> X xi;
> void f() {}
>
> Line 3 should be ill-formed here due to redefinition, but we don't detect
> that because we don't believe that the declaration we instantiated on line 2
> is a definition. That seems to be the heart of the problem.
>
> Instead of adding this function, can you try changing
> `FunctionDecl::isThisDeclarationADefinition` to contain the above logic (that
> is: if this function is a friend that was instantiated from a definition,
> then it's a definition even if we don't have a body yet)?
The code snippet miscompiles due to another problem, it is related to friend
functions, not function templates, and is addressed by D30170.
> Instead of adding this function, can you try changing
> FunctionDecl::isThisDeclarationADefinition to contain the above logic
`FunctionDecl::isThisDeclarationADefinition` is used in many places, in some it
should not consider uninstantiated bodies. Making special function for such
checks makes the fix more compact and reduces the risk to break existing code.
For function declarations it is `isThisDeclarationAnOdrDefinition` introduced
in D30170, for function templates `isDefined` is created in the updated version
of this patch.
Comment at: lib/Sema/SemaDecl.cpp:8856
+ (NewTemplateDecl->getFriendObjectKind() != Decl::FOK_None ||
+ OldTemplateDecl->getFriendObjectKind() != Decl::FOK_None))
+if (FunctionTemplateDecl *NewDef = NewTemplateDecl->getDefinition())
rsmith wrote:
> This doesn't look right; the `OldTemplateDecl` might be `FOK_None` but could
> still have a definition as a `friend`.
>
> I'm not convinced this is the right place for this check. There are two
> different cases here:
>
> 1) The redefinition error is introduced by parsing a definition in the source
> code. That should already be handled by `ActOnStartOfFunctionDef`.
> 2) The redefinition error is introduced by instantiating a friend function
> template declaration (that has a definition).
>
> I think check (2) belongs in the template instantiation code rather than
> here. In fact, at the end of `TemplateDeclInstantiator::VisitFunctionDecl`,
> there is an attempt to enforce the relevant rule; it just doesn't get all the
> cases right.
Indeed, marking both `FunctionTemplateDecl` and corresponding `FunctionDecl` as
friends was enough to make `TemplateDeclInstantiator::VisitFunctionDecl` work
properly.
As for the case (1), the check was moved to `CheckForFunctionRedefinition`,
which is eventually called from `ActOnStartOfFunctionDef`.
https://reviews.llvm.org/D21508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Sema/AnalysisBasedWarnings.cpp:338 + // In a coroutine, only co_return statements count as normal returns. Remember + // if we are processing the coroutine or not. + const bool IsCoroutine = isa(AC.getBody()); the -> a Comment at: lib/Sema/SemaCoroutine.cpp:1079-1083 + // FIXME: ActOnReturnStmt expects a scope that is inside of the function, due + // to CheckJumpOutOfSEHFinally(*this, ReturnLoc, *CurScope->getFnParent()); + // S.getCurScope()->getFnParent() == nullptr at ActOnFinishFunctionBody when + // CoroutineBodyStmt is built. Figure it out and fix it. + // Use BuildReturnStmt here to unbreak sanitized tests. (Gor:3/27/2017) I'm not sure you need this FIXME; using `BuildReturnStmt` here instead of `ActOnReturnStmt` to skip the checks that we apply to parsed return statements but not instantiated return statements seems appropriate to me, so there's probably nothing to fix. https://reviews.llvm.org/D31646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D31414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 added a comment. In https://reviews.llvm.org/D3#761233, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#761224, @rnk wrote: > > > Re: clang-tidy, I would rather implement this as a traditional compiler > > warning. > > > > In https://reviews.llvm.org/D3#761208, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D3#761126, @jyu2 wrote: > > > > > > > As I said, I don't think checking throw type matching catch handler > > > > type is compiler's job. I'd like either silence all warning for that. > > > > What do you think? > > > > > > > > > Consider the following cases: > > > ... > > > I think the expected behavior in these cases is reasonable (and can be > > > done with a CFG) rather than manually looking at the scope stack like > > > you're doing. > > > > > > I agree with @jyu2, we should do something simple. I think it would be > > simple to handle all cases except for `h`, which requires semantic analysis > > to figure out if the thrown object would be caught by the catch parameter. > > Implementing that is more likely to introduce bugs in the compiler than it > > is to find bugs in user code. > > > I'm not certain I agree with the assertion it's more likely to introduce bugs > in the compiler than find bugs in user code. Machine-generated code runs into > silly things like this, and it's nice to warn the user about it. However, it > may be reasonable to do that in a follow-up patch -- but that patch is likely > to have a lot of churn since it's not really plausible to implement with the > way @jyu2 has structured this patch. By making this an analysis-based warning > instead, that would alleviate my concerns (but would likely require > implementing the CFG approach anyway). > > > I doubt we need the CFG for any of this. The later examples are all > > throwing exceptions from catch blocks, which is the same as not having an > > open try scope. > > Sure, if you cut out the cases that require a CFG, you don't need a CFG for > it. :-P Use of a CFG would eliminate obvious false-positives such as `h()`. > > Also, there should be some tests with function-try-blocks. Sure, I will add that. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
aaron.ballman added a comment. In https://reviews.llvm.org/D3#761224, @rnk wrote: > Re: clang-tidy, I would rather implement this as a traditional compiler > warning. > > In https://reviews.llvm.org/D3#761208, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D3#761126, @jyu2 wrote: > > > > > As I said, I don't think checking throw type matching catch handler type > > > is compiler's job. I'd like either silence all warning for that. What > > > do you think? > > > > > > Consider the following cases: > > ... > > I think the expected behavior in these cases is reasonable (and can be > > done with a CFG) rather than manually looking at the scope stack like > > you're doing. > > > I agree with @jyu2, we should do something simple. I think it would be simple > to handle all cases except for `h`, which requires semantic analysis to > figure out if the thrown object would be caught by the catch parameter. > Implementing that is more likely to introduce bugs in the compiler than it is > to find bugs in user code. I'm not certain I agree with the assertion it's more likely to introduce bugs in the compiler than find bugs in user code. Machine-generated code runs into silly things like this, and it's nice to warn the user about it. However, it may be reasonable to do that in a follow-up patch -- but that patch is likely to have a lot of churn since it's not really plausible to implement with the way @jyu2 has structured this patch. By making this an analysis-based warning instead, that would alleviate my concerns (but would likely require implementing the CFG approach anyway). > I doubt we need the CFG for any of this. The later examples are all throwing > exceptions from catch blocks, which is the same as not having an open try > scope. Sure, if you cut out the cases that require a CFG, you don't need a CFG for it. :-P Use of a CFG would eliminate obvious false-positives such as `h()`. Also, there should be some tests with function-try-blocks. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
faisalv marked 2 inline comments as done. faisalv added a comment. mark aaron's feedback as done. https://reviews.llvm.org/D31414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
faisalv updated this revision to Diff 99787.
faisalv added a comment.
Incorporated Aaron's feedback - thanks Aaron!
https://reviews.llvm.org/D31414
Files:
include/clang/Basic/TemplateKinds.h
include/clang/Sema/ParsedTemplate.h
lib/Parse/ParseExprCXX.cpp
lib/Parse/ParseTemplate.cpp
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1011,25 +1011,21 @@
// Build a template-id annotation token that can be processed
// later.
Tok.setKind(tok::annot_template_id);
-TemplateIdAnnotation *TemplateId
- = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
-TemplateId->TemplateNameLoc = TemplateNameLoc;
-if (TemplateName.getKind() == UnqualifiedId::IK_Identifier) {
- TemplateId->Name = TemplateName.Identifier;
- TemplateId->Operator = OO_None;
-} else {
- TemplateId->Name = nullptr;
- TemplateId->Operator = TemplateName.OperatorFunctionId.Operator;
-}
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); Arg != ArgEnd; ++Arg)
- Args[Arg] = ParsedTemplateArgument(TemplateArgs[Arg]);
+
+IdentifierInfo *TemplateII =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? TemplateName.Identifier
+: nullptr;
+
+OverloadedOperatorKind OpKind =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: TemplateName.OperatorFunctionId.Operator;
+
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+ SS, TemplateKWLoc, TemplateNameLoc, TemplateII, OpKind, Template, TNK,
+ LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
Tok.setAnnotationValue(TemplateId);
if (TemplateKWLoc.isValid())
Tok.setLocation(TemplateKWLoc);
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2120,31 +2120,18 @@
Id.getKind() == UnqualifiedId::IK_LiteralOperatorId) {
// Form a parsed representation of the template-id to be stored in the
// UnqualifiedId.
-TemplateIdAnnotation *TemplateId
- = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
// FIXME: Store name for literal operator too.
-if (Id.getKind() == UnqualifiedId::IK_Identifier) {
- TemplateId->Name = Id.Identifier;
- TemplateId->Operator = OO_None;
- TemplateId->TemplateNameLoc = Id.StartLocation;
-} else {
- TemplateId->Name = nullptr;
- TemplateId->Operator = Id.OperatorFunctionId.Operator;
- TemplateId->TemplateNameLoc = Id.StartLocation;
-}
+IdentifierInfo *TemplateII =
+Id.getKind() == UnqualifiedId::IK_Identifier ? Id.Identifier : nullptr;
+OverloadedOperatorKind OpKind = Id.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: Id.OperatorFunctionId.Operator;
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size();
- Arg != ArgEnd; ++Arg)
- Args[Arg] = TemplateArgs[Arg];
-
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+SS, TemplateKWLoc, Id.StartLocation, TemplateII, OpKind, Template, TNK,
+LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
Id.setTemplateId(TemplateId);
return false;
}
Index: include/clang/Sema/ParsedTemplate.h
===
--- include/clang/Sema/ParsedTemplate.h
+++ include/clang/Sema/ParsedTemplate.h
@@ -145,12 +145,15 @@
/// expressions, or template names, and the source locations for important
/// tokens. All of the information about template arguments is allocated
/// directly after this structure.
- struct TemplateIdAnnotation {
+ struct TemplateIdAnnotation final
+ : private llvm::TrailingObjects {
+friend TrailingObjects;
/// \brief The nested-name-specifier that precedes the template name.
CXXScopeSpec SS;
-/// TemplateKWLoc - The location of the template keyword within the
-/// source.
+/// TemplateKWLoc - The location of the template keyword.
+/// For e.g. typename T::template Y
SourceLocation TemplateKWLoc;
/// TemplateNameLoc - The location of the template
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
rnk added a comment. Re: clang-tidy, I would rather implement this as a traditional compiler warning. In https://reviews.llvm.org/D3#761208, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#761126, @jyu2 wrote: > > > As I said, I don't think checking throw type matching catch handler type is > > compiler's job. I'd like either silence all warning for that. What do you > > think? > > > Consider the following cases: > ... > I think the expected behavior in these cases is reasonable (and can be done > with a CFG) rather than manually looking at the scope stack like you're doing. I agree with @jyu2, we should do something simple. I think it would be simple to handle all cases except for `h`, which requires semantic analysis to figure out if the thrown object would be caught by the catch parameter. Implementing that is more likely to introduce bugs in the compiler than it is to find bugs in user code. I doubt we need the CFG for any of this. The later examples are all throwing exceptions from catch blocks, which is the same as not having an open try scope. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
aaron.ballman added a comment.
In https://reviews.llvm.org/D3#761126, @jyu2 wrote:
> In https://reviews.llvm.org/D3#760431, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D3#760419, @jyu2 wrote:
> >
> > > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote:
> > >
> > > > As an FYI, there is a related check currently under development in
> > > > clang-tidy; we probably should not duplicate this functionality in both
> > > > places. See https://reviews.llvm.org/D19201 for the other review.
> > >
> > >
> > > To my understanding, clang-tidy is kind of source check tool. It does
> > > more intensive checking than compiler.
> > > My purpose here is to emit minimum warning form compiler, in case,
> > > clang-tidy is not used.
> >
> >
> > Sort of correct. clang-tidy doesn't necessarily do more intensive checking
> > than the compiler. It's more an AST matching source checking tool for
> > checks that are either expensive or have a higher false-positive rate than
> > we'd want to see in the frontend.
> >
> > I think that this particular check should probably be in the frontend, like
> > you're doing. My biggest concern is that we do not duplicate functionality
> > between the two tools. https://reviews.llvm.org/D19201 has has a
> > considerable amount of review, so you should look at the test cases there
> > and ensure you are handling them (including the fixits). Hopefully your
> > patch ends up covering all of the functionality in the clang-tidy patch.
> >
> > > In https://reviews.llvm.org/D3#760353, @Prazek wrote:
> > >
> > >> Could you add similar tests as the ones that Stanislaw provied in his
> > >> patch?
> > >> Like the one with checking if throw is catched, or the conditional
> > >> noexcept (by a macro, etc)
> > >
> > >
> > > Good idea! Could add “marco” test for this. But I am not sure compiler
> > > want to add check for throw caught by different catch handler. Because
> > > at compile time, compiler can not statically determine which catch
> > > handler will be used to caught the exception on all time. I would think
> > > that is pragma's responsibility.
> > >
> > > For example:
> > >
> > > If (a) throw A else throw B;
> > >
> > >
> > >
> > > My main concern there is implicit noexcept gets set by compiler, which
> > > cause runtime to termination.
>
>
> As I said, I don't think checking throw type matching catch handler type is
> compiler's job. I'd like either silence all warning for that. What do you
> think?
Consider the following cases:
void f() noexcept { // Should not diagnose
try {
throw 12;
} catch (int) {
}
}
void g() noexcept { // Should not diagnose
try {
throw 12;
} catch (...) {
}
}
void h() noexcept { // Should diagnose
try {
throw 12;
} catch (const char *) {
}
}
void i() noexcept { // Should diagnose
try {
throw 12;
} catch (int) {
throw;
}
}
void j() noexcept { // Should diagnose
try {
throw 12;
} catch (int) {
throw "haha";
}
}
void k() noexcept { // Should diagnose
try {
throw 12;
} catch (...) {
throw;
}
}
I think the expected behavior in these cases is reasonable (and can be done
with a CFG) rather than manually looking at the scope stack like you're doing.
Comment at: include/clang/Basic/DiagnosticGroups.td:140
def Exceptions : DiagGroup<"exceptions">;
+def ThrowInNoexceptFunc : DiagGroup<"throw-in-noexcept-function">;
No need to add a group for this; you can define one inline with the warning,
but perhaps the exceptions group already covers it.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6335
+def warn_throw_in_noexcept_func
+: Warning<"'%0' function assumed not to throw an exception but does. "
+ "Throwing exception may cause termination.">,
Do not quote %0. Also, remove the full-stop and capitalized "Throwing".
Diagnostics are not complete sentences. I think it should probably read `"%0
has a non-throwing exception specification but can still throw, resulting in
unexpected program termination"` (or something along those lines).
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6342
+def note_throw_in_function
+: Note<"__declspec(nothrow), throw(), noexcept(true), or noexcept was "
+ "specified on the function">;
Rather than make the note spell out all the various ways a function can be
nothrow, we should either say "nonthrowing function declare here" or specify
which reason the function is not throwing using %select.
Comment at: include/clang/Sema/Sema.h:4970
bool CheckCXXThrowOperand(SourceLocation ThrowLoc, QualType ThrowTy, Expr
*E);
+ /// Check if throw is used in function with non-throwing noexcept
+ /// specifier.
with non
Re: r303546 - [mips] Support `micromips` attribute
On Mon, May 22, 2017 at 12:00 PM, Simon Atanasyan wrote:
> On Mon, May 22, 2017 at 3:57 PM, Aaron Ballman wrote:
>> On Mon, May 22, 2017 at 8:47 AM, Simon Atanasyan via cfe-commits
>> wrote:
>>> Author: atanasyan
>>> Date: Mon May 22 07:47:43 2017
>>> New Revision: 303546
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
>>> Log:
>>> [mips] Support `micromips` attribute
>>>
>>> This patch adds support for the `micromips` and `nomicromips` attributes
>>> for MIPS targets.
>>>
>>> Differential revision: https://reviews.llvm.org/D33363
>
> [...]
>
>>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
>>> ==
>>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>>> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
>>> @@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
>>>let Documentation = [MipsInterruptDocs];
>>> }
>>>
>>> +def MicroMips : InheritableAttr, TargetSpecificAttr {
>>> + let Spellings = [GCC<"micromips">];
>>> + let Subjects = SubjectList<[Function], ErrorDiag>;
>>
>> Why is this an error rather than a warning? Same question below.
>
> Because GCC shows the error in this case. And I cannot imagine any
> reason why micromips attribute added to non-function declaration.
Misapplication of attributes generally means the attribute is ignored;
assuming the program can continue to execute reasonably with the
attribute ignored, we usually don't diagnose as an error. I can't
think of a reason why ignoring this attribute would require us to fail
to translate the program. (This is why the default behavior of the
SubjectList is to warn rather than err, btw.)
Thank you for the other fixes!
~Aaron
>
> % mips-mti-linux-gnu-gcc --version
> mips-mti-linux-gnu-gcc (Codescape GNU Tools 2016.05-01 for MIPS MTI Linux)
> 4.9.2
>
> % mips-mti-linux-gnu-gcc -c attr-micromips.c
> tools/clang/test/Sema/attr-micromips.c:6:1: error: ‘nomicromips’
> attribute only applies to functions
> __attribute((nomicromips)) int a; // expected-error {{attribute only
> applies to functions}}
> ^
> tools/clang/test/Sema/attr-micromips.c:7:1: error: ‘micromips’
> attribute only applies to functions
> __attribute((micromips)) int b; // expected-error {{attribute only
> applies to functions}}
>
>>> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
>>> ==
>>> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
>>> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
>>> @@ -1269,6 +1269,19 @@ The semantics are as follows:
>>>}];
>>> }
>>>
>>> +def MicroMipsDocs : Documentation {
>>> + let Category = DocCatFunction;
>>> + let Content = [{
>>> +Clang supports the GNU style ``__attribute__((micromips))`` and
>>> +``__attribute__((nomicromips))`` attributes on MIPS targets. These
>>> attributes
>>> +may be attached to a function definition and instructs the backend to
>>> generate
>>> +or not to generate microMIPS code for that function.
>>> +
>>> +These attributes override the -mmicromips and -mno-micromips options
>>
>> Please quote the command line options with ``.
>
> Fixed at r303564.
>
>>> Added: cfe/trunk/test/Sema/attr-micromips.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303546&view=auto
>>> ==
>>> --- cfe/trunk/test/Sema/attr-micromips.c (added)
>>> +++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 07:47:43 2017
>>> @@ -0,0 +1,15 @@
>>> +// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
>>> +
>>> +__attribute__((nomicromips(0))) void foo1(); // expected-error
>>> {{'nomicromips' attribute takes no arguments}}
>>> +__attribute__((micromips(1))) void foo2();// expected-error
>>> {{'micromips' attribute takes no arguments}}
>>> +
>>> +__attribute((nomicromips)) int a; // expected-error {{attribute only
>>> applies to functions}}
>>> +__attribute((micromips)) int b; // expected-error {{attribute only
>>> applies to functions}}
>>> +
>>> +__attribute__((micromips,mips16)) void foo5(); // expected-error
>>> {{'micromips' and 'mips16' attributes are not compatible}} \
>>> +// expected-note
>>> {{conflicting attribute is here}}
>>> +__attribute__((mips16,micromips)) void foo6(); // expected-error
>>> {{'mips16' and 'micromips' attributes are not compatible}} \
>>> +// expected-note
>>> {{conflicting attribute is here}}
>>
>> Can you also add a test like:
>>
>> __attribute__((mips16))
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
sepavloff added a comment.
Thank you!
I put updated fix here. If it is OK, I'll commit it tomorrow.
Comment at: lib/Tooling/CompilationDatabase.cpp:292
+ if (Argc == 0) {
+ErrorMsg = "error: no arguments specified\n";
+return nullptr;
alexfh wrote:
> The lack of the arguments (or the `--` below) shouldn't be treated as an
> error (at least an error that is worth showing to the user). The caller will
> just move on to trying the next kind of a compilation database. The only
> actual errors we can get here may be coming from `stripPositionalArgs`.
>
> The caller should be modified accordingly (i.e. not output anything when both
> the return value is `nullptr` and `ErrorMsg` is empty).
What if error message contains a prefix that indicates if this is error or
warning? Errors could be printed and warnings ignored in
`CommonOptionsParser.cpp:122`. Such solution would allow clients to investigate
failures in more details.
https://reviews.llvm.org/D33272
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
sepavloff updated this revision to Diff 99786.
sepavloff marked an inline comment as done.
sepavloff added a comment.
Addressed reviewer's notes.
https://reviews.llvm.org/D33272
Files:
include/clang/Tooling/CompilationDatabase.h
lib/Frontend/CreateInvocationFromCommandLine.cpp
lib/Tooling/CommonOptionsParser.cpp
lib/Tooling/CompilationDatabase.cpp
lib/Tooling/Tooling.cpp
unittests/Tooling/CompilationDatabaseTest.cpp
Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -504,29 +504,35 @@
TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
int Argc = 0;
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr, ErrorMsg);
EXPECT_FALSE(Database);
+ EXPECT_EQ(ErrorMsg, "warning: no arguments specified\n");
EXPECT_EQ(0, Argc);
}
TEST(ParseFixedCompilationDatabase, ReturnsNullWithoutDoubleDash) {
int Argc = 2;
const char *Argv[] = { "1", "2" };
+ std::string ErrorMsg;
std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
EXPECT_FALSE(Database);
+ EXPECT_EQ(ErrorMsg, "warning: double dash not found\n");
EXPECT_EQ(2, Argc);
}
TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) {
int Argc = 5;
const char *Argv[] = {
"1", "2", "--\0no-constant-folding", "-DDEF3", "-DDEF4"
};
+ std::string ErrorMsg;
std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -543,9 +549,11 @@
TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) {
int Argc = 3;
const char *Argv[] = { "1", "2", "--\0no-constant-folding" };
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -560,9 +568,11 @@
TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"};
int Argc = sizeof(Argv) / sizeof(char*);
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -579,9 +589,11 @@
TEST(ParseFixedCompilationDatabase, HandlesArgv0) {
const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"};
int Argc = sizeof(Argv) / sizeof(char*);
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -260,6 +260,8 @@
Driver->setCheckInputsExist(false);
const std::unique_ptr Compilation(
Driver->BuildCompilation(llvm::makeArrayRef(Argv)));
+ if (!Compilation)
+return false;
const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments(
&Diagnostics, Compilation.get());
if (!CC1Args) {
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -27,6 +27,7 @@
#include "llvm/Option/Arg.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
#include
#include
using namespace clang;
@@ -150,23 +151,21 @@
// options.
class UnusedInputDiagConsumer : public DiagnosticConsumer {
public:
- UnusedInputDiagConsumer() : Other(nullptr) {}
-
- // Useful for debugging, chain diagnostics to another consumer after
- // recording for our own purposes.
- UnusedInputDiag
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/CodeGen/CGBuiltin.cpp:7526 +llvm::Type *ResultType = ConvertType(E->getType()); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, Ops); oren_ben_simhon wrote: > oren_ben_simhon wrote: > > craig.topper wrote: > > > Why did the call to ConvertType come back? This code can just be > > > > > > > > > ``` > > > llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, Ops[0]->getType()); > > > return Builder.CreateCall(F, Ops); > > > ``` > > After checking it again, I reverted that change because AFAIK Ops[0] is the > > first argument and not the return type of the statement. > After checking it again, I reverted that change because AFAIK Ops[0] is the > first argument and not the return type of the statement. That's true, but for this intrinsic they are required to be the same type. But its not a big deal. Repository: rL LLVM https://reviews.llvm.org/D33170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
rnk added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:674 + Expr::EvalResult EvlResult; + // Try to evaluate the identifier as enum constant + if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult, Please add a comment explaining that non-enum integer constants should not be folded. I expected that MSVC would fold constexpr integers here, but it does not, so that's worth noting. Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12 + const int a = 0; + // CHECK-NOT: mov eax, [$$0] + __asm mov eax, [a] mharoush wrote: > rnk wrote: > > Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing > > ms-inline-asm.c tests do so that this test is easier to debug when it fails. > This test case was just meant verify that other Integer constants are not > folded since we get a different behavior for statements such as mov eax, [a]. > #--- > In this example X86AsmParser regards the address of the variable 'a' and not > its value i.e. we end up with the value of 'a' in eax (loaded from the stack) > and not with the value pointed by the const int value of 'a' as its address. > ---# > > I can clarify the intention in a comment or completely remove the test case > since this isn't really required here. The test case is fine and the intention is clear, I am just suggesting that you use more FileCheck features (CHECK-LABEL and CHECK-SAME) to make the test easier to debug when it fails in the future. Repository: rL LLVM https://reviews.llvm.org/D33277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
rnk added inline comments.
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:64
unsigned &Offset) = 0;
+ virtual bool EvaluateLookupAsEnum(void *LookupResult,int64_t &Result) = 0;
};
mharoush wrote:
> rnk wrote:
> > It would be cleaner if InlineAsmIdentifierInfo simply contained an APInt
> > indicating that the identifier in question was an enum, or other integral
> > constant expression. Less callbacks to implement.
> Done
Thanks! This is better.
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:40
void *OpDecl;
bool IsVarDecl;
unsigned Length, Size, Type;
Please group the booleans to reduce padding. Ideally, make this an enum
something like:
```
enum IdKind : uint8_t {
Variable,
Function,
ConstInt,
};
IdKind Kind;
```
This is smaller and prevents nonsense states.
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+ StringRef ErrMsg;
Please use clang-format here and elsewhere
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722
+ bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier);
std::unique_ptr
mharoush wrote:
> rnk wrote:
> > Please try to eliminate the need for this extra boolean out parameter.
> This flag is used in case we try to compile something like mov edx, A+6 (
> when A is defined as ConstantEnum ) the original condition (Line:1905) will
> skip the rewrite since we have an identifier as the first Token, however if
> we try to compile 6+A it will be rewritten as expected.
>
> I tried to solve this in different ways, I got either the exact opposite (A+6
> was replaced and 6+A wasn't) or a collision of rewrites when trying to
> preform other forms of replacements and leaving this section intact.
>
> I can perhaps change the way we handle general expressions to the same way we
> handle them under parseIntelBracExpression, meaning use the first iteration
> of X86AsmParser to reformat the string (write imm prefixes and replace
> identifiers when needed) then refactor the string to its canonical form on
> the second pass.
>
> In short this was the simplest solution that worked without regression, maybe
> you have a better idea?
>
> If the issue is the method signature pollution I can move this flag into the
> SM class as a member to indicate a rewrite is needed, however since this flag
> and method are limited to this context alone, I am not sure if the overhead
> is wanted in such case .
I suspect there is a way to simplify the code so that this corner case no
longer exists. I don't really have time to dig through the test cases to come
up with it, but please make an effort.
Repository:
rL LLVM
https://reviews.llvm.org/D33278
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303568 - [index] Index the default template parameter values
Author: arphaman
Date: Mon May 22 11:50:54 2017
New Revision: 303568
URL: http://llvm.org/viewvc/llvm-project?rev=303568&view=rev
Log:
[index] Index the default template parameter values
rdar://32323724
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303568&r1=303567&r2=303568&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 11:50:54 2017
@@ -63,6 +63,17 @@ public:
case TemplateArgument::Type:
IndexCtx.indexTypeSourceInfo(LocInfo.getAsTypeSourceInfo(), Parent, DC);
break;
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+ IndexCtx.indexNestedNameSpecifierLoc(TALoc.getTemplateQualifierLoc(),
+ Parent, DC);
+ if (const TemplateDecl *TD = TALoc.getArgument()
+ .getAsTemplateOrTemplatePattern()
+ .getAsTemplateDecl()) {
+if (const NamedDecl *TTD = TD->getTemplatedDecl())
+ IndexCtx.handleReference(TTD, TALoc.getTemplateNameLoc(), Parent,
DC);
+ }
+ break;
default:
break;
}
@@ -610,8 +621,43 @@ public:
return true;
}
+ static bool shouldIndexTemplateParameterDefaultValue(const NamedDecl *D) {
+if (!D)
+ return false;
+// We want to index the template parameters only once when indexing the
+// canonical declaration.
+if (const auto *FD = dyn_cast(D))
+ return FD->getCanonicalDecl() == FD;
+else if (const auto *TD = dyn_cast(D))
+ return TD->getCanonicalDecl() == TD;
+else if (const auto *VD = dyn_cast(D))
+ return VD->getCanonicalDecl() == VD;
+return true;
+ }
+
bool VisitTemplateDecl(const TemplateDecl *D) {
// FIXME: Template parameters.
+
+// Index the default values for the template parameters.
+const NamedDecl *Parent = D->getTemplatedDecl();
+if (D->getTemplateParameters() &&
+shouldIndexTemplateParameterDefaultValue(Parent)) {
+ const TemplateParameterList *Params = D->getTemplateParameters();
+ for (const NamedDecl *TP : *Params) {
+if (const auto *TTP = dyn_cast(TP)) {
+ if (TTP->hasDefaultArgument())
+IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(),
Parent);
+} else if (const auto *NTTP = dyn_cast(TP)) {
+ if (NTTP->hasDefaultArgument())
+IndexCtx.indexBody(NTTP->getDefaultArgument(), Parent);
+} else if (const auto *TTPD = dyn_cast(TP)) {
+ if (TTPD->hasDefaultArgument())
+handleTemplateArgumentLoc(TTPD->getDefaultArgument(), Parent,
+ /*DC=*/nullptr);
+}
+ }
+}
+
return Visit(D->getTemplatedDecl());
}
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303568&r1=303567&r2=303568&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 11:50:54 2017
@@ -374,3 +374,62 @@ struct DeletedMethods {
// CHECK: [[@LINE-3]]:24 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
// CHECK: [[@LINE-4]]:3 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
};
+
+namespace ns2 {
+template struct ACollectionDecl { };
+}
+
+template |
Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+ int x = Record::C,
+// CHECK: [[@LINE-1]]:26 | static-property/C++ | C | c:@S@Record@C |
__ZN6Record1CE | Ref,Read,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+// CHECK: [[@LINE-3]]:18 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+ template class Collection = ns2::ACollectionDecl>
+// CHECK: [[@LINE-1]]:49 | namespace/C++ | ns2 | c:@N@ns2 | |
Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+// CHECK: [[@LINE-3]]:54 | struct(Gen)/C++ | ACollectionDecl |
c:@N@ns2@ST>1#T@ACollectionDecl | | Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+struct TemplateDefaultValues { };
+
+template |
Ref,RelCont | rel: 1
+ int x = sizeof(Cls)>
+// CHECK: [[@LINE-1]]:25 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
+void functionTemplateDefaultValues() { }
+
+namespace ensureDefaultTemplateParamsAreRecordedOnce {
+
+template
+// CHECK: [[@LINE-1]]:23 | class/C++ |
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +if (k1 <= k0) return first; compnerd wrote: > I'm not sure how `k1` can be `<` than `k0`. Isn't this effectively always > going down the `==` path? If I'm just mistaken, then I think that this needs > a comment. I agree that it should be impossible, but the previous version handled this case with the `if (db.names.size() < 2)` check, when really `db.names.size()` can only be 1 or greater. I think it makes more sense to be paranoid here, I trust this file about as far as I can throw it (And I can't throw it very far, its not a tangible object). https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
mharoush marked an inline comment as done.
mharoush added inline comments.
Comment at: lib/Parse/ParseStmtAsm.cpp:100
+ // result of a call to LookupInlineAsmIdentifier.
+ bool EvaluateLookupAsEnum(void *LookupResult, int64_t &Result) {
+if (!LookupResult) return false;
rnk wrote:
> Please format this, clang-format will do the job
Revised
Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12
+ const int a = 0;
+ // CHECK-NOT: mov eax, [$$0]
+ __asm mov eax, [a]
rnk wrote:
> Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing
> ms-inline-asm.c tests do so that this test is easier to debug when it fails.
This test case was just meant verify that other Integer constants are not
folded since we get a different behavior for statements such as mov eax, [a].
#---
In this example X86AsmParser regards the address of the variable 'a' and not
its value i.e. we end up with the value of 'a' in eax (loaded from the stack)
and not with the value pointed by the const int value of 'a' as its address.
---#
I can clarify the intention in a comment or completely remove the test case
since this isn't really required here.
Repository:
rL LLVM
https://reviews.llvm.org/D33277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. looks good to me https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
mharoush updated this revision to Diff 99780.
mharoush added a comment.
removed functions and dropped an irrelevant test case.
Repository:
rL LLVM
https://reviews.llvm.org/D33277
Files:
lib/Sema/SemaStmtAsm.cpp
test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
}
fillInlineAsmTypeInfo(Context, T, Info);
-
+
+ Expr *Res = Result.get();
// We can work with the expression as long as it's not an r-value.
- if (!Result.get()->isRValue())
+ if (!Res->isRValue()) {
Info.IsVarDecl = true;
+return Result;
+ }
+ Expr::EvalResult EvlResult;
+ // Try to evaluate the identifier as enum constant
+ if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult,
+ getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+ }
return Result;
}
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+ enum { A = 1 };
+ // CHECK: mov eax, $$12
+ __asm mov eax, x::A
+ // CHECK: mov eax, $$17
+ __asm mov eax, x::y::A
+ // CHECK: mov eax, $$1
+ __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+ enum { A = 1, B };
+ // CHECK: mov eax, $$21
+ __asm mov eax, (A + 9) * 2 + A
+ // CHECK: mov eax, $$4
+ __asm mov eax, A << 2
+ // CHECK: mov eax, $$2
+ __asm mov eax, B & 3
+ // CHECK: mov eax, $$5
+ __asm mov eax, 3 + (B & 3)
+ // CHECK: mov eax, $$8
+ __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+ int arr[4];
+ enum { A = 4, B };
+
+ // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+ __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+ // CHECK: mov eax, dword ptr $$4$0
+ __asm { mov eax, dword ptr [arr + A] }
+ // CHECK: mov eax, dword ptr $$8$0
+ __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
}
fillInlineAsmTypeInfo(Context, T, Info);
-
+
+ Expr *Res = Result.get();
// We can work with the expression as long as it's not an r-value.
- if (!Result.get()->isRValue())
+ if (!Res->isRValue()) {
Info.IsVarDecl = true;
+return Result;
+ }
+ Expr::EvalResult EvlResult;
+ // Try to evaluate the identifier as enum constant
+ if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult,
+ getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+ }
return Result;
}
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+ enum { A = 1 };
+ // CHECK: mov eax, $$12
+ __asm mov eax, x::A
+ // CHECK: mov eax, $$17
+ __asm mov eax, x::y::A
+ // CHECK: mov eax, $$1
+ __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+ enum { A = 1, B };
+ // CHECK: mov eax, $$21
+ __asm mov eax, (A + 9) * 2 + A
+ // CHECK: mov eax, $$4
+ __asm mov eax, A << 2
+ // CHECK: mov eax, $$2
+ __asm mov eax, B & 3
+ // CHECK: mov eax, $$5
+ __asm mov eax, 3 + (B & 3)
+ // CHECK: mov eax, $$8
+ __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+ int arr[4];
+ enum { A = 4, B };
+
+ // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+ __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+ // CHECK: mov eax, dword ptr $$4$0
+ __asm { mov eax, dword ptr [arr + A] }
+ // CHECK: mov eax, dword ptr $$8$0
+ __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
yaxunl added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); bader wrote: > yaxunl wrote: > > bader wrote: > > > yaxunl wrote: > > > > yaxunl wrote: > > > > > bader wrote: > > > > > > yaxunl wrote: > > > > > > > Anastasia wrote: > > > > > > > > echuraev wrote: > > > > > > > > > yaxunl wrote: > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could > > > > > > > > > > > > > you please verify that changing from `SIZE_MAX` to > > > > > > > > > > > > > `0` would be fine. > > > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for > > > > > > > > > > > > > `CLK_NULL_EVENT`. > > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail > > > > > > > > > > > > and not part of the spec. I would suggest to remove it > > > > > > > > > > > > from this header file. > > > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be > > > > > > > > > > > > defined but does not define its value. Naturally a > > > > > > > > > > > > valid id starts from 0 and increases. I don't see > > > > > > > > > > > > significant advantage to change CLK_NULL_RESERVE_ID > > > > > > > > > > > > from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > > I don't see issues to commit things outside of spec as > > > > > > > > > > > soon as they prefixed properly with "__". But I agree it > > > > > > > > > > > would be nice to see if it's any useful and what the > > > > > > > > > > > motivation is for having different implementation. > > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > > implementations assume that. Actually I am curious why that > > > > > > > > > > is needed too. > > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is > > > > > > > > > valid if significant bit equal to one. `CLK_NULL_RESERVE_ID > > > > > > > > > refers to an invalid reservation, so if `CLK_NULL_RESERVE_ID > > > > > > > > > equal to 0, we can be sure that significant bit doesn't equal > > > > > > > > > to 1 and it is invalid reserve id. Also it is more obviously > > > > > > > > > if CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > > previous implementation also assumes that one specific bit > > > > > > > > > was of a reverse id was used to indicate that the reserve id > > > > > > > > > is valid. So, we just increased reserve id size by one bit on > > > > > > > > > 32-bit platforms and by 33 bits on 64-bit platforms. > > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but > > > > > > > > spec doesn't define it of course. > > > > > > > In our implementation, valid reserve id starts at 0 and > > > > > > > increasing linearly until `__SIZE_MAX-1`. This change will break > > > > > > > our implementation. > > > > > > > > > > > > > > However, we can modify our implementation to adopt this change > > > > > > > since it brings about benefits overall. > > > > > > Ideally it would be great to have unified implementation, but we > > > > > > can define device specific value for CLK_NULL_RESERVE_ID by using > > > > > > ifdef directive. > > > > > How about > > > > > > > > > > ``` > > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > > > ``` > > > > > I think the spec does not require it to be compile time constant. > > > > > Then each library can implement its own __clk_null_reserve_id() > > > > > whereas the IR is target independent. > > > > Or we only do this for SPIR and define it as target specific value for > > > > other targets. > > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but > > > IMHO compile time constant is preferable option. > > > I don't think making it compile time constant for SPIR only makes sense > > > to me - in this case we can use constant for all targets. > > > > > > How about following approach: use 0 by default and allow other targets > > > re-define CLK_NULL_RESERVE_ID value. > > > > > > ``` > > > #ifndef CLK_NULL_RESERVE_ID > > > #define CLK_NULL_RESERVE_ID 0 > > > #endif // CLK_NULL_RESERVE_ID > > > ``` > > > > > > If CLK_NULL_RESERVE_ID defined via -D comm
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
mharoush updated this revision to Diff 99778.
mharoush added a comment.
Using identifier info to pass enum information.
Repository:
rL LLVM
https://reviews.llvm.org/D33278
Files:
include/llvm/MC/MCParser/MCAsmParser.h
lib/Target/X86/AsmParser/X86AsmParser.cpp
Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -718,7 +718,8 @@
ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
- bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
+ bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier);
std::unique_ptr
ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
bool isSymbol, unsigned Size);
@@ -1306,8 +1307,9 @@
return false;
return true;
}
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier) {
+ ReplaceEnumIdentifier = false;
MCAsmParser &Parser = getParser();
const AsmToken &Tok = Parser.getTok();
@@ -1368,11 +1370,40 @@
PrevTK == AsmToken::RBrac) {
return false;
} else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
+Info.clear();
if (ParseIntelIdentifier(Val, Identifier, Info,
/*Unevaluated=*/false, End))
return true;
-SM.onIdentifierExpr(Val, Identifier);
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+ StringRef ErrMsg;
+ // SM should treat the value as it would an explicit integer in the
+ // expression.
+ if(SM.onInteger(CE->getValue(), ErrMsg))
+return Error(IdentLoc, ErrMsg);
+ // In case we are called on a bracketed expression,
+ if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed
+// expression in order to match the behavior for the equivalent
+// integer tokens.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc,Len,
+CE->getValue());
+break;
+ }
+ // Set force rewrite flag only when not bracketed expression.
+ ReplaceEnumIdentifier = true;
+} else {
+ // Notify the SM a variable identifier was found.
+ InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+ SMInfo = Info;
+ SM.onIdentifierExpr(Val, Identifier);
+}
}
break;
}
@@ -1452,7 +1483,8 @@
// may have already parsed an immediate displacement before the bracketed
// expression.
IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
- if (ParseIntelExpression(SM, End))
+ bool ReplaceEnumIdentifier;
+ if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
return nullptr;
const MCExpr *Disp = nullptr;
@@ -1559,7 +1591,15 @@
// failed parsing.
assert((End.getPointer() == EndPtr || !Result) &&
"frontend claimed part of a token?");
-
+
+ // Check if the search yielded a constant integer (enum identifier).
+ if (Result && Info.IsConstEnum) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(Info.ConstIntValue.getSExtValue(),
+ getParser().getContext());
+return false;
+ }
// If the identifier lookup was unsuccessful, assume that we are dealing with
// a label.
if (!Result) {
@@ -1852,18 +1892,21 @@
AsmToken StartTok = Tok;
IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
/*AddImmPrefix=*/false);
- if (ParseIntelExpression(SM, End))
+ // The parsed expression may contain enum identifier tokens which we must
+ // replace, ReplaceEnumIdentifier flag lets us know when to force rewrite.
+ bool ReplaceEnumIdentifier;
+ if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
return nullptr;
-
bool isSymbol = SM.getSym() &&
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
mharoush marked an inline comment as done. mharoush added inline comments. Comment at: include/llvm/MC/MCParser/MCAsmParser.h:64 unsigned &Offset) = 0; + virtual bool EvaluateLookupAsEnum(void *LookupResult,int64_t &Result) = 0; }; rnk wrote: > It would be cleaner if InlineAsmIdentifierInfo simply contained an APInt > indicating that the identifier in question was an enum, or other integral > constant expression. Less callbacks to implement. Done Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722 + bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, +bool &ReplaceEnumIdentifier); std::unique_ptr rnk wrote: > Please try to eliminate the need for this extra boolean out parameter. This flag is used in case we try to compile something like mov edx, A+6 ( when A is defined as ConstantEnum ) the original condition (Line:1905) will skip the rewrite since we have an identifier as the first Token, however if we try to compile 6+A it will be rewritten as expected. I tried to solve this in different ways, I got either the exact opposite (A+6 was replaced and 6+A wasn't) or a collision of rewrites when trying to preform other forms of replacements and leaving this section intact. I can perhaps change the way we handle general expressions to the same way we handle them under parseIntelBracExpression, meaning use the first iteration of X86AsmParser to reformat the string (write imm prefixes and replace identifiers when needed) then refactor the string to its canonical form on the second pass. In short this was the simplest solution that worked without regression, maybe you have a better idea? If the issue is the method signature pollution I can move this flag into the SM class as a member to indicate a rewrite is needed, however since this flag and method are limited to this context alone, I am not sure if the overhead is wanted in such case . Repository: rL LLVM https://reviews.llvm.org/D33278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 added a comment. In https://reviews.llvm.org/D3#760431, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#760419, @jyu2 wrote: > > > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > > > > > As an FYI, there is a related check currently under development in > > > clang-tidy; we probably should not duplicate this functionality in both > > > places. See https://reviews.llvm.org/D19201 for the other review. > > > > > > To my understanding, clang-tidy is kind of source check tool. It does more > > intensive checking than compiler. > > My purpose here is to emit minimum warning form compiler, in case, > > clang-tidy is not used. > > > Sort of correct. clang-tidy doesn't necessarily do more intensive checking > than the compiler. It's more an AST matching source checking tool for checks > that are either expensive or have a higher false-positive rate than we'd want > to see in the frontend. > > I think that this particular check should probably be in the frontend, like > you're doing. My biggest concern is that we do not duplicate functionality > between the two tools. https://reviews.llvm.org/D19201 has has a considerable > amount of review, so you should look at the test cases there and ensure you > are handling them (including the fixits). Hopefully your patch ends up > covering all of the functionality in the clang-tidy patch. > > > In https://reviews.llvm.org/D3#760353, @Prazek wrote: > > > >> Could you add similar tests as the ones that Stanislaw provied in his > >> patch? > >> Like the one with checking if throw is catched, or the conditional > >> noexcept (by a macro, etc) > > > > > > Good idea! Could add “marco” test for this. But I am not sure compiler > > want to add check for throw caught by different catch handler. Because at > > compile time, compiler can not statically determine which catch handler > > will be used to caught the exception on all time. I would think that is > > pragma's responsibility. > > > > For example: > > > > If (a) throw A else throw B; > > > > > > > > My main concern there is implicit noexcept gets set by compiler, which > > cause runtime to termination. As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think? https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:11 +#include "../ClangdLSPServer.h" +#include "../JSONRPCDispatcher.h" #include "llvm/Support/CommandLine.h" krasimir wrote: > I'd suggest to not have parent directory includes, but add them to the cmake > file as in other extra tools, like in: > https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-query/tool/CMakeLists.txt Done. I've actually followed a pattern of CMakeLists.txt from tool-extra, albeit a different one :-) https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/tool/ClangTidyMain.cpp https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement
GorNishanov added a comment. @rsmith barely audible ping https://reviews.llvm.org/D31646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.
ilya-biryukov created this revision.
Herald added a subscriber: mgorny.
Custom vfs::FileSystem is currently used for unit tests.
This revision depends on https://reviews.llvm.org/D33397.
https://reviews.llvm.org/D33416
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.h
unittests/CMakeLists.txt
unittests/clangd/CMakeLists.txt
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- /dev/null
+++ unittests/clangd/ClangdTests.cpp
@@ -0,0 +1,361 @@
+//===-- ClangdTes.cpp - Change namespace unit tests ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdServer.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include "gtest/gtest.h"
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace vfs {
+
+/// An implementation of vfs::FileSystem that only allows access to
+/// files and folders inside a set of whitelisted directories.
+///
+/// FIXME(ibiryukov): should it also emulate access to parents of whitelisted
+/// directories with only whitelisted contents?
+class FilteredFileSystem : public vfs::FileSystem {
+public:
+ /// The paths inside \p WhitelistedDirs should be absolute
+ FilteredFileSystem(std::vector WhitelistedDirs,
+ IntrusiveRefCntPtr InnerFS)
+ : WhitelistedDirs(std::move(WhitelistedDirs)), InnerFS(InnerFS) {
+assert(std::all_of(WhitelistedDirs.begin(), WhitelistedDirs.end(),
+ [](const std::string &Path) -> bool {
+ return llvm::sys::path::is_absolute(Path);
+ }) &&
+ "Not all WhitelistedDirs are absolute");
+ }
+
+ virtual llvm::ErrorOr status(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->status(Path);
+ }
+
+ virtual llvm::ErrorOr>
+ openFileForRead(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->openFileForRead(Path);
+ }
+
+ llvm::ErrorOr>
+ getBufferForFile(const Twine &Name, int64_t FileSize = -1,
+ bool RequiresNullTerminator = true,
+ bool IsVolatile = false) {
+if (!isInsideWhitelistedDir(Name))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->getBufferForFile(Name, FileSize, RequiresNullTerminator,
+ IsVolatile);
+ }
+
+ virtual directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) {
+if (!isInsideWhitelistedDir(Dir)) {
+ EC = llvm::errc::no_such_file_or_directory;
+ return directory_iterator();
+}
+return InnerFS->dir_begin(Dir, EC);
+ }
+
+ virtual std::error_code setCurrentWorkingDirectory(const Twine &Path) {
+return InnerFS->setCurrentWorkingDirectory(Path);
+ }
+
+ virtual llvm::ErrorOr getCurrentWorkingDirectory() const {
+return InnerFS->getCurrentWorkingDirectory();
+ }
+
+ bool exists(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return false;
+return InnerFS->exists(Path);
+ }
+
+ std::error_code makeAbsolute(SmallVectorImpl &Path) const {
+return InnerFS->makeAbsolute(Path);
+ }
+
+private:
+ bool isInsideWhitelistedDir(const Twine &InputPath) const {
+SmallString<128> Path;
+InputPath.toVector(Path);
+
+if (makeAbsolute(Path))
+ return false;
+
+for (const auto &Dir : WhitelistedDirs) {
+ if (Path.startswith(Dir))
+return true;
+}
+return false;
+ }
+
+ std::vector WhitelistedDirs;
+ IntrusiveRefCntPtr InnerFS;
+};
+
+/// Create a vfs::FileSystem that has access only to temporary directories
+/// (obtained by calling system_temp_directory).
+IntrusiveRefCntPtr getTempOnlyFS() {
+ llvm::SmallString<128> TmpDir1;
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, TmpDir1);
+ llvm::SmallString<128> TmpDir2;
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2);
+
+ std::vector TmpDirs;
+ TmpDirs.push_back(TmpDir1.str());
+ if (TmpDir2 != TmpDir2)
+TmpDirs.push_back(TmpDir2.str());
+ return new vfs::FilteredFileSystem(std::move(TmpDirs),
+ vfs::getRealFileSystem());
+}
+} // namespace vfs
+
+namespace clangd {
+namespace {
+
+class ErrorCheckingDiagConsumer : public DiagnosticsConsumer {
+public:
+ void onDiagnosticsReady(PathRef File,
+
Re: r303546 - [mips] Support `micromips` attribute
On Mon, May 22, 2017 at 3:57 PM, Aaron Ballman wrote:
> On Mon, May 22, 2017 at 8:47 AM, Simon Atanasyan via cfe-commits
> wrote:
>> Author: atanasyan
>> Date: Mon May 22 07:47:43 2017
>> New Revision: 303546
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
>> Log:
>> [mips] Support `micromips` attribute
>>
>> This patch adds support for the `micromips` and `nomicromips` attributes
>> for MIPS targets.
>>
>> Differential revision: https://reviews.llvm.org/D33363
[...]
>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
>> @@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
>>let Documentation = [MipsInterruptDocs];
>> }
>>
>> +def MicroMips : InheritableAttr, TargetSpecificAttr {
>> + let Spellings = [GCC<"micromips">];
>> + let Subjects = SubjectList<[Function], ErrorDiag>;
>
> Why is this an error rather than a warning? Same question below.
Because GCC shows the error in this case. And I cannot imagine any
reason why micromips attribute added to non-function declaration.
% mips-mti-linux-gnu-gcc --version
mips-mti-linux-gnu-gcc (Codescape GNU Tools 2016.05-01 for MIPS MTI Linux) 4.9.2
% mips-mti-linux-gnu-gcc -c attr-micromips.c
tools/clang/test/Sema/attr-micromips.c:6:1: error: ‘nomicromips’
attribute only applies to functions
__attribute((nomicromips)) int a; // expected-error {{attribute only
applies to functions}}
^
tools/clang/test/Sema/attr-micromips.c:7:1: error: ‘micromips’
attribute only applies to functions
__attribute((micromips)) int b; // expected-error {{attribute only
applies to functions}}
>> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
>> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
>> @@ -1269,6 +1269,19 @@ The semantics are as follows:
>>}];
>> }
>>
>> +def MicroMipsDocs : Documentation {
>> + let Category = DocCatFunction;
>> + let Content = [{
>> +Clang supports the GNU style ``__attribute__((micromips))`` and
>> +``__attribute__((nomicromips))`` attributes on MIPS targets. These
>> attributes
>> +may be attached to a function definition and instructs the backend to
>> generate
>> +or not to generate microMIPS code for that function.
>> +
>> +These attributes override the -mmicromips and -mno-micromips options
>
> Please quote the command line options with ``.
Fixed at r303564.
>> Added: cfe/trunk/test/Sema/attr-micromips.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303546&view=auto
>> ==
>> --- cfe/trunk/test/Sema/attr-micromips.c (added)
>> +++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 07:47:43 2017
>> @@ -0,0 +1,15 @@
>> +// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
>> +
>> +__attribute__((nomicromips(0))) void foo1(); // expected-error
>> {{'nomicromips' attribute takes no arguments}}
>> +__attribute__((micromips(1))) void foo2();// expected-error
>> {{'micromips' attribute takes no arguments}}
>> +
>> +__attribute((nomicromips)) int a; // expected-error {{attribute only
>> applies to functions}}
>> +__attribute((micromips)) int b; // expected-error {{attribute only
>> applies to functions}}
>> +
>> +__attribute__((micromips,mips16)) void foo5(); // expected-error
>> {{'micromips' and 'mips16' attributes are not compatible}} \
>> +// expected-note
>> {{conflicting attribute is here}}
>> +__attribute__((mips16,micromips)) void foo6(); // expected-error
>> {{'mips16' and 'micromips' attributes are not compatible}} \
>> +// expected-note
>> {{conflicting attribute is here}}
>
> Can you also add a test like:
>
> __attribute__((mips16)) void foo9(void) __attribute__((micromips));
Fixed at r303565.
--
Simon Atanasyan
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
ilya-biryukov updated this revision to Diff 99774.
ilya-biryukov added a comment.
Got rid of relative includes from parent dir in ClangdMain.cpp. (Addressed
krasimir's comments)
https://reviews.llvm.org/D33395
Files:
clangd/CMakeLists.txt
clangd/ClangdMain.cpp
clangd/tool/CMakeLists.txt
clangd/tool/ClangdMain.cpp
Index: clangd/ClangdMain.cpp
===
--- /dev/null
+++ clangd/ClangdMain.cpp
@@ -1,40 +0,0 @@
-//===--- ClangdMain.cpp - clangd server loop --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "ClangdLSPServer.h"
-#include "JSONRPCDispatcher.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Program.h"
-
-#include
-#include
-#include
-
-using namespace clang;
-using namespace clang::clangd;
-
-static llvm::cl::opt
-RunSynchronously("run-synchronously",
- llvm::cl::desc("parse on main thread"),
- llvm::cl::init(false), llvm::cl::Hidden);
-
-int main(int argc, char *argv[]) {
- llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
-
- llvm::raw_ostream &Outs = llvm::outs();
- llvm::raw_ostream &Logs = llvm::errs();
- JSONOutput Out(Outs, Logs);
-
- // Change stdin to binary to not lose \r\n on windows.
- llvm::sys::ChangeStdinToBinary();
-
- ClangdLSPServer LSPServer(Out, RunSynchronously);
- LSPServer.run(std::cin);
-}
Index: clangd/tool/CMakeLists.txt
===
--- clangd/tool/CMakeLists.txt
+++ clangd/tool/CMakeLists.txt
@@ -1,20 +1,14 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
add_clang_executable(clangd
- ClangdLSPServer.cpp
ClangdMain.cpp
- ClangdServer.cpp
- ClangdUnit.cpp
- ClangdUnitStore.cpp
- DraftStore.cpp
- GlobalCompilationDatabase.cpp
- JSONRPCDispatcher.cpp
- Protocol.cpp
- ProtocolHandlers.cpp
)
install(TARGETS clangd RUNTIME DESTINATION bin)
target_link_libraries(clangd
clangBasic
+ clangDaemon
clangFormat
clangFrontend
clangSema
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -1,6 +1,5 @@
-add_clang_executable(clangd
+add_clang_library(clangDaemon
ClangdLSPServer.cpp
- ClangdMain.cpp
ClangdServer.cpp
ClangdUnit.cpp
ClangdUnitStore.cpp
@@ -11,14 +10,14 @@
ProtocolHandlers.cpp
)
-install(TARGETS clangd RUNTIME DESTINATION bin)
-
-target_link_libraries(clangd
+target_link_libraries(clangDaemon
clangBasic
clangFormat
clangFrontend
clangSema
clangTooling
clangToolingCore
LLVMSupport
)
+
+add_subdirectory(tool)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303565 - [mips] Add one more check to the micromips attribute test case. NFC
Author: atanasyan
Date: Mon May 22 10:53:34 2017
New Revision: 303565
URL: http://llvm.org/viewvc/llvm-project?rev=303565&view=rev
Log:
[mips] Add one more check to the micromips attribute test case. NFC
Modified:
cfe/trunk/test/Sema/attr-micromips.c
Modified: cfe/trunk/test/Sema/attr-micromips.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303565&r1=303564&r2=303565&view=diff
==
--- cfe/trunk/test/Sema/attr-micromips.c (original)
+++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 10:53:34 2017
@@ -13,3 +13,5 @@ __attribute__((mips16,micromips)) void f
__attribute((micromips)) void foo7();
__attribute((nomicromips)) void foo8();
+__attribute__((mips16)) void foo9(void) __attribute__((micromips)); //
expected-error {{'micromips' and 'mips16' attributes are not compatible}} \
+//
expected-note {{conflicting attribute is here}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303564 - [mips] Quote command line options with `` in the micromips attribute description. NFC
Author: atanasyan Date: Mon May 22 10:53:31 2017 New Revision: 303564 URL: http://llvm.org/viewvc/llvm-project?rev=303564&view=rev Log: [mips] Quote command line options with `` in the micromips attribute description. NFC Modified: cfe/trunk/include/clang/Basic/AttrDocs.td Modified: cfe/trunk/include/clang/Basic/AttrDocs.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303564&r1=303563&r2=303564&view=diff == --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 10:53:31 2017 @@ -1277,7 +1277,7 @@ Clang supports the GNU style ``__attribu may be attached to a function definition and instructs the backend to generate or not to generate microMIPS code for that function. -These attributes override the -mmicromips and -mno-micromips options +These attributes override the `-mmicromips` and `-mno-micromips` options on the command line. }]; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: Replaced WorkerRequest with std::function...
ilya-biryukov created this revision.
And implemented a helper function to dump an AST of a file for
testing/debugging purposes.
https://reviews.llvm.org/D33415
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.h
Index: clangd/ClangdUnitStore.h
===
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -50,6 +50,18 @@
std::forward(Action));
}
+ /// Run the specified \p Action on the ClangdUnit for \p File.
+ /// Unit for \p File should exist in the store.
+ template
+ void runOnExistingUnit(PathRef File, Func Action) {
+std::lock_guard Lock(Mutex);
+
+auto It = OpenedFiles.find(File);
+assert(It != OpenedFiles.end() && "File is not in OpenedFiles");
+
+Action(It->second);
+ }
+
/// Remove ClangdUnit for \p File, if any
void removeUnitIfPresent(PathRef File);
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -16,6 +16,10 @@
#include "clang/Tooling/Core/Replacement.h"
#include
+namespace llvm {
+class raw_ostream;
+}
+
namespace clang {
class ASTUnit;
class PCHContainerOperations;
@@ -52,6 +56,10 @@
/// located in the current file.
std::vector getLocalDiagnostics() const;
+ /// For testing/debugging purposes. Note that this method deserializes all
+ /// unserialized Decls, so use with care.
+ void dumpAST(llvm::raw_ostream &OS) const;
+
private:
Path FileName;
std::unique_ptr Unit;
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -222,3 +222,7 @@
}
return Result;
}
+
+void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const {
+ Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true);
+}
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -24,6 +24,7 @@
#include "Protocol.h"
#include
+#include
#include
#include
#include
@@ -49,30 +50,21 @@
std::vector Diagnostics) = 0;
};
-enum class WorkerRequestKind { ParseAndPublishDiagnostics, RemoveDocData };
-
-/// A request to the worker thread
-class WorkerRequest {
-public:
- WorkerRequest() = default;
- WorkerRequest(WorkerRequestKind Kind, Path File, DocVersion Version);
-
- WorkerRequestKind Kind;
- Path File;
- DocVersion Version;
-};
-
class ClangdServer;
/// Handles running WorkerRequests of ClangdServer on a separate threads.
/// Currently runs only one worker thread.
class ClangdScheduler {
public:
- ClangdScheduler(ClangdServer &Server, bool RunSynchronously);
+ ClangdScheduler(bool RunSynchronously);
~ClangdScheduler();
- /// Enqueue WorkerRequest to be run on a worker thread
- void enqueue(ClangdServer &Server, WorkerRequest Request);
+ /// Add \p Request to the start of the queue. \p Request will be run on a
+ /// separate worker thread.
+ void addToFront(std::function Request);
+ /// Add \p Request to the end of the queue. \p Request will be run on a
+ /// separate worker thread.
+ void addToEnd(std::function Request);
private:
bool RunSynchronously;
@@ -83,11 +75,10 @@
std::thread Worker;
/// Setting Done to true will make the worker thread terminate.
bool Done = false;
- /// A LIFO queue of requests. Note that requests are discarded if the
- /// `version` field is not equal to the one stored inside DraftStore.
+ /// A queue of requests.
/// FIXME(krasimir): code completion should always have priority over parsing
/// for diagnostics.
- std::deque RequestQueue;
+ std::deque> RequestQueue;
/// Condition variable to wake up the worker thread.
std::condition_variable RequestCV;
};
@@ -126,12 +117,12 @@
/// conversions in outside code, maybe there's a way to get rid of it.
std::string getDocument(PathRef File);
-private:
- friend class ClangdScheduler;
-
- /// This function is called on a worker thread.
- void handleRequest(WorkerRequest Request);
+ /// Only for testing purposes.
+ /// Waits until all requests to worker thread are finished and dumps AST for
+ /// \p File. \p File must be in the list of added documents.
+ std::string dumpAST(PathRef File);
+private:
std::unique_ptr CDB;
std::unique_ptr DiagConsumer;
DraftStore DraftMgr;
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -15,6 +15,8 @@
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include
using namespace clang;
using namespace clang::clangd;
@@ -56,22 +58,18 @@
return {Lines, Cols};
}
-WorkerRequest:
r303563 - [index] Index the deleted functions
Author: arphaman
Date: Mon May 22 10:42:45 2017
New Revision: 303563
URL: http://llvm.org/viewvc/llvm-project?rev=303563&view=rev
Log:
[index] Index the deleted functions
rdar://32323386
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303563&r1=303562&r2=303563&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 10:42:45 2017
@@ -217,9 +217,6 @@ public:
}
bool VisitFunctionDecl(const FunctionDecl *D) {
-if (D->isDeleted())
- return true;
-
SymbolRoleSet Roles{};
SmallVector Relations;
if (auto *CXXMD = dyn_cast(D)) {
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303563&r1=303562&r2=303563&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 10:42:45 2017
@@ -366,3 +366,11 @@ struct IndexDefaultValue {
// CHECK: [[@LINE-2]]:30 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
}
};
+
+struct DeletedMethods {
+ DeletedMethods(const DeletedMethods &) = delete;
+// CHECK: [[@LINE-1]]:3 | constructor/cxx-copy-ctor/C++ | DeletedMethods |
c:@S@DeletedMethods@F@DeletedMethods#&1$@S@DeletedMethods# |
__ZN14DeletedMethodsC1ERKS_ | Def,RelChild | rel: 1
+// CHECK: RelChild | DeletedMethods | c:@S@DeletedMethods
+// CHECK: [[@LINE-3]]:24 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
+// CHECK: [[@LINE-4]]:3 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303562: [Sema][ObjC] Fix a bug where
-Wunguarded-availability was emitted at the wrong… (authored by epilk).
Changed prior to commit:
https://reviews.llvm.org/D33250?vs=99605&id=99772#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33250
Files:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaObjC/unguarded-availability.m
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(),
PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial
here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available
on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:'
is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; //
expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(), PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
r303562 - [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location
Author: epilk
Date: Mon May 22 10:41:12 2017
New Revision: 303562
URL: http://llvm.org/viewvc/llvm-project?rev=303562&view=rev
Log:
[Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong
location
Differential revision: https://reviews.llvm.org/D33250
Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaObjC/unguarded-availability.m
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303562&r1=303561&r2=303562&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon May 22 10:41:12 2017
@@ -7258,6 +7258,12 @@ public:
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(),
PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@ bool DiagnoseUnguardedAvailability::Visi
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Modified: cfe/trunk/test/SemaObjC/unguarded-availability.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unguarded-availability.m?rev=303562&r1=303561&r2=303562&view=diff
==
--- cfe/trunk/test/SemaObjC/unguarded-availability.m (original)
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m Mon May 22 10:41:12 2017
@@ -135,6 +135,26 @@ void (^topLevelBlockDecl)() = ^ {
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial
here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available
on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:'
is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; //
expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.
yawanng added a comment. I will make some major changes to this CL based on the current suggestions from reviewers and update it for further review later. Thank you for the valuable advice. Repository: rL LLVM https://reviews.llvm.org/D33304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33412: Add support for #pragma clang section
javed.absar created this revision.
Herald added a subscriber: aemerson.
This patch adds support for a '#pragma clang section' directive in clang.
An RFC was sent out earlier by my colleague James Molloy:
http://lists.llvm.org/pipermail/cfe-dev/2017-March/053100.html
Purpose:
The purpose of this is to provide to developers a means to specify
section-names
for global variables, functions and static variables, using #pragma
directives.
This will provide a migration path towards clang for developers in the
embedded
and automotive domain. For example, AUTOSAR, an automotive standard, mandates
the use of a #pragma in header files to determine in which sections
initialized
and uninitialized data get put into.
This feature is implemented in our legacy ARM Compiler 5 toolchain and GCC
forks
used across the automotive space that have this feature implemented compatible
with the ARM Compiler 5 implementation. The documentation is here:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472m/chr1359124985290.html
User-Visible Behavior:
The developer can specify section names as:
#pragma clang section bss="myBSS" data="myData" rodata="myRodata"
text="myText"
The developer can "unspecify" a section name with empty string e.g.
#pragma clang section bss="" data="" text="" rodata=""
1. The pragma applies to all global variable, statics and function
declarations
from the pragma to the end of the translation unit.
2. The pragma clang section is enabled automatically, without need of any
flags.
3. This feature is only defined to work sensibly for ELF targets.
4. If section name is specified through _attribute_((section("myname"))), then
the attribute name gains precedence over any applicable section name via
pragma directives.
5. Global variables, including - basic types, arrays, struct - that are
initialized to zero
e.g. int x = 0; will be placed in the named bss section, if one is present.
6. The section type using '#pragma clang section' approach does not does NOT
try to infer section-kind from the name.
For example, assigning a section ".bss.mysec" does NOT mean it will be placed
in BSS.
Design:
The decision about which section-kind applies to each global is not taken by
this
implementation but is purely inquired from the default back-end
implementation in LLVM.
Once the section-kind is known, appropriate section name as specified by the
user
using pragma directive, is applied to that global.
Note that this is more effective approach than choosing the section name in
the front-end
when optimisation passes have not been run and the final proper section is
not known.
There is a llvm corresponding patch with this patch.
https://reviews.llvm.org/D33412
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenModule.cpp
lib/Parse/ParsePragma.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaDecl.cpp
test/CodeGenCXX/clang-sections-tentative.c
test/CodeGenCXX/clang-sections.cpp
test/Sema/pragma-clang-section.c
Index: test/Sema/pragma-clang-section.c
===
--- /dev/null
+++ test/Sema/pragma-clang-section.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple arm-none-eabi
+#pragma clang section bss="mybss.1" data="mydata.1" rodata="myrodata.1" text="mytext.1"
+#pragma clang section bss="" data="" rodata="" text=""
+#pragma clang section
+
+#pragma clang section dss="mybss.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section deta="mydata.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section rodeta="rodata.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section taxt="text.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+
+#pragma clang section section bss="mybss.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+
+#pragma clang section bss "mybss.2" // expected-error {{expected '=' following '#pragma clang section bss'}}
+#pragma clang section data "mydata.2" // expected-error {{expected '=' following '#pragma clang section data'}}
+#pragma clang section rodata "myrodata.2" // expected-error {{expected '=' following '#pragma clang section rodata'}}
+#pragma clang section bss="" data="" rodata="" text="" more //expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+int a;
Index: test/CodeGenCXX/clang-sections.cpp
===
--- /dev/null
+++ test/C
r303559 - [index] Visit the default argument values in function definitions
Author: arphaman
Date: Mon May 22 10:17:44 2017
New Revision: 303559
URL: http://llvm.org/viewvc/llvm-project?rev=303559&view=rev
Log:
[index] Visit the default argument values in function definitions
rdar://32323315
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303559&r1=303558&r2=303559&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 10:17:44 2017
@@ -98,6 +98,17 @@ public:
}
}
}
+} else {
+ // Index the default parameter value for function definitions.
+ if (const auto *FD = dyn_cast(D)) {
+if (FD->isThisDeclarationADefinition()) {
+ for (const auto *PV : FD->parameters()) {
+if (PV->hasDefaultArg() && !PV->hasUninstantiatedDefaultArg() &&
+!PV->hasUnparsedDefaultArg())
+ IndexCtx.indexBody(PV->getDefaultArg(), D);
+ }
+}
+ }
}
}
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303559&r1=303558&r2=303559&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 10:17:44 2017
@@ -1,7 +1,7 @@
// RUN: c-index-test core -print-source-symbols -- %s -std=c++14 -target
x86_64-apple-macosx10.7 | FileCheck %s
// CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] | | Def
| rel: 0
-class Cls {
+class Cls { public:
// CHECK: [[@LINE+3]]:3 | constructor/C++ | Cls | c:@S@Cls@F@Cls#I# |
__ZN3ClsC1Ei | Decl,RelChild | rel: 1
// CHECK-NEXT: RelChild | Cls | c:@S@Cls
// CHECK: [[@LINE+1]]:3 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
@@ -350,3 +350,19 @@ void innerUsingNamespace() {
// CHECK-NOT: [[@LINE-3]]:21
}
}
+
+void indexDefaultValueInDefn(Cls cls = Cls(gvi), Record param = Record()) {
+// CHECK: [[@LINE-1]]:40 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:44 | variable/C | gvi | c:@gvi | _gvi | Ref,Read,RelCont
| rel: 1
+// CHECK-NOT: [[@LINE-3]]:44
+// CHECK: [[@LINE-4]]:65 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+// CHECK-NOT: [[@LINE-5]]:65
+}
+
+template class T>
+struct IndexDefaultValue {
+ IndexDefaultValue(int k = Record::C) {
+// CHECK: [[@LINE-1]]:38 | static-property/C++ | C | c:@S@Record@C |
__ZN6Record1CE | Ref,Read,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:30 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+ }
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
bader added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); yaxunl wrote: > bader wrote: > > yaxunl wrote: > > > yaxunl wrote: > > > > bader wrote: > > > > > yaxunl wrote: > > > > > > Anastasia wrote: > > > > > > > echuraev wrote: > > > > > > > > yaxunl wrote: > > > > > > > > > Anastasia wrote: > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could > > > > > > > > > > > > you please verify that changing from `SIZE_MAX` to `0` > > > > > > > > > > > > would be fine. > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for `CLK_NULL_EVENT`. > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail > > > > > > > > > > > and not part of the spec. I would suggest to remove it > > > > > > > > > > > from this header file. > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be defined > > > > > > > > > > > but does not define its value. Naturally a valid id > > > > > > > > > > > starts from 0 and increases. I don't see significant > > > > > > > > > > > advantage to change CLK_NULL_RESERVE_ID from __SIZE_MAX > > > > > > > > > > > to 0. > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > I don't see issues to commit things outside of spec as soon > > > > > > > > > > as they prefixed properly with "__". But I agree it would > > > > > > > > > > be nice to see if it's any useful and what the motivation > > > > > > > > > > is for having different implementation. > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > implementations assume that. Actually I am curious why that > > > > > > > > > is needed too. > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is valid > > > > > > > > if significant bit equal to one. `CLK_NULL_RESERVE_ID refers to > > > > > > > > an invalid reservation, so if `CLK_NULL_RESERVE_ID equal to 0, > > > > > > > > we can be sure that significant bit doesn't equal to 1 and it > > > > > > > > is invalid reserve id. Also it is more obviously if > > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > previous implementation also assumes that one specific bit was > > > > > > > > of a reverse id was used to indicate that the reserve id is > > > > > > > > valid. So, we just increased reserve id size by one bit on > > > > > > > > 32-bit platforms and by 33 bits on 64-bit platforms. > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but > > > > > > > spec doesn't define it of course. > > > > > > In our implementation, valid reserve id starts at 0 and increasing > > > > > > linearly until `__SIZE_MAX-1`. This change will break our > > > > > > implementation. > > > > > > > > > > > > However, we can modify our implementation to adopt this change > > > > > > since it brings about benefits overall. > > > > > Ideally it would be great to have unified implementation, but we can > > > > > define device specific value for CLK_NULL_RESERVE_ID by using ifdef > > > > > directive. > > > > How about > > > > > > > > ``` > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > ``` > > > > I think the spec does not require it to be compile time constant. Then > > > > each library can implement its own __clk_null_reserve_id() whereas the > > > > IR is target independent. > > > Or we only do this for SPIR and define it as target specific value for > > > other targets. > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but IMHO > > compile time constant is preferable option. > > I don't think making it compile time constant for SPIR only makes sense to > > me - in this case we can use constant for all targets. > > > > How about following approach: use 0 by default and allow other targets > > re-define CLK_NULL_RESERVE_ID value. > > > > ``` > > #ifndef CLK_NULL_RESERVE_ID > > #define CLK_NULL_RESERVE_ID 0 > > #endif // CLK_NULL_RESERVE_ID > > ``` > > > > If CLK_NULL_RESERVE_ID defined via -D command line option or included > > before OpenCL C header file (via -include option), the defined value will > > be used, otherwise 0. > > > > Will it work for you? > No. That makes us unable to consume SPIR since CLK_N
r303557 - clang-format: [JS] avoid line breaks before unindented r_parens.
Author: mprobst
Date: Mon May 22 09:58:26 2017
New Revision: 303557
URL: http://llvm.org/viewvc/llvm-project?rev=303557&view=rev
Log:
clang-format: [JS] avoid line breaks before unindented r_parens.
The change that enabled wrapping at the previous scope's indentation had
unintended side-effects in that clang-format would prefer to wrap
closing parentheses to the next line if it avoided a wrap on the next
line (assuming very narrow lines):
fooObject
.someCall(barbazbam)
.then(bam);
Would get formatted as:
fooObject.someCall(barbazbam
).then(bam);
Because the ')' is now indented at the parent level (fooObject).
Normally formatting a builder pattern style call sequence like that is
outlawed in clang-format anyway. However for JavaScript this is special
cased to support trailing .bind calls.
This change disallows this special case when following a closing ')' to
avoid the problem.
Included are some random comment fixes.
Reviewers: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D33399
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon May 22 09:58:26 2017
@@ -207,7 +207,7 @@ bool ContinuationIndenter::mustBreak(con
// ...
// }.bind(...));
// FIXME: We should find a more generic solution to this problem.
- !(State.Column <= NewLineColumn &&
+ !(State.Column <= NewLineColumn && Previous.isNot(tok::r_paren) &&
Style.Language == FormatStyle::LK_JavaScript))
return true;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon May 22 09:58:26 2017
@@ -2570,7 +2570,7 @@ bool TokenAnnotator::canBreakBefore(cons
Keywords.kw_interface, Keywords.kw_type, tok::kw_static,
tok::kw_public, tok::kw_private, tok::kw_protected,
Keywords.kw_abstract, Keywords.kw_get, Keywords.kw_set))
- return false; // Otherwise a semicolon is inserted.
+ return false; // Otherwise automatic semicolon insertion would trigger.
if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
return false;
if (Left.is(TT_JsTypeColon))
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon May 22 09:58:26 2017
@@ -1823,6 +1823,11 @@ TEST_F(FormatTestJS, NonNullAssertionOpe
verifyFormat("let x = !foo;\n");
verifyFormat("let x = foo[0]!;\n");
verifyFormat("let x = (foo)!;\n");
+ verifyFormat("let x = x(foo!);\n");
+ verifyFormat(
+ "a.aa(a.a!).then(\n"
+ "x => x(x));\n",
+ getGoogleJSStyleWithColumns(20));
verifyFormat("let x = foo! - 1;\n");
verifyFormat("let x = {foo: 1}!;\n");
verifyFormat(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33285: clang-format: do not reflow bullet lists
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303556: clang-format: do not reflow bullet lists (authored
by Typz).
Changed prior to commit:
https://reviews.llvm.org/D33285?vs=99436&id=99764#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33285
Files:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -1577,7 +1577,7 @@
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines having content that is a single character.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1602,7 +1602,7 @@
format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines starting with 'TODO'.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1671,6 +1671,69 @@
"// long",
getLLVMStyleWithColumns(20)));
+ // Don't reflow separate bullets in list
+ EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+ "// * long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+ "// + long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+ "// 2. long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+ "// -# long",
+ getLLVMStyleWithColumns(20)));
+
+ EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// - long",
+format("// - long long long long\n"
+ "// long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+
+ // Large number (>2 digits) are not list items
+ EXPECT_EQ("// long long long\n"
+"// long 1024. long.",
+format("// long long long long\n"
+ "// 1024. long.",
+ getLLVMStyleWithColumns(20)));
+
+ // Do not break before number, to avoid introducing a non-reflowable doxygen
+ // list item.
+ EXPECT_EQ("// long long\n"
+"// long 10. long.",
+format("// long long long 10.\n"
+ "// long.",
+ getLLVMStyleWithColumns(20)));
+
// Don't break or reflow after implicit string literals.
verifyFormat("#include // l l l\n"
" // l",
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -78,6 +78,14 @@
}
StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+ // Do not split before a number followed by a dot: this would be interpreted
+ // as a numbered list, which would prevent re-flowing in subsequent passes.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+ if (SpaceOffset != StringRef::npos &&
+ kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
if (SpaceOffset == StringRef::npos ||
// Don't break at leading whitespace.
Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -299,15 +307,24 @@
static bool mayReflowContent(StringRef Content) {
Content = Content.trim(Blanks);
// Lines starting with '@' commonly have special meaning.
- static const SmallVector kSpecialMeaningPrefixes = {
- "@", "TODO", "FIXME", "XXX"};
+ // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+ static const SmallVector kSpecialMeaningPrefixes = {
+ "@", "TODO", "FIXME"
r303556 - clang-format: do not reflow bullet lists
Author: typz
Date: Mon May 22 09:47:17 2017
New Revision: 303556
URL: http://llvm.org/viewvc/llvm-project?rev=303556&view=rev
Log:
clang-format: do not reflow bullet lists
Summary:
This patch prevents reflowing bullet lists in block comments.
It handles all lists supported by doxygen and markdown, e.g. bullet
lists starting with '-', '*', '+', as well as numbered lists starting
with -# or a number followed by a dot.
Reviewers: krasimir
Reviewed By: krasimir
Subscribers: djasper, klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D33285
Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=303556&r1=303555&r2=303556&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Mon May 22 09:47:17 2017
@@ -78,6 +78,14 @@ static BreakableToken::Split getCommentS
}
StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+ // Do not split before a number followed by a dot: this would be interpreted
+ // as a numbered list, which would prevent re-flowing in subsequent passes.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+ if (SpaceOffset != StringRef::npos &&
+ kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
if (SpaceOffset == StringRef::npos ||
// Don't break at leading whitespace.
Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -299,8 +307,9 @@ const FormatToken &BreakableComment::tok
static bool mayReflowContent(StringRef Content) {
Content = Content.trim(Blanks);
// Lines starting with '@' commonly have special meaning.
- static const SmallVector kSpecialMeaningPrefixes = {
- "@", "TODO", "FIXME", "XXX"};
+ // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+ static const SmallVector kSpecialMeaningPrefixes = {
+ "@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* " };
bool hasSpecialMeaningPrefix = false;
for (StringRef Prefix : kSpecialMeaningPrefixes) {
if (Content.startswith(Prefix)) {
@@ -308,6 +317,14 @@ static bool mayReflowContent(StringRef C
break;
}
}
+
+ // Numbered lists may also start with a number followed by '.'
+ // To avoid issues if a line starts with a number which is actually the end
+ // of a previous line, we only consider numbers with up to 2 digits.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\. ");
+ hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||
+kNumberedListRegexp.match(Content);
+
// Simple heuristic for what to reflow: content should contain at least two
// characters and either the first or second character must be
// non-punctuation.
Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=303556&r1=303555&r2=303556&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Mon May 22 09:47:17 2017
@@ -1577,7 +1577,7 @@ TEST_F(FormatTestComments, ReflowsCommen
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines having content that is a single character.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1602,7 +1602,7 @@ TEST_F(FormatTestComments, ReflowsCommen
format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines starting with 'TODO'.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1671,6 +1671,69 @@ TEST_F(FormatTestComments, ReflowsCommen
"// long",
getLLVMStyleWithColumns(20)));
+ // Don't reflow separate bullets in list
+ EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+ "// * long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+ "// + long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// 1. long long
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
yaxunl added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); bader wrote: > yaxunl wrote: > > yaxunl wrote: > > > bader wrote: > > > > yaxunl wrote: > > > > > Anastasia wrote: > > > > > > echuraev wrote: > > > > > > > yaxunl wrote: > > > > > > > > Anastasia wrote: > > > > > > > > > yaxunl wrote: > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could you > > > > > > > > > > > please verify that changing from `SIZE_MAX` to `0` would > > > > > > > > > > > be fine. > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for `CLK_NULL_EVENT`. > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail and > > > > > > > > > > not part of the spec. I would suggest to remove it from > > > > > > > > > > this header file. > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be defined > > > > > > > > > > but does not define its value. Naturally a valid id starts > > > > > > > > > > from 0 and increases. I don't see significant advantage to > > > > > > > > > > change CLK_NULL_RESERVE_ID from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > I don't see issues to commit things outside of spec as soon > > > > > > > > > as they prefixed properly with "__". But I agree it would be > > > > > > > > > nice to see if it's any useful and what the motivation is for > > > > > > > > > having different implementation. > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > indicate that the reserve id is valid. Not all implementations > > > > > > > > assume that. Actually I am curious why that is needed too. > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is valid if > > > > > > > significant bit equal to one. `CLK_NULL_RESERVE_ID refers to an > > > > > > > invalid reservation, so if `CLK_NULL_RESERVE_ID equal to 0, we > > > > > > > can be sure that significant bit doesn't equal to 1 and it is > > > > > > > invalid reserve id. Also it is more obviously if > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > previous implementation also assumes that one specific bit was of > > > > > > > a reverse id was used to indicate that the reserve id is valid. > > > > > > > So, we just increased reserve id size by one bit on 32-bit > > > > > > > platforms and by 33 bits on 64-bit platforms. > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but spec > > > > > > doesn't define it of course. > > > > > In our implementation, valid reserve id starts at 0 and increasing > > > > > linearly until `__SIZE_MAX-1`. This change will break our > > > > > implementation. > > > > > > > > > > However, we can modify our implementation to adopt this change since > > > > > it brings about benefits overall. > > > > Ideally it would be great to have unified implementation, but we can > > > > define device specific value for CLK_NULL_RESERVE_ID by using ifdef > > > > directive. > > > How about > > > > > > ``` > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > ``` > > > I think the spec does not require it to be compile time constant. Then > > > each library can implement its own __clk_null_reserve_id() whereas the IR > > > is target independent. > > Or we only do this for SPIR and define it as target specific value for > > other targets. > Defining CLK_NULL_RESERVE_ID as a function call should also work, but IMHO > compile time constant is preferable option. > I don't think making it compile time constant for SPIR only makes sense to me > - in this case we can use constant for all targets. > > How about following approach: use 0 by default and allow other targets > re-define CLK_NULL_RESERVE_ID value. > > ``` > #ifndef CLK_NULL_RESERVE_ID > #define CLK_NULL_RESERVE_ID 0 > #endif // CLK_NULL_RESERVE_ID > ``` > > If CLK_NULL_RESERVE_ID defined via -D command line option or included before > OpenCL C header file (via -include option), the defined value will be used, > otherwise 0. > > Will it work for you? No. That makes us unable to consume SPIR since CLK_NULL_RESERVE_ID is hardcoded as 0 when the source code is translated to SPIR whereas our target expects ~0U. To get a portable IR we need to represent CLK_NULL_RESERVE_ID as a function which can be lowered to a constant by a specific t
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz updated this revision to Diff 99763.
Typz added a comment.
Remove dependency on https://reviews.llvm.org/D33314
https://reviews.llvm.org/D32480
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1309,6 +1309,140 @@
Style));
}
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+ FormatStyle Style = getLLVMStyle();
+ Style.CompactNamespaces = true;
+
+ verifyFormat("namespace A { namespace B {\n"
+ "}} // namespace A::B",
+ Style);
+
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "} // namespace in\n"
+ "} // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace out { namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"}} // namespace out::in2",
+format("namespace out {\n"
+ "namespace in1 {\n"
+ "} // namespace in1\n"
+ "namespace in2 {\n"
+ "} // namespace in2\n"
+ "} // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+ "namespace in { int j; } // namespace in\n"
+ "int k; } // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+ "namespace C {\n"
+ "}} // namespace B::C\n"
+ "} // namespace A\n",
+ Style));
+
+ EXPECT_EQ("namespace {\n"
+ "namespace {\n"
+"}} // namespace ::",
+format("namespace {\n"
+ "namespace {\n"
+ "} // namespace \n"
+ "} // namespace ",
+ Style));
+
+ EXPECT_EQ("namespace a { namespace b {\n"
+ "namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+ "namespace b {\n"
+ "namespace c {\n"
+ "} // namespace c\n"
+ "} // namespace b\n"
+ "} // namespace a",
+ Style));
+
+ // Missing comments are added
+ EXPECT_EQ("namespace out { namespace in {\n"
+ "int i;\n"
+ "int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n"
+ "}",
+ Style));
+
+ // Incorrect comments are fixed
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+ "}} // namespace out",
+ Style));
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+ "}} // namespace in",
+ Style));
+
+ // Extra semicolon after 'inner' closing brace prevents merging
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "}; // namespace in\n"
+ "} // namespace out",
+ Style));
+
+ // Extra semicolon after 'outer' closing brace is conserved
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "} // namespace in\n"
+ "}; // namespace out",
+ Style));
+
+ Style.NamespaceIndentation = FormatStyle::NI_All;
+ EXPECT_EQ("namespace out { namespace in {\n"
+"int i;\n"
+"}} // namespace out::in",
+format("na
