Skip to content

small beautifications #7

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

Closed
wants to merge 12 commits into from
Closed

Conversation

pannous
Copy link

@pannous pannous commented Jan 5, 2018

I fully understand if you don't want these in the master branch.

aliases int, double, float, real, byte for i32, i64, ...

added 'and' operator for &&
added 'or' operator for ||

src/program.ts Outdated
["i64", Type.i64],
["double", Type.i64],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my understanding, a double always is a 64-bit float. isn't it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, sorry for sloppy friday night commit ;)
Playing into the hands of AssemblyScript/prototype#104

src/program.ts Outdated
@@ -121,9 +121,12 @@ export class Program extends DiagnosticEmitter {

this.types = new Map([
["i8", Type.i8],
["byte", Type.i8],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, aliases like these were in the prototype compiler, along the lines of:

i8 = sbyte
u8 = byte
i16 = short
u16 = ushort
i32 = int
u32 = uint
f32 = float
f64 = double

That is, until this issue emerged: https://github.com/AssemblyScript/prototype/issues/104

@@ -187,6 +189,7 @@ export namespace Token {
export function fromKeyword(text: string): Token {
switch (text) {
case "abstract": return Token.ABSTRACT;
case "and": return Token.AND;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still TypeScript then?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, and it breaks a test

export function and(loLeft: u32 ...
and,

in i64-polyfill.ts(16,3)

Let me know if you ever consider creating a *new* language, I love the clarity of your code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The breakage is probably due to missing entries in Token.isAlsoIdentifier. Though, adding non-TS-compatible logical operators doesn't feel right to me.

Pannous added 3 commits January 5, 2018 23:57
      ["short", Type.i8],
      ["byte", Type.u8],

      ["double", Type.f64],
      ["int8", Type.i8],
      ["int16", Type.i16],
      ["int32", Type.i32],
      ["int64", Type.i64],
@dcodeIO
Copy link
Member

dcodeIO commented Jan 7, 2018

I have thought about the proposed aliases a bit and am currently favoring to keep is simple without any aliases, at least on the compiler-side, for two reasons: 1) It keeps lookup maps small and efficient internally and 2) most devs will use the default types so AS types look familiar accross different projects, which might be a nice-to-have in open source projects so foreign sources are easier to understand.

Aside from that, it's also easy to add aliases on a per-project level if there's a need to:

type int = i32;
type uint = u32;
type float = f32;
type double = f64;
...

@dcodeIO
Copy link
Member

dcodeIO commented Jan 27, 2018

Closing this issue for now as it hasn't received any replies recently. Feel free to reopen it if necessary!

@dcodeIO dcodeIO closed this Jan 27, 2018
willemneal pushed a commit to willemneal/assemblyscript that referenced this pull request May 31, 2019
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.

2 participants