From da20db5e0e840a9ba385c924ca5cf7d365a1ab57 Mon Sep 17 00:00:00 2001 From: Walter Bright Date: Fri, 2 Feb 2024 07:26:59 -0800 Subject: [PATCH] mv findVtblIndex() and overrideInterface() to funcsem.d (#16127) --- compiler/src/dmd/cppmanglewin.d | 3 +- compiler/src/dmd/declaration.h | 1 - compiler/src/dmd/expressionsem.d | 2 +- compiler/src/dmd/frontend.h | 1 - compiler/src/dmd/func.d | 148 ----------------------------- compiler/src/dmd/funcsem.d | 156 ++++++++++++++++++++++++++++++- 6 files changed, 156 insertions(+), 155 deletions(-) diff --git a/compiler/src/dmd/cppmanglewin.d b/compiler/src/dmd/cppmanglewin.d index 4b96ac6add69..cf5f64c20a9a 100644 --- a/compiler/src/dmd/cppmanglewin.d +++ b/compiler/src/dmd/cppmanglewin.d @@ -25,6 +25,7 @@ import dmd.dtemplate; import dmd.errors; import dmd.expression; import dmd.func; +import dmd.funcsem; import dmd.globals; import dmd.id; import dmd.identifier; @@ -513,7 +514,7 @@ extern(D): // Pivate methods always non-virtual in D and it should be mangled as non-virtual in C++ //printf("%s: isVirtualMethod = %d, isVirtual = %d, vtblIndex = %d, interfaceVirtual = %p\n", //d.toChars(), d.isVirtualMethod(), d.isVirtual(), cast(int)d.vtblIndex, d.interfaceVirtual); - if ((d.isVirtual() && (d.vtblIndex != -1 || d.interfaceVirtual || d.overrideInterface())) || (d.isDtorDeclaration() && d.parent.isClassDeclaration() && !d.isFinal())) + if ((d.isVirtual() && (d.vtblIndex != -1 || d.interfaceVirtual || overrideInterface(d))) || (d.isDtorDeclaration() && d.parent.isClassDeclaration() && !d.isFinal())) { mangleVisibility(buf, d, "EMU"); } diff --git a/compiler/src/dmd/declaration.h b/compiler/src/dmd/declaration.h index e4efbbcc91be..afbb9975cc9c 100644 --- a/compiler/src/dmd/declaration.h +++ b/compiler/src/dmd/declaration.h @@ -703,7 +703,6 @@ class FuncDeclaration : public Declaration Expressions *fdensureParams(Expressions *fdep); bool equals(const RootObject * const o) const override final; - int findVtblIndex(Dsymbols *vtbl, int dim); bool overloadInsert(Dsymbol *s) override; bool inUnittest(); static MATCH leastAsSpecialized(FuncDeclaration *f, FuncDeclaration *g, Identifiers *names); diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index b472fd0e2ec8..d7377dbdea89 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -6290,7 +6290,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor auto ad2 = b.sym; ue.e1 = ue.e1.castTo(sc, ad2.type.addMod(ue.e1.type.mod)); ue.e1 = ue.e1.expressionSemantic(sc); - auto vi = exp.f.findVtblIndex(&ad2.vtbl, cast(int)ad2.vtbl.length); + auto vi = findVtblIndex(exp.f, ad2.vtbl[]); assert(vi >= 0); exp.f = ad2.vtbl[vi].isFuncDeclaration(); assert(exp.f); diff --git a/compiler/src/dmd/frontend.h b/compiler/src/dmd/frontend.h index 400f8e026cab..dd4faf576465 100644 --- a/compiler/src/dmd/frontend.h +++ b/compiler/src/dmd/frontend.h @@ -3793,7 +3793,6 @@ class FuncDeclaration : public Declaration Array* fdensureParams(Array* param); FuncDeclaration* syntaxCopy(Dsymbol* s) override; bool equals(const RootObject* const o) const final override; - int32_t findVtblIndex(Array* vtbl, int32_t dim); bool overloadInsert(Dsymbol* s) override; bool inUnittest(); static MATCH leastAsSpecialized(FuncDeclaration* f, FuncDeclaration* g, Array* names); diff --git a/compiler/src/dmd/func.d b/compiler/src/dmd/func.d index 371ca060a05d..ddf21a2079f4 100644 --- a/compiler/src/dmd/func.d +++ b/compiler/src/dmd/func.d @@ -531,154 +531,6 @@ extern (C++) class FuncDeclaration : Declaration return result; } - /************************************************* - * Find index of function in vtbl[0..length] that - * this function overrides. - * Prefer an exact match to a covariant one. - * Params: - * vtbl = vtable to use - * dim = maximal vtable dimension - * Returns: - * -1 didn't find one - * -2 can't determine because of forward references - */ - final int findVtblIndex(Dsymbols* vtbl, int dim) - { - //printf("findVtblIndex() %s\n", toChars()); - import dmd.typesem : covariant; - - FuncDeclaration mismatch = null; - StorageClass mismatchstc = 0; - int mismatchvi = -1; - int exactvi = -1; - int bestvi = -1; - for (int vi = 0; vi < dim; vi++) - { - FuncDeclaration fdv = (*vtbl)[vi].isFuncDeclaration(); - if (fdv && fdv.ident == ident) - { - if (type.equals(fdv.type)) // if exact match - { - if (fdv.parent.isClassDeclaration()) - { - if (fdv.isFuture()) - { - bestvi = vi; - continue; // keep looking - } - return vi; // no need to look further - } - - if (exactvi >= 0) - { - .error(loc, "%s `%s` cannot determine overridden function", kind, toPrettyChars); - return exactvi; - } - exactvi = vi; - bestvi = vi; - continue; - } - - StorageClass stc = 0; - const cov = type.covariant(fdv.type, &stc); - //printf("\tbaseclass cov = %d\n", cov); - final switch (cov) - { - case Covariant.distinct: - // types are distinct - break; - - case Covariant.yes: - bestvi = vi; // covariant, but not identical - break; - // keep looking for an exact match - - case Covariant.no: - mismatchvi = vi; - mismatchstc = stc; - mismatch = fdv; // overrides, but is not covariant - break; - // keep looking for an exact match - - case Covariant.fwdref: - return -2; // forward references - } - } - } - if (_linkage == LINK.cpp && bestvi != -1) - { - StorageClass stc = 0; - FuncDeclaration fdv = (*vtbl)[bestvi].isFuncDeclaration(); - assert(fdv && fdv.ident == ident); - if (type.covariant(fdv.type, &stc, /*cppCovariant=*/true) == Covariant.no) - { - /* https://issues.dlang.org/show_bug.cgi?id=22351 - * Under D rules, `type` and `fdv.type` are covariant, but under C++ rules, they are not. - * For now, continue to allow D covariant rules to apply when `override` has been used, - * but issue a deprecation warning that this behaviour will change in the future. - * Otherwise, follow the C++ covariant rules, which will create a new vtable entry. - */ - if (isOverride()) - { - /* @@@DEPRECATED_2.110@@@ - * After deprecation period has ended, be sure to remove this entire `LINK.cpp` branch, - * but also the `cppCovariant` parameter from Type.covariant, and update the function - * so that both `LINK.cpp` covariant conditions within are always checked. - */ - .deprecation(loc, "overriding `extern(C++)` function `%s%s` with `const` qualified function `%s%s%s` is deprecated", - fdv.toPrettyChars(), fdv.type.toTypeFunction().parameterList.parametersTypeToChars(), - toPrettyChars(), type.toTypeFunction().parameterList.parametersTypeToChars(), type.modToChars()); - - const char* where = type.isNaked() ? "parameters" : "type"; - deprecationSupplemental(loc, "Either remove `override`, or adjust the `const` qualifiers of the " - ~ "overriding function %s", where); - } - else - { - // Treat as if Covariant.no - mismatchvi = bestvi; - mismatchstc = stc; - mismatch = fdv; - bestvi = -1; - } - } - } - if (bestvi == -1 && mismatch) - { - //type.print(); - //mismatch.type.print(); - //printf("%s %s\n", type.deco, mismatch.type.deco); - //printf("stc = %llx\n", mismatchstc); - if (mismatchstc) - { - // Fix it by modifying the type to add the storage classes - type = type.addStorageClass(mismatchstc); - bestvi = mismatchvi; - } - } - return bestvi; - } - - /********************************* - * If function a function in a base class, - * return that base class. - * Returns: - * base class if overriding, null if not - */ - extern (D) final BaseClass* overrideInterface() - { - for (ClassDeclaration cd = toParent2().isClassDeclaration(); cd; cd = cd.baseClass) - { - foreach (b; cd.interfaces) - { - auto v = findVtblIndex(&b.sym.vtbl, cast(int)b.sym.vtbl.length); - if (v >= 0) - return b; - } - } - return null; - } - /**************************************************** * Overload this FuncDeclaration with the new one f. * Return true if successful; i.e. no conflict. diff --git a/compiler/src/dmd/funcsem.d b/compiler/src/dmd/funcsem.d index fed61be7c92c..9e706ee80d9d 100644 --- a/compiler/src/dmd/funcsem.d +++ b/compiler/src/dmd/funcsem.d @@ -502,7 +502,7 @@ void funcDeclarationSemantic(Scope* sc, FuncDeclaration funcdecl) /* Find index of existing function in base class's vtbl[] to override * (the index will be the same as in cd's current vtbl[]) */ - int vi = cd.baseClass ? funcdecl.findVtblIndex(&cd.baseClass.vtbl, cast(int)cd.baseClass.vtbl.length) : -1; + int vi = cd.baseClass ? findVtblIndex(funcdecl, cd.baseClass.vtbl[]) : -1; bool doesoverride = false; switch (vi) @@ -539,7 +539,7 @@ void funcDeclarationSemantic(Scope* sc, FuncDeclaration funcdecl) /* if overriding an interface function, then this is not * introducing and don't put it in the class vtbl[] */ - funcdecl.interfaceVirtual = funcdecl.overrideInterface(); + funcdecl.interfaceVirtual = overrideInterface(funcdecl); if (funcdecl.interfaceVirtual) { //printf("\tinterface function %s\n", toChars()); @@ -764,7 +764,7 @@ void funcDeclarationSemantic(Scope* sc, FuncDeclaration funcdecl) { foreach (b; bcd.interfaces) { - vi = funcdecl.findVtblIndex(&b.sym.vtbl, cast(int)b.sym.vtbl.length); + vi = findVtblIndex(funcdecl, b.sym.vtbl[]); switch (vi) { case -1: @@ -1217,3 +1217,153 @@ extern (D) bool checkForwardRef(FuncDeclaration fd, const ref Loc loc) } return false; } + +/************************************************* + * Find index of function in vtbl[0..length] that + * this function overrides. + * Prefer an exact match to a covariant one. + * Params: + * fd = function + * vtbl = vtable to use + * Returns: + * -1 didn't find one + * -2 can't determine because of forward references + */ +int findVtblIndex(FuncDeclaration fd, Dsymbol[] vtbl) +{ + //printf("findVtblIndex() %s\n", toChars()); + import dmd.typesem : covariant; + + FuncDeclaration mismatch = null; + StorageClass mismatchstc = 0; + int mismatchvi = -1; + int exactvi = -1; + int bestvi = -1; + for (int vi = 0; vi < cast(int)vtbl.length; vi++) + { + FuncDeclaration fdv = vtbl[vi].isFuncDeclaration(); + if (fdv && fdv.ident == fd.ident) + { + if (fd.type.equals(fdv.type)) // if exact match + { + if (fdv.parent.isClassDeclaration()) + { + if (fdv.isFuture()) + { + bestvi = vi; + continue; // keep looking + } + return vi; // no need to look further + } + + if (exactvi >= 0) + { + .error(fd.loc, "%s `%s` cannot determine overridden function", fd.kind, fd.toPrettyChars); + return exactvi; + } + exactvi = vi; + bestvi = vi; + continue; + } + + StorageClass stc = 0; + const cov = fd.type.covariant(fdv.type, &stc); + //printf("\tbaseclass cov = %d\n", cov); + final switch (cov) + { + case Covariant.distinct: + // types are distinct + break; + + case Covariant.yes: + bestvi = vi; // covariant, but not identical + break; + // keep looking for an exact match + + case Covariant.no: + mismatchvi = vi; + mismatchstc = stc; + mismatch = fdv; // overrides, but is not covariant + break; + // keep looking for an exact match + + case Covariant.fwdref: + return -2; // forward references + } + } + } + if (fd._linkage == LINK.cpp && bestvi != -1) + { + StorageClass stc = 0; + FuncDeclaration fdv = vtbl[bestvi].isFuncDeclaration(); + assert(fdv && fdv.ident == fd.ident); + if (fd.type.covariant(fdv.type, &stc, /*cppCovariant=*/true) == Covariant.no) + { + /* https://issues.dlang.org/show_bug.cgi?id=22351 + * Under D rules, `type` and `fdv.type` are covariant, but under C++ rules, they are not. + * For now, continue to allow D covariant rules to apply when `override` has been used, + * but issue a deprecation warning that this behaviour will change in the future. + * Otherwise, follow the C++ covariant rules, which will create a new vtable entry. + */ + if (fd.isOverride()) + { + /* @@@DEPRECATED_2.110@@@ + * After deprecation period has ended, be sure to remove this entire `LINK.cpp` branch, + * but also the `cppCovariant` parameter from Type.covariant, and update the function + * so that both `LINK.cpp` covariant conditions within are always checked. + */ + .deprecation(fd.loc, "overriding `extern(C++)` function `%s%s` with `const` qualified function `%s%s%s` is deprecated", + fdv.toPrettyChars(), fdv.type.toTypeFunction().parameterList.parametersTypeToChars(), + fd.toPrettyChars(), fd.type.toTypeFunction().parameterList.parametersTypeToChars(), fd.type.modToChars()); + + const char* where = fd.type.isNaked() ? "parameters" : "type"; + deprecationSupplemental(fd.loc, "Either remove `override`, or adjust the `const` qualifiers of the " + ~ "overriding function %s", where); + } + else + { + // Treat as if Covariant.no + mismatchvi = bestvi; + mismatchstc = stc; + mismatch = fdv; + bestvi = -1; + } + } + } + if (bestvi == -1 && mismatch) + { + //type.print(); + //mismatch.type.print(); + //printf("%s %s\n", type.deco, mismatch.type.deco); + //printf("stc = %llx\n", mismatchstc); + if (mismatchstc) + { + // Fix it by modifying the type to add the storage classes + fd.type = fd.type.addStorageClass(mismatchstc); + bestvi = mismatchvi; + } + } + return bestvi; +} + +/********************************* + * If function is a function in a base class, + * return that base class. + * Params: + * fd = function + * Returns: + * base class if overriding, null if not + */ +BaseClass* overrideInterface(FuncDeclaration fd) +{ + for (ClassDeclaration cd = fd.toParent2().isClassDeclaration(); cd; cd = cd.baseClass) + { + foreach (b; cd.interfaces) + { + auto v = findVtblIndex(fd, b.sym.vtbl[]); + if (v >= 0) + return b; + } + } + return null; +}