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 of model property/enum member when it is defined as constant number #3314

Open
haolingdong-msft opened this issue May 10, 2024 · 8 comments
Assignees

Comments

@haolingdong-msft
Copy link
Member

TSP:

model Operation {
 age: 50;
}

@fixed
enum Priority {
  High: 100,
  Low: 0,
}

For model property/enum member defined as constant number, in above example, Operation.age, Property.High, Priority.Low, how to interprete the type of the property/enum member? Should it be int32 or int64?

If it is interpreted as int32, in the future, if another int64 enum value added, or the model property is set to a int64 number, it would be breaking changes for Java and .Net client code, as the type changed from int to long.
If it is interpreted as int64, then we should make sure service understand this and can handle when user passes long value.

To solve this:

  • One solution is to have an explicit way to define the type of the constant number as int32 or int64. Avoid the definition like age: 50 as this is ambiguous.
  • Another solution is to an agreement from TypeSepc level on how to interprete the constant number defined as above, so that service and client can have correct behavior.

Would like to hear your thoughts and suggestions, Thanks.

@pshao25
Copy link
Contributor

pshao25 commented May 10, 2024

This is the C# syntax.

  1. enum with long value
enum LongValueEnum: long
{
    A = 100000000000000000,
    B = 2,
}
  1. no extends means int
enum IntValueEnum
{
    A = 2,
    B = 8,
}

@archerzz
Copy link
Member

archerzz commented May 10, 2024

Like other languages, literal values without specific identifier (e.g. 50l, 50.1f) should be of default types.

I think there are two strategies:

  • Use the supertypes, per TypeSpec spec. For example, 50 is of type integer. That leaves it to each emitter to decide the exact type.

For some emit targets, BigInt or BigDecimal might be an analogous type satisfying the TypeSpec types integer and decimal respectively. For other targets where the language, serialization format, or protocol does not support an analogous type, emitters may decide on a policy for emitting the numeric supertypes.

  • Or we need to explicitly clarify the default types, which means we need to update the spec about literal types.

Personally, I perfer the 2nd strategy, because TypeSpec is used to represent both client side and server side. It would be weird if the data types for the server side depends on the languages implementing the service.

@haolingdong-msft
Copy link
Member Author

haolingdong-msft commented May 11, 2024

Unlike .Net, Java client's current behavior is to use long(int64) if constant number value is defined like 50. But we will soon upgrade to use TCGC models, which interprete it as int32.
Anyway, I think it is better to have an aglinment from TypeSpec level, instead of letting client to guess.

@tadelesh
Copy link
Member

from typespec compiler, we just get an number value. in tcgc, we will type them with int32 or float32.

@markcowl markcowl self-assigned this May 13, 2024
@markcowl
Copy link
Contributor

@tadelesh We think the right thing here is to have a default behavior in the emitter and add an emitter decorator that allows specifying a preferred client representation type, for emitters where that makes sense.

So, I think we want to move this issue into the Azure repo, or manage is as an emitter-specific issue for client emitters here.

@markcowl markcowl added needs-info Mark an issue that needs reply from the author or it will be closed automatically and removed needs-area labels May 13, 2024
@archerzz
Copy link
Member

Can we document the decision for the types of literal value? Then I think we can close this issue.

Copy link
Contributor

Hi @haolingdong-msft. Since there hasn't been recent engagement, we're going to close this out. Please feel free to reopen if you have any further questions or concerns.

@haolingdong-msft
Copy link
Member Author

Heard from @srnagar that it will be handled from typespec level and typespec team will document the solution down. @markcowl could you please document down the agreement between TypeSpec team and SDK archs?

@markcowl markcowl reopened this May 30, 2024
@markcowl markcowl removed the needs-info Mark an issue that needs reply from the author or it will be closed automatically label May 30, 2024
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

No branches or pull requests

5 participants