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

[cpp] Fix typing for abstracts #9542

Merged

Conversation

vonagam
Copy link
Member

@vonagam vonagam commented Jun 6, 2020

When you use an abstract with an underlying type of typedef or an other abstract, typing skips resolving the underlying type itself jumping straight to resolving most concrete type instead.

Example why this problematic - if the underlying type is
Star<T>, Reference<T> or Struct<T>, the runtime C++ type end up being just T not T*, T& or cpp::Struct<T>:

@:structAccess
extern class Foo {
  @:native('new Foo')
  static function create(): cpp.Star<Foo>;
}

abstract Bar(cpp.Star<Foo>) from cpp.Star<Foo> {}

class Main5 {
  public static function main() {
    var bar: Bar = Foo.create();
  }
}

Cpp output:

::Foo bar = (*((new Foo())));

Don't know how to properly check resulted C++ runtime type in a test.

@Simn
Copy link
Member

Simn commented Jun 6, 2020

Could you add the failing example as a test?

@Simn Simn added the platform-cpp Everything related to CPP / C++ label Jun 6, 2020
@vonagam
Copy link
Member Author

vonagam commented Jun 6, 2020

Yeah, but it will pass with or without this change, since in both cases cpp output will be valid, just not what you want, so it will be not really a test...

@vonagam
Copy link
Member Author

vonagam commented Jun 6, 2020

Had an idea for the minimal test, found other potential bug:

var int: Int = 0;
var foo: cpp.Reference<Int> = int;
int += 1;
eq(foo, 1);

With -D analyzer-optimize produces cpp (cpp.Reference has @:semantics(reference), don't know if it is intended to help with such cases) (without analyzer-optimize it works ok):

this->eq(0,1,...)

@vonagam vonagam force-pushed the fix-cpp-abstract-over-typedefs branch from df9f417 to 52153af Compare June 6, 2020 16:16
@vonagam vonagam force-pushed the fix-cpp-abstract-over-typedefs branch from 52153af to badb0a2 Compare June 6, 2020 16:17
@vonagam
Copy link
Member Author

vonagam commented Jun 6, 2020

Added proper test.

@skial skial mentioned this pull request Jun 8, 2020
1 task
@RealyUniqueName RealyUniqueName added this to the Bugs milestone Jun 10, 2020
@Simn Simn merged commit 0b656d5 into HaxeFoundation:development Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-cpp Everything related to CPP / C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants