Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a C++ derived class that defines an operator hides automatic base class conversion #65

Closed
timotheecour opened this issue Jan 9, 2018 · 4 comments

Comments

@timotheecour
Copy link
Collaborator

@Syniurge : a C++ derived class that defines an operator hides automatic base class conversion

test.h:

struct A{ int x;};

struct B : public A{
  // comment to remove error
  operator float() const;
};

main.d:

modmap (C++) "test.h";
import (C++) A;
import (C++) B;

void main(){
  B b;
  int bx=b.x;
}

compiling produces this error:
Error: template instance opCast!(A) does not match template declaration opCast(type : float)

The error goes away if we remove operator float() const;

It works if we add operator A*() {return this;} in B and change the D code to:
int bx=(cast(A*)b).x; but this is obviously not practical.

@timotheecour
Copy link
Collaborator Author

timotheecour commented Jan 10, 2018

workaround: skip generating opCast!T from C++ operator T()
obviously this is not a good workaround since it'll skip generating some C++ functions

Calypso/ddmd/cpp/calypso.cpp:

Identifier *getIdentifierOrNull(const clang::NamedDecl *D, SpecValue *spec, bool useCanonicalType)

    else if (auto Conv = dyn_cast<clang::CXXConversionDecl>(D))
    {
       // return immediately here, which skips generating opCast!T from C++ `operator T()`
        return nullptr;
       // ...
}

NOTE: I had intially tried in Calypso/ddmd/expression.d CastExp:semantic(Scope* sc) commenting out the block if (Expression e = op_overload(sc)){ but it had worse side effects as it'll just skip existing opCast templates

A better workaround would be to generate opCast(BaseClass*) if at least one clang::CXXConversionDecl is encountered (and generated) but I don't know how this could be done simply in current design. Any help appreciated.

@timotheecour
Copy link
Collaborator Author

timotheecour commented Jan 10, 2018

still a better workaround (requiring no change in Calypso/ddmd/cpp/calypso.cpp, hence not suppressing generation of opCast's):

Calypso/ddmd/expression.d in class CastExp:

if (!to.equals(e1.type) && mod == cast(ubyte)~0)
        {
            uint errors = global.startGagging();
            Expression e = op_overload(sc);
            if(global.endGagging(errors))
                e=null;
            if(e)
              return e.implicitCastTo(sc, to);
        }
  • what could be downsides?
  • how could we require this change to only apply for C++ classes?

@timotheecour
Copy link
Collaborator Author

improved version that only gags when both e1.type and to are C++ classes from langPlugin:

                    bool is_cpp_class(Type t){
                        // NOTE: this doesn't work:
                        // if(auto cd = t.isClassHandle()) return cd.isCPPclass();

                        Type tv = t.baseElemOf();
                        if (!tv.isAggregateValue()) return false;
                        if(!tv.getAggregateSym().langPlugin()) return false;
                        return true;
                    }
                    if(!is_cpp_class(e1.type)) goto Handle_default;
                    if(!is_cpp_class(to)) goto Handle_default;

                    uint errors = global.startGagging();
                    Expression e = op_overload(sc);
                    if(global.endGagging(errors)){
                        e=null;
                    }
                    if(e){
                        return e.implicitCastTo(sc, to);
                    }
                    goto Handle_other; // TODO: needed?
                }

                Handle_default:
                    if (Expression e = op_overload(sc)){
                        return e.implicitCastTo(sc, to);
                    }
                Handle_other:

Syniurge added a commit that referenced this issue Mar 12, 2018
…ble overloading by opCast when a CastExp is a downcast to a C++ base.

In D code an explicit cast gets overloaded if there's a member named opCast even if opCast!BaseClass isn't valid.
Syniurge added a commit that referenced this issue Mar 12, 2018
…ble overloading by opCast when a CastExp is a downcast to a C++ base.

In D code an explicit cast gets overloaded if there's a member named opCast even if opCast!BaseClass isn't valid.
Syniurge added a commit that referenced this issue Mar 13, 2018
…) Disable overloading by opCast when a CastExp is a downcast to a C++ base."

This reverts commit ea850ce.
Syniurge added a commit that referenced this issue Mar 13, 2018
…(Issue #65) Disable overloading by opCast when a CastExp is a downcast to a C++ base.""

This reverts commit e7fb20a.
Syniurge added a commit that referenced this issue Mar 14, 2018
…t to a C++ base.

In D code an explicit cast gets overloaded if there's a member named opCast even if opCast!BaseClass isn't valid.

Thanks to @timotheecour for the original fix in PR #95.
@Syniurge
Copy link
Owner

Fixed by c3573b9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants