From b6781a8eae399e339951cd12429930f28c886c71 Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 01:56:39 +0100 Subject: [PATCH 1/8] Added proper "need 'this' to access member foo" errors instead of "variable foo not resolved" for some cases, added FIXME for the old error! Added a bit more information to the runtime's cyclic dependency detection exception. --- gen/toir.cpp | 11 +++++++++++ runtime/internal/genobj.d | 11 ++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/gen/toir.cpp b/gen/toir.cpp index 1e5e987c..11829ce4 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -56,10 +56,18 @@ DValue* VarExp::toElem(IRState* p) LOG_SCOPE; assert(var); + if (VarDeclaration* vd = var->isVarDeclaration()) { Logger::println("VarDeclaration ' %s ' of type ' %s '", vd->toChars(), vd->type->toChars()); + // this is an error! must be accessed with DotVarExp + if (var->needThis()) + { + error("need 'this' to access member %s", toChars()); + fatal(); + } + // _arguments if (vd->ident == Id::_arguments && p->func()->_arguments) { @@ -144,6 +152,9 @@ DValue* VarExp::toElem(IRState* p) LLValue* val; if (!vd->ir.isSet() || !(val = vd->ir.getIrValue())) { + // FIXME: this error is bad! + // We should be VERY careful about adding errors in general, as they have + // a tendency to "mask" out the underlying problems ... error("variable %s not resolved", vd->toChars()); if (Logger::enabled()) Logger::cout() << "unresolved variable had type: " << *DtoType(vd->type) << '\n'; diff --git a/runtime/internal/genobj.d b/runtime/internal/genobj.d index 5a64b886..01499abb 100644 --- a/runtime/internal/genobj.d +++ b/runtime/internal/genobj.d @@ -1060,7 +1060,7 @@ extern (C) void _moduleCtor() _moduleinfo_dtors = new ModuleInfo[_moduleinfo_array.length]; debug(PRINTF) printf("_moduleinfo_dtors = x%x\n", cast(void *)_moduleinfo_dtors); _moduleIndependentCtors(); - _moduleCtor2(_moduleinfo_array, 0); + _moduleCtor2(null, _moduleinfo_array, 0); } extern (C) void _moduleIndependentCtors() @@ -1076,7 +1076,7 @@ extern (C) void _moduleIndependentCtors() debug(PRINTF) printf("_moduleIndependentCtors() DONE\n"); } -void _moduleCtor2(ModuleInfo[] mi, int skip) +void _moduleCtor2(ModuleInfo from, ModuleInfo[] mi, int skip) { debug(PRINTF) printf("_moduleCtor2(): %d modules\n", mi.length); for (uint i = 0; i < mi.length; i++) @@ -1096,11 +1096,12 @@ void _moduleCtor2(ModuleInfo[] mi, int skip) if (m.flags & MIctorstart) { if (skip || m.flags & MIstandalone) continue; - throw new Exception( "Cyclic dependency in module " ~ m.name ); + assert(from !is null); + throw new Exception( "Cyclic dependency in module " ~ from.name ~ " for import " ~ m.name); } m.flags |= MIctorstart; - _moduleCtor2(m.importedModules, 0); + _moduleCtor2(m, m.importedModules, 0); if (m.ctor) (*m.ctor)(); m.flags &= ~MIctorstart; @@ -1114,7 +1115,7 @@ void _moduleCtor2(ModuleInfo[] mi, int skip) else { m.flags |= MIctordone; - _moduleCtor2(m.importedModules, 1); + _moduleCtor2(m, m.importedModules, 1); } } debug(PRINTF) printf("_moduleCtor2() DONE\n"); From a2bf0796ce2527a7c75ca804f9081925809e4f60 Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 02:07:24 +0100 Subject: [PATCH 2/8] Added back a bunch of static's to gen/classes.cpp, it's not superfluous, it makes the functions internal to the compilation unit, and we don't call them anywhere else. --- gen/classes.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/gen/classes.cpp b/gen/classes.cpp index 3132604a..0597b796 100644 --- a/gen/classes.cpp +++ b/gen/classes.cpp @@ -22,7 +22,7 @@ ////////////////////////////////////////////////////////////////////////////////////////// // adds the base interfaces of b and the given iri to IrStruct's interfaceMap -void add_base_interfaces(IrStruct* to, IrInterface* iri, BaseClass* b) +static void add_base_interfaces(IrStruct* to, IrInterface* iri, BaseClass* b) { for (unsigned j = 0; j < b->baseInterfaces_dim; j++) { @@ -39,7 +39,7 @@ void add_base_interfaces(IrStruct* to, IrInterface* iri, BaseClass* b) // adds interface b to target, if newinstance != 0, then target must provide all // functions required to implement b (it reimplements b) -void add_interface(ClassDeclaration* target, BaseClass* b, int newinstance) +static void add_interface(ClassDeclaration* target, BaseClass* b, int newinstance) { Logger::println("adding interface: %s", b->base->toChars()); LOG_SCOPE; @@ -90,7 +90,7 @@ void add_interface(ClassDeclaration* target, BaseClass* b, int newinstance) ////////////////////////////////////////////////////////////////////////////////////////// -void add_class_data(ClassDeclaration* target, ClassDeclaration* cd) +static void add_class_data(ClassDeclaration* target, ClassDeclaration* cd) { Logger::println("Adding data from class: %s", cd->toChars()); LOG_SCOPE; @@ -125,7 +125,7 @@ void add_class_data(ClassDeclaration* target, ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////////////////// -void DtoResolveInterface(InterfaceDeclaration* cd) +static void DtoResolveInterface(InterfaceDeclaration* cd) { if (cd->ir.resolved) return; cd->ir.resolved = true; @@ -275,7 +275,7 @@ void DtoResolveClass(ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////////////////// -void DtoDeclareInterface(InterfaceDeclaration* cd) +static void DtoDeclareInterface(InterfaceDeclaration* cd) { if (cd->ir.declared) return; cd->ir.declared = true; @@ -440,7 +440,7 @@ void DtoDeclareClass(ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////// // adds data fields and interface vtables to the constant initializer of class cd -size_t init_class_initializer(std::vector& inits, ClassDeclaration* target, ClassDeclaration* cd, size_t offsetbegin) +static size_t init_class_initializer(std::vector& inits, ClassDeclaration* target, ClassDeclaration* cd, size_t offsetbegin) { // first do baseclasses if (cd->baseClass) @@ -543,7 +543,7 @@ size_t init_class_initializer(std::vector& inits, ClassDeclaration* ////////////////////////////////////////////////////////////////////////////// // build the vtable initializer for class cd -void init_class_vtbl_initializer(ClassDeclaration* cd) +static void init_class_vtbl_initializer(ClassDeclaration* cd) { // generate vtable initializer std::vector sinits(cd->vtbl.dim, NULL); @@ -596,7 +596,7 @@ void init_class_vtbl_initializer(ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////// -void init_class_interface_vtbl_initializers(ClassDeclaration* cd) +static void init_class_interface_vtbl_initializers(ClassDeclaration* cd) { IrStruct* irstruct = cd->ir.irStruct; @@ -697,7 +697,7 @@ void init_class_interface_vtbl_initializers(ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////// -void DtoConstInitInterface(InterfaceDeclaration* cd) +static void DtoConstInitInterface(InterfaceDeclaration* cd) { if (cd->ir.initialized) return; cd->ir.initialized = true; @@ -780,7 +780,7 @@ void DtoConstInitClass(ClassDeclaration* cd) ////////////////////////////////////////////////////////////////////////////////////////// -void DefineInterfaceInfos(IrStruct* irstruct) +static void DefineInterfaceInfos(IrStruct* irstruct) { // always do interface info array when possible std::vector infoInits; @@ -810,7 +810,7 @@ void DefineInterfaceInfos(IrStruct* irstruct) ////////////////////////////////////////////////////////////////////////////////////////// -void DtoDefineInterface(InterfaceDeclaration* cd) +static void DtoDefineInterface(InterfaceDeclaration* cd) { if (cd->ir.defined) return; cd->ir.defined = true; @@ -1323,7 +1323,7 @@ void DtoDeclareClassInfo(ClassDeclaration* cd) #if GENERATE_OFFTI // build a single element for the OffsetInfo[] of ClassInfo -LLConstant* build_offti_entry(ClassDeclaration* cd, VarDeclaration* vd) +static LLConstant* build_offti_entry(ClassDeclaration* cd, VarDeclaration* vd) { std::vector inits(2); @@ -1348,7 +1348,7 @@ LLConstant* build_offti_entry(ClassDeclaration* cd, VarDeclaration* vd) return llvm::ConstantStruct::get(inits); } -LLConstant* build_offti_array(ClassDeclaration* cd, const LLType* arrayT) +static LLConstant* build_offti_array(ClassDeclaration* cd, const LLType* arrayT) { IrStruct* irstruct = cd->ir.irStruct; @@ -1383,7 +1383,7 @@ LLConstant* build_offti_array(ClassDeclaration* cd, const LLType* arrayT) #endif // GENERATE_OFFTI -LLConstant* build_class_dtor(ClassDeclaration* cd) +static LLConstant* build_class_dtor(ClassDeclaration* cd) { FuncDeclaration* dtor = cd->dtor; @@ -1395,7 +1395,7 @@ LLConstant* build_class_dtor(ClassDeclaration* cd) return llvm::ConstantExpr::getBitCast(dtor->ir.irFunc->func, getPtrToType(LLType::Int8Ty)); } -unsigned build_classinfo_flags(ClassDeclaration* cd) +static unsigned build_classinfo_flags(ClassDeclaration* cd) { // adapted from original dmd code unsigned flags = 0; From 417aa575015ba8f8d9e724064d88cade96aa6d60 Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 03:01:19 +0100 Subject: [PATCH 3/8] This ''should'' fix #139 , I failed to produce a testcase, but I would imagine this to be correct, and it removes '''that''' error when building Hybrid. --- gen/toir.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gen/toir.cpp b/gen/toir.cpp index 11829ce4..8186d963 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -224,6 +224,13 @@ LLConstant* VarExp::toConstElem(IRState* p) m = llvm::ConstantExpr::getBitCast(m, vartype); return m; } + else if (VarDeclaration* vd = var->isVarDeclaration()) + { + // return the initializer + assert(vd->init); + return DtoConstInitializer(loc, type, vd->init); + } + // fail assert(0 && "Unsupported const VarExp kind"); return NULL; } From 3c400ff21ce843ee1ef57613870099ea144d547e Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 14:07:30 +0100 Subject: [PATCH 4/8] Removed error on naked, not fully complete, but I'll be doing more work on it during this Christmas, and some things do work. Fixed taking delegate of final class method. see mini/delegate3.d. --- gen/classes.cpp | 1 + gen/functions.cpp | 7 ------- gen/toir.cpp | 2 +- tests/mini/delegate3.d | 18 ++++++++++++++++++ 4 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 tests/mini/delegate3.d diff --git a/gen/classes.cpp b/gen/classes.cpp index 0597b796..bbe4dcf7 100644 --- a/gen/classes.cpp +++ b/gen/classes.cpp @@ -1260,6 +1260,7 @@ LLValue* DtoVirtualFunctionPointer(DValue* inst, FuncDeclaration* fdecl) { // sanity checks assert(fdecl->isVirtual()); + assert(!fdecl->isFinal()); assert(fdecl->vtblIndex > 0); // 0 is always ClassInfo/Interface* assert(inst->getType()->toBasetype()->ty == Tclass); diff --git a/gen/functions.cpp b/gen/functions.cpp index 3c695645..9e9065b3 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -628,13 +628,6 @@ void DtoDefineFunc(FuncDeclaration* fd) Logger::println("DtoDefineFunc(%s): %s", fd->toPrettyChars(), fd->loc.toChars()); LOG_SCOPE; - // error on naked - if (fd->naked) - { - fd->error("naked is not supported"); - fatal(); - } - // debug info if (global.params.symdebug) { Module* mo = fd->getModule(); diff --git a/gen/toir.cpp b/gen/toir.cpp index 8186d963..dd581f0c 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -1970,7 +1970,7 @@ DValue* DelegateExp::toElem(IRState* p) Logger::println("func: '%s'", func->toPrettyChars()); LLValue* castfptr; - if (func->isVirtual()) + if (func->isVirtual() && !func->isFinal()) castfptr = DtoVirtualFunctionPointer(u, func); else if (func->isAbstract()) assert(0 && "TODO delegate to abstract method"); diff --git a/tests/mini/delegate3.d b/tests/mini/delegate3.d new file mode 100644 index 00000000..06c3d644 --- /dev/null +++ b/tests/mini/delegate3.d @@ -0,0 +1,18 @@ +module bar; + +class S +{ + int i; + final int foo() + { + return i; + } +} + +void main() +{ + auto s = new S; + s.i = 42; + auto dg = &s.foo; + assert(dg() == 42); +} From 99396c2e7a72eb27f0a1d56d27cd354f184cc3d7 Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 14:57:01 +0100 Subject: [PATCH 5/8] Fixed problem with nested function inside static nested function. see mini/compile_nested2.d. fixes #143 . --- gen/functions.cpp | 35 +++++++++++++++++++++-------------- tests/mini/compile_nested2.d | 13 +++++++++++++ 2 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 tests/mini/compile_nested2.d diff --git a/gen/functions.cpp b/gen/functions.cpp index 9e9065b3..94270323 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -771,24 +771,31 @@ void DtoDefineFunc(FuncDeclaration* fd) if (!fd->nestedVars.empty()) { Logger::println("has nested frame"); - // start with add all enclosing parent frames + // start with adding all enclosing parent frames until a static parent is reached int nparelems = 0; - Dsymbol* par = fd->toParent2(); - while (par) + if (!fd->isStatic()) { - if (FuncDeclaration* parfd = par->isFuncDeclaration()) + Dsymbol* par = fd->toParent2(); + while (par) { - nparelems += parfd->nestedVars.size(); + if (FuncDeclaration* parfd = par->isFuncDeclaration()) + { + nparelems += parfd->nestedVars.size(); + // stop at first static + if (parfd->isStatic()) + break; + } + else if (ClassDeclaration* parcd = par->isClassDeclaration()) + { + // nothing needed + } + else + { + break; + } + + par = par->toParent2(); } - else if (ClassDeclaration* parcd = par->isClassDeclaration()) - { - // nothing needed - } - else - { - break; - } - par = par->toParent2(); } int nelems = fd->nestedVars.size() + nparelems; diff --git a/tests/mini/compile_nested2.d b/tests/mini/compile_nested2.d new file mode 100644 index 00000000..e05a4ffc --- /dev/null +++ b/tests/mini/compile_nested2.d @@ -0,0 +1,13 @@ +void test(void delegate() spam) +{ + static void foo() // static is the problem + { + uint x; + void peek() { x = 0; } + } + + void bar() + { + spam(); + } +} From 827113a0b2fe489037752cff4450ec129a7fc45a Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 17:14:52 +0100 Subject: [PATCH 6/8] Fixed another moreatatime (as opposed to oneatatime) issue with indexing unresolved class. --- gen/classes.cpp | 3 +++ gen/tocall.cpp | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/gen/classes.cpp b/gen/classes.cpp index bbe4dcf7..f5ab7320 100644 --- a/gen/classes.cpp +++ b/gen/classes.cpp @@ -1218,6 +1218,9 @@ LLValue* DtoIndexClass(LLValue* src, ClassDeclaration* cd, VarDeclaration* vd) if (Logger::enabled()) Logger::cout() << "src: " << *src << '\n'; + // make sure class is resolved + DtoResolveClass(cd); + // vd must be a field IrField* field = vd->ir.irField; assert(field); diff --git a/gen/tocall.cpp b/gen/tocall.cpp index 05220cc6..94be68f0 100644 --- a/gen/tocall.cpp +++ b/gen/tocall.cpp @@ -242,10 +242,8 @@ DValue* DtoCallFunction(Loc& loc, Type* resulttype, DValue* fnval, Expressions* const LLFunctionType* callableTy = DtoExtractFunctionType(callable->getType()); assert(callableTy); - if (Logger::enabled()) - { - Logger::cout() << "callable: " << *callable << '\n'; - } +// if (Logger::enabled()) +// Logger::cout() << "callable: " << *callable << '\n'; // get n arguments size_t n_arguments = arguments ? arguments->dim : 0; From 637c59a42294a4e788dc9c1e12d135f30580f1c2 Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Tue, 9 Dec 2008 18:45:25 +0100 Subject: [PATCH 7/8] more moreatatime fixes --- gen/classes.cpp | 1 + gen/structs.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/gen/classes.cpp b/gen/classes.cpp index f5ab7320..f9ae157c 100644 --- a/gen/classes.cpp +++ b/gen/classes.cpp @@ -1059,6 +1059,7 @@ DValue* DtoCastClass(DValue* val, Type* _to) Logger::println("static down cast"); // get the from class ClassDeclaration* cd = fc->sym->isClassDeclaration(); + DtoResolveClass(cd); // add this IrStruct* irstruct = cd->ir.irStruct; // find interface impl IrStruct::InterfaceMapIter iriter = irstruct->interfaceMap.find(it); diff --git a/gen/structs.cpp b/gen/structs.cpp index c5202959..b0ca9049 100644 --- a/gen/structs.cpp +++ b/gen/structs.cpp @@ -303,6 +303,8 @@ LLValue* DtoIndexStruct(LLValue* src, StructDeclaration* sd, VarDeclaration* vd) Logger::println("indexing struct field %s:", vd->toPrettyChars()); LOG_SCOPE; + DtoResolveStruct(sd); + // vd must be a field IrField* field = vd->ir.irField; assert(field); From c14996f39e97ada7bed59c5a4dc2c40bc53b9e0f Mon Sep 17 00:00:00 2001 From: Tomas Lindquist Olsen Date: Wed, 10 Dec 2008 13:56:10 +0100 Subject: [PATCH 8/8] Removed insufficient fix for DMD bug 1161, it was causing problems with instantiating imported templates, and passing private variables as aliases. I failed to come up with a proper fix! --- dmd/expression.c | 4 ---- dmd2/expression.c | 4 ---- 2 files changed, 8 deletions(-) diff --git a/dmd/expression.c b/dmd/expression.c index ddf71ea0..516c6f21 100644 --- a/dmd/expression.c +++ b/dmd/expression.c @@ -3761,10 +3761,6 @@ Expression *VarExp::semantic(Scope *sc) #endif } - // LDC: Fixes bug 1161, http://d.puremagic.com/issues/show_bug.cgi?id=1161 - // check access to VarDeclaration - accessCheck(loc, sc, NULL, var); - VarDeclaration *v = var->isVarDeclaration(); if (v) { diff --git a/dmd2/expression.c b/dmd2/expression.c index b2af49c2..7212c51f 100644 --- a/dmd2/expression.c +++ b/dmd2/expression.c @@ -3829,10 +3829,6 @@ Expression *VarExp::semantic(Scope *sc) #endif } - // LDC: Fixes bug 1161, http://d.puremagic.com/issues/show_bug.cgi?id=1161 - // check access to VarDeclaration - accessCheck(loc, sc, NULL, var); - VarDeclaration *v = var->isVarDeclaration(); if (v) {