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

Typescript support #162

Closed
wants to merge 20 commits into from
Closed

Typescript support #162

wants to merge 20 commits into from

Conversation

DutChen18
Copy link
Member

@DutChen18 DutChen18 commented Sep 29, 2023

This PR implements the automatic generation of typescript declaration files for jsexported code.
Also moved the MODULE_TYPE enum from a private inner type of CheerpWriter into Utility.h because it is also required by CheerpDTSWriter.

New flags:

  • -cheerp-make-dts
  • -cheerp-dts-output-file=<file>

Example:

class [[cheerp::jsexport]] Rect {
  int width, height;
public:
  Rect(int width, int height) : width(width), height(height) {}

  int getWidth() const { return width; }
  int getHeight() const { return height; }
};

[[cheerp::jsexport]]
int area(const Rect& rect) {
  return rect.getWidth() * rect.getHeight();
}
/opt/cheerp/bin/clang++ rect.cpp -o rect.js -cheerp-make-dts -target cheerp-wasm -cheerp-pretty-code -cheerp-make-module=es6
export interface Rect {
        getWidth(): number;
        getHeight(): number;
        delete(): void;
}
type __Options = { buffer: ArrayBuffer } | { absPath: String | URL };
export default function(options?: __Options): Promise<{
        Rect: {
                new(_0: number, _1: number): Rect;
        };
        area(rect: Rect): number;
}>;

@yuri91
Copy link
Member

yuri91 commented Sep 29, 2023

I am glancing at the PR, and I wonder about a possible improvement in the output:

  Rect: {
  new(_0: number, _1: number): Rect;
  };

Here there are placeholder names for the arguments, but in theory we could get the llvm argument names if present, and use those.
For example, when compiling to asmjs the constructor (with pretty code) we get:
function __ZN4RectC1Eii(Lthis,Lwidth,Lheight){

Here there is an extra 'L' in front that stands for "local", but for typescript we could just sanitize keywords and invalid characters.
This stuff for the writer is in NameGenerator.cpp

@yuri91
Copy link
Member

yuri91 commented Sep 29, 2023

another thing, to help with the review:

Try to refactor the commits so that commits that fix something you wrote earlier in the PR disappear (Example: 9f245fc).

And put at the beginning commits that don't add functional changes. Example: the last commit 787fceb could be at the beginning, so all the other commits can just use those types from the start.

@yuri91
Copy link
Member

yuri91 commented Sep 29, 2023

Actually about my first comment: it might be that those nice names are only available in debug builds of llvm. In that case, it woudl be useless.

@alexp-sssup ?

@DutChen18
Copy link
Member Author

another thing, to help with the review:

Try to refactor the commits so that commits that fix something you wrote earlier in the PR disappear (Example: 9f245fc).

And put at the beginning commits that don't add functional changes. Example: the last commit 787fceb could be at the beginning, so all the other commits can just use those types from the start.

Thanks for the tips, I will definitely keep this in mind going forward.

I am glancing at the PR, and I wonder about a possible improvement in the output:

  Rect: {
  new(_0: number, _1: number): Rect;
  };

Here there are placeholder names for the arguments, but in theory we could get the llvm argument names if present, and use those. For example, when compiling to asmjs the constructor (with pretty code) we get: function __ZN4RectC1Eii(Lthis,Lwidth,Lheight){

Here there is an extra 'L' in front that stands for "local", but for typescript we could just sanitize keywords and invalid characters. This stuff for the writer is in NameGenerator.cpp

Yes, you're right, I didn't notice them in the output before because I was compiling to wasm. The reason we don't have true argument names for the constructor is because that declaration is not for the actual constructor, it's for the "new helper", which looks like this:

function __ZN4Rect3newEii(Larg0,Larg1){
        var tmp0=null;
        tmp0={i0:0,i1:0};
        __ZN4RectC1Eii(tmp0,Larg0,Larg1);
        return tmp0;
}

I do believe the true names are available for the actual constructor even in release builds, and special logic could be implemented to use those names instead when generating the declaration for the new helper.

@DutChen18
Copy link
Member Author

Also, good note on sanitizing variable names, it had not occurred to me that those might conflict with typescript keywords.

@DutChen18
Copy link
Member Author

While looking into how to obtain the constructor from a Function* to a new helper, I stumbled across a much simpler and (imo) more elegant solution. When defining the new helper, we should simply use the same parameter names as the constructor, instead of leaving them blank.

The new helper in javascript now looks like this:

function __ZN4Rect3newEii(Lwidth,Lheight){
        var tmp0=null;
        tmp0={i0:0,i1:0};
        __ZN4RectC1Eii(tmp0,Lwidth,Lheight);
        return tmp0;
}

And its typescript declaration looks like this:

        Rect: {
                new(width: number, height: number): Rect;
        };

@alexp-sssup
Copy link
Member

Actually about my first comment: it might be that those nice names are only available in debug builds of llvm. In that case, it woudl be useless.

Unsure, there is certainly no guarantee that they have any specific value

@alexp-sssup
Copy link
Member

Merged

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

Successfully merging this pull request may close these issues.

3 participants