It looks like turning it to an assert wouldn’t be correct, I’ll probably just remove it.
> On May 2, 2016, at 3:12 PM, Akira Hatanaka via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > I see. Perhaps this should be an assert? > >> On May 2, 2016, at 3:05 PM, Richard Smith <rich...@metafoo.co.uk >> <mailto:rich...@metafoo.co.uk>> wrote: >> >> On Mon, May 2, 2016 at 2:52 PM, Akira Hatanaka via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> Author: ahatanak >> Date: Mon May 2 16:52:57 2016 >> New Revision: 268314 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=268314&view=rev >> <http://llvm.org/viewvc/llvm-project?rev=268314&view=rev> >> Log: >> [CodeGenObjCXX] Don't rematerialize default arguments of function >> parameters in the body of a block. >> >> This fixes a bug where clang would materialize the default argument >> inside the body of a block instead of passing the value via the block >> descriptor. >> >> For example, in the code below, foo1 would always print 42 regardless >> of the value of argument "a" passed to foo1. >> >> void foo1(const int a = 42 ) { >> auto block = ^{ >> printf("%d\n", a); >> }; >> block(); >> } >> >> rdar://problem/24449235 <rdar://problem/24449235> >> >> Added: >> cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm >> <http://block-default-arg.mm/> >> Modified: >> cfe/trunk/lib/CodeGen/CGBlocks.cpp >> >> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=268314&r1=268313&r2=268314&view=diff >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=268314&r1=268313&r2=268314&view=diff> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Mon May 2 16:52:57 2016 >> @@ -262,6 +262,11 @@ static bool isSafeForCXXConstantCapture( >> static llvm::Constant *tryCaptureAsConstant(CodeGenModule &CGM, >> CodeGenFunction *CGF, >> const VarDecl *var) { >> + // Don't rematerialize default arguments of function parameters. >> + if (auto *PD = dyn_cast<ParmVarDecl>(var)) >> + if (PD->hasDefaultArg()) >> >> I don't think you need this test, and I think it somewhat confuses the >> intent here. (A reader would wonder why you want to keep going for >> ParmVarDecls that don't have default arguments.) >> >> + return nullptr; >> + >> QualType type = var->getType(); >> >> // We can only do this if the variable is const. >> >> Added: cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm >> <http://block-default-arg.mm/> >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm?rev=268314&view=auto >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm?rev=268314&view=auto> >> ============================================================================== >> --- cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm >> <http://block-default-arg.mm/> (added) >> +++ cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm >> <http://block-default-arg.mm/> Mon May 2 16:52:57 2016 >> @@ -0,0 +1,16 @@ >> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s >> -std=c++11 -fblocks -fobjc-arc | FileCheck %s >> + >> +// CHECK: define internal void @___Z16test_default_argi_block_invoke(i8* >> %[[BLOCK_DESCRIPTOR:.*]]) >> +// CHECK: %[[BLOCK:.*]] = bitcast i8* %[[BLOCK_DESCRIPTOR]] to <{ i8*, i32, >> i32, i8*, %struct.__block_descriptor*, i32 }>* >> +// CHECK: %[[BLOCK_CAPTURE_ADDR:.*]] = getelementptr inbounds <{ i8*, i32, >> i32, i8*, %struct.__block_descriptor*, i32 }>, <{ i8*, i32, i32, i8*, >> %struct.__block_descriptor*, i32 }>* %[[BLOCK]], i32 0, i32 5 >> +// CHECK: %[[V0:.*]] = load i32, i32* %[[BLOCK_CAPTURE_ADDR]] >> +// CHECK: call void @_Z4foo1i(i32 %[[V0]]) >> + >> +void foo1(int); >> + >> +void test_default_arg(const int a = 42) { >> + auto block = ^{ >> + foo1(a); >> + }; >> + block(); >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits