scalar methods do not respect the signed types that Core Data forces

[altimac]

Question

when settings min value to 0 for an attribute of type "Integer 64", mogenerator generates value accessors with uint64_t primitive types.

Expected Behavior

Core Data internally uses int64_t (remember: "Integer 64") so mogenerator value accessors should respect that and expose int64_t types.

Actual Behavior

mogenerator probably look at the constraints (min value set 0 let it think it's an unsigned value), and decides to expose uint64_t primitive types in value accessors. This is wrong as internally Core Data will use int64_t anyway (you can see that using their new code generation)

Additional Information

iOS 10 / Xcode 8

[seanm]

It so happens I'm upgrading from old mogenerator 1.27 which didn't have this new behaviour of sometimes using unsigned and I just noticed this issue you created.

I think your analysis may be correct.

I have an entity with a 32 bit integer attribute and minimum value of 0. As a test, I set it to 2147483648 (INT32_MAX + 1) programmatically calling either:

[entity setFoo:@2147483648];
[entity setFooValue:2147483648]; // this setter takes uint32_t

I figured Core Data was like NSNumber and didn't encode the signedness, but it seems it does.

Printing the object shows it interpreted as signed:

(lldb) po e
<MyEntity: 0x13e9b4fb0> (entity: MyEntity; id: 0x9d632880e5a1fd76 <x-coredata://CEF77CE5-2E41-4E17-9650-899746168AA6/MyEntity/p1>; data: {
    foo = "-2147483648";
})

but more damningly, saving my store fails with a validation error that "'foo' is too small". Recall the attribute's minimum is 0.

So it seems mogenerator creating unsigned accessors is just a lie, and should indeed not be done.

Unless I'm missing something... @rentzsch thoughts?

[seanm]

Looks like this unsigned stuff was added in: #214 there doesn't seem to have been any deep discussion at the time...

[rentzsch]

I guess I never pushed the edge cases here: testing the condition of setting the signed bit.

I agree internally Core Data seems to just use int32. The simple solution would be to give up on this "feature" and just expose its machinery directly as an int32 regardless of the attribute's stated minimum value. Sean you may want to fork the template and do that in your custom version.

However my ideal would be to keep the unsigned type generation since it helps document the assumptions of the model's usage directly in the code. I'd beef up the generated setter and have it fail if the argument was negative or beyond INT32_MAX.

[seanm]

The simple solution would be to give up on this "feature" and just expose its machinery directly as an int32 regardless of the attribute's stated minimum value.

That's my inclination as well. I could make such a PR.

Sean you may want to fork the template and do that in your custom version.

Can that be done only changing the template? The template seems to use scalarAccessorMethodName which in the code is implemented like:

        case NSInteger32AttributeType:
            if (isUnsigned) {
                return @"unsignedIntValue";
            }
            return @"intValue";
            break;

Seems that code would need to change. For now we've hacked isUnsigned to always return NO.

However my ideal would be to keep the unsigned type generation since it helps document the assumptions of the model's usage directly in the code. I'd beef up the generated setter and have it fail if the argument was negative or beyond INT32_MAX.

Wouldn't setter code having such checks just be a duplication of the existing min/max mechanism in the xcdatamodel? Users that want uint32_t should probably be setting attribute max values to be INT32_MAX (or less) in their xcdatamodels. (In fact, it occurred to us in all this that we should be setting ever lower maximums generally, to prevent craziness like pasting megabytes of text into little text fields.)