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

Type error with negative floats as static expressions #4054

Closed
Gekctek opened this issue Jun 20, 2023 · 6 comments · Fixed by #4073
Closed

Type error with negative floats as static expressions #4054

Gekctek opened this issue Jun 20, 2023 · 6 comments · Fixed by #4073

Comments

@Gekctek
Copy link
Contributor

Gekctek commented Jun 20, 2023

Moc version: 0.8.8

I get the following error when trying to declare a static value with a negative float in it: type error [M0014], non-static expression in library or module

module {
	public let t : Float = -147.1; // Error
	public let t2 : Float = 147.1; // No Error
	public let t3 : Int = -147; // No Error
}

Is this intended?
I am trying to generate a large static piece of data but all the negative floats wont work

@nomeata
Copy link
Collaborator

nomeata commented Jun 20, 2023

Thanks for the good bugreport.

There is some tension whether -123 should be thought of as a single literal, or an unary operator applied to a literal. But the inconsistency you point out is certainly not great, and the use in modules is a strong reason to treat them as one IMHO. Could be a simple fix, didn't look at the code so far.

@rossberg
Copy link
Contributor

Regardless of that distinction, I think -(147.1) should be considered static as well. In general, I see no particular reason not to consider all arithmetic operators static whose semantics is free of traps.

I'm confused about the difference between Float and Int, though. I'm pretty sure signs used to be treated uniformly.

@nomeata
Copy link
Collaborator

nomeata commented Jun 20, 2023

Ah, the not fully resolved question of whether our notion of “static” for modules means “is a (manifest) value” or “can be evaluated (purely) to a value”. But I guess we don't need to fully answer that, and just continue to find a pragmatic middle ground - we already allow projections, certain (soon more) negations, without claiming to be fully principled here.

@nomeata
Copy link
Collaborator

nomeata commented Jun 20, 2023

So the staticness checker regards any use of UnE as nonstatic. So why does it work for Int? Probably because of this code in the parser;

  | op=unop e=exp_un(ob)
    { match op, e.it with
      | (PosOp | NegOp), LitE {contents = PreLit (s, Type.Nat)} ->
        let signed = match op with NegOp -> "-" ^ s | _ -> "+" ^ s in
        LitE(ref (PreLit (signed, Type.Int))) @? at $sloc
      | _ -> UnE(ref Type.Pre, op, e) @? at $sloc
    }

Is there a reason to do that only for Type.Nat, and not also Type.Float, @rossberg?

@rossberg
Copy link
Contributor

Okay, it seems that was a hack introduced in #505.

Treating signs as part of literals is nasty. For one, there shouldn't be a difference between -1 and - 1, and I shouldn't be forced to write the former for obscure reasons.

Second, it inevitably screws up operator precedence: for example, making-2**2 result in +4 is surprising and inconsistent with math -- that said, we screwed up precedence of unary operators anyway, and - 2**2 is +4 as well. Other languages are no better, but we set out to avoid such pitfalls.

@nomeata
Copy link
Collaborator

nomeata commented Jun 21, 2023

Yeah, it’s a mess either way. But doing this to just Type.Nat but not Type.Float is probably not making things better?

crusso added a commit that referenced this issue Jun 23, 2023
mergify bot pushed a commit that referenced this issue Jun 23, 2023
Fix for issue #4054 
Adopting the same hack we use for integral literals.
@mergify mergify bot closed this as completed in #4073 Jun 26, 2023
mergify bot pushed a commit that referenced this issue Jun 26, 2023
Add changelog entry for #4063.

Fixed #4054
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 a pull request may close this issue.

3 participants