From 3c865d5eedaff4debf7f99df19c38ccb996a017b Mon Sep 17 00:00:00 2001 From: Walter Bright Date: Sun, 8 Oct 2017 01:03:00 -0700 Subject: [PATCH] fix Issue 16273 - [REG 2.072a] dmd segfault with inheritance, templates, override --- src/ddmd/dclass.d | 67 ++++++++++++++++++++++++++++++------- src/ddmd/declaration.d | 2 +- src/ddmd/declaration.h | 3 +- src/ddmd/dsymbolsem.d | 22 +++++++++++- src/ddmd/expressionsem.d | 1 + src/ddmd/func.d | 23 +++++++++++++ test/compilable/test16273.d | 22 ++++++++++++ 7 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 test/compilable/test16273.d diff --git a/src/ddmd/dclass.d b/src/ddmd/dclass.d index dd53b314c029..106a7e434fcf 100644 --- a/src/ddmd/dclass.d +++ b/src/ddmd/dclass.d @@ -12,6 +12,7 @@ module ddmd.dclass; // Online documentation: https://dlang.org/phobos/ddmd_dclass.html +import core.stdc.stdio; import core.stdc.string; import ddmd.aggregate; @@ -827,9 +828,21 @@ extern (C++) class ClassDeclaration : AggregateDeclaration */ final bool isAbstract() { + enum log = false; if (isabstract != ABSfwdref) return isabstract == ABSyes; + if (log) printf("isAbstract(%s)\n", toChars()); + + bool no() { if (log) printf("no\n"); isabstract = ABSno; return false; } + bool yes() { if (log) printf("yes\n"); isabstract = ABSyes; return true; } + + if (storage_class & STCabstract || _scope && _scope.stc & STCabstract) + return yes(); + + if (errors) + return no(); + /* https://issues.dlang.org/show_bug.cgi?id=11169 * Resolve forward references to all class member functions, * and determine whether this class is abstract. @@ -842,9 +855,6 @@ extern (C++) class ClassDeclaration : AggregateDeclaration if (fd.storage_class & STCstatic) return 0; - if (fd._scope) - fd.semantic(null); - if (fd.isAbstract()) return 1; return 0; @@ -855,26 +865,59 @@ extern (C++) class ClassDeclaration : AggregateDeclaration auto s = (*members)[i]; if (s.apply(&func, cast(void*)this)) { - isabstract = ABSyes; - return true; + return yes(); + } + } + + /* If the base class is not abstract, then this class cannot + * be abstract. + */ + if (!baseClass || !baseClass.isAbstract()) + return no(); + + /* If any abstract functions are inherited, but not overridden, + * then the class is abstract. Do this by checking the vtbl[]. + * Need to do semantic() on class to fill the vtbl[]. + */ + this.semantic(null); + + /* The next line should work, but does not because when ClassDeclaration.semantic() + * is called recursively it can set PASSsemanticdone without finishing it. + */ + //if (semanticRun < PASSsemanticdone) + { + /* Could not complete semantic(). Try running semantic() on + * each of the virtual functions, + * which will fill in the vtbl[] overrides. + */ + extern (C++) static int virtualSemantic(Dsymbol s, void* param) + { + auto fd = s.isFuncDeclaration(); + if (fd && !(fd.storage_class & STCstatic)) + fd.semantic(null); + return 0; + } + + for (size_t i = 0; i < members.dim; i++) + { + auto s = (*members)[i]; + s.apply(&virtualSemantic, cast(void*)this); } } - /* Iterate inherited member functions and check their abstract attribute. + /* Finally, check the vtbl[] */ - for (size_t i = 1; i < vtbl.dim; i++) + foreach (i; 1 .. vtbl.dim) { auto fd = vtbl[i].isFuncDeclaration(); - //if (fd) printf("\tvtbl[%d] = [%s] %s\n", i, fd.loc.toChars(), fd.toChars()); + //if (fd) printf("\tvtbl[%d] = [%s] %s\n", i, fd.loc.toChars(), fd.toPrettyChars()); if (!fd || fd.isAbstract()) { - isabstract = ABSyes; - return true; + return yes(); } } - isabstract = ABSno; - return false; + return no(); } /**************************************** diff --git a/src/ddmd/declaration.d b/src/ddmd/declaration.d index ecf4f24cb185..010ef5394c0d 100644 --- a/src/ddmd/declaration.d +++ b/src/ddmd/declaration.d @@ -265,7 +265,7 @@ extern (C++) abstract class Declaration : Dsymbol return (storage_class & STCfinal) != 0; } - final bool isAbstract() + bool isAbstract() { return (storage_class & STCabstract) != 0; } diff --git a/src/ddmd/declaration.h b/src/ddmd/declaration.h index b56e46efcf97..a50cb05387ec 100644 --- a/src/ddmd/declaration.h +++ b/src/ddmd/declaration.h @@ -141,7 +141,7 @@ class Declaration : public Dsymbol virtual bool isCodeseg(); bool isCtorinit() { return (storage_class & STCctorinit) != 0; } bool isFinal() { return (storage_class & STCfinal) != 0; } - bool isAbstract() { return (storage_class & STCabstract) != 0; } + virtual bool isAbstract() { return (storage_class & STCabstract) != 0; } bool isConst() { return (storage_class & STCconst) != 0; } bool isImmutable() { return (storage_class & STCimmutable) != 0; } bool isWild() { return (storage_class & STCwild) != 0; } @@ -596,6 +596,7 @@ class FuncDeclaration : public Declaration bool isImportedSymbol(); bool isCodeseg(); bool isOverloadable(); + bool isAbstract(); PURE isPure(); PURE isPureBypassingInference(); bool setImpure(); diff --git a/src/ddmd/dsymbolsem.d b/src/ddmd/dsymbolsem.d index edd035d80fe3..11a1d1451cd9 100644 --- a/src/ddmd/dsymbolsem.d +++ b/src/ddmd/dsymbolsem.d @@ -5393,7 +5393,13 @@ extern(C++) final class DsymbolSemanticVisitor : Visitor if (!sd.determineFields()) { - assert(sd.type.ty == Terror); + if (sd.type.ty != Terror) + { + sd.error(sd.loc, "circular or forward reference"); + sd.errors = true; + sd.type = Type.terror; + } + sc2.pop(); sd.semanticRun = PASSsemanticdone; return; @@ -6046,6 +6052,20 @@ extern(C++) final class DsymbolSemanticVisitor : Visitor sc2.pop(); + /* isAbstract() is undecidable in some cases because of circular dependencies. + * Now that semantic is finished, get a definitive result, and error if it is not the same. + */ + if (cldec.isabstract != ABSfwdref) // if evaluated it before completion + { + const isabstractsave = cldec.isabstract; + cldec.isabstract = ABSfwdref; + cldec.isAbstract(); // recalculate + if (cldec.isabstract != isabstractsave) + { + cldec.error("cannot infer `abstract` attribute due to circular dependencies"); + } + } + if (cldec.type.ty == Tclass && (cast(TypeClass)cldec.type).sym != cldec) { // https://issues.dlang.org/show_bug.cgi?id=17492 diff --git a/src/ddmd/expressionsem.d b/src/ddmd/expressionsem.d index 44800448b454..eb2a4325c096 100644 --- a/src/ddmd/expressionsem.d +++ b/src/ddmd/expressionsem.d @@ -1087,6 +1087,7 @@ extern (C++) final class ExpressionSemanticVisitor : Visitor result = new ErrorExp(); return; } + if (cd.isAbstract()) { exp.error("cannot create instance of abstract class %s", cd.toChars()); diff --git a/src/ddmd/func.d b/src/ddmd/func.d index 83ab1d10f095..c6b6c0783d18 100644 --- a/src/ddmd/func.d +++ b/src/ddmd/func.d @@ -1108,6 +1108,29 @@ extern (C++) class FuncDeclaration : Declaration return true; // functions can be overloaded } + /*********************************** + * Override so it can work even if semantic() hasn't yet + * been run. + */ + override final bool isAbstract() + { + if (storage_class & STCabstract) + return true; + if (semanticRun >= PASSsemanticdone) + return false; + + if (_scope) + { + if (_scope.stc & STCabstract) + return true; + parent = _scope.parent; + Dsymbol parent = toParent(); + if (parent.isInterfaceDeclaration()) + return true; + } + return false; + } + /********************************** * Decide if attributes for this function can be inferred from examining * the function body. diff --git a/test/compilable/test16273.d b/test/compilable/test16273.d new file mode 100644 index 000000000000..c079f0a552b7 --- /dev/null +++ b/test/compilable/test16273.d @@ -0,0 +1,22 @@ +// https://issues.dlang.org/show_bug.cgi?id=16273 + +class A() +{ + alias MyD = D!(); +} + +class B +{ + void f() {} + alias MyA = A!(); +} + +class C : B +{ + override void f() {} +} + +class D() : A!() +{ + void g() { new C; } +}