[frontend/ffi] no-implicit-NativeType

[dcharkes]

Allowing users to write code with Pointer<NativeType> allows them to write generic code. But letting the type system infer NativeType as type argument automatically could give the impression that the user has written type safe code while he didn't. This is a case very similar to inferring List<dynamic>.

Some users might want to write code with List<dynamic> or Pointer<NativeType>, and some don't. This is why List<dynamic> can be opted in to be flagged with no-implicit-dynamic. We should have a lint for Pointer<NativeType> that does the same.

Should the lint be opt-out or opt-in? Maybe we should make it opt-out, so that people that want to write generic code can opt-out. And new users don't shoot themselves in the foot accidentally.

The lint should catch the following failing code:

typedef DoCallbackNative = Void Function(Pointer);
typedef DoCallback = void Function(Pointer);

final DoCallback doCallback =
  testLibrary.lookupFunction<DoCallbackNative, DoCallback>("callCallback");

Pointer<Int8> intPointer = allocate();
doCallback(intPointer); // segfaults

This code is inferred by Dart as:

typedef DoCallbackNative = Void Function(Pointer<NativeType>);
typedef DoCallback = void Function(Pointer<NativeType>);

With the lint enabled a user would have to write Pointer<NativeType> explicitly if he does want to have a generic pointer in the DoCallback signature.

/cc @stereotype441 @sjindel-google

[stereotype441]

@pq FYI

[pq]

Thanks for looping me in @stereotype441!

This certainly feels like a valuable analysis.

Should the lint be opt-out or opt-in?

As it stands, all lints are opt-in since they each need to be explicitly enabled. The closest we could get would be a lint that's in a common set of default rules, such as pedantic (@davidmorgan FYI for that.)

If we wanted true opt-out semantics we'd have to make this a hint or warning. And maybe we should? Is there any reason anyone would not want this feedback?

/cc @bwilkerson

[sjindel-google]

I feel that we are going backwards here. The FFI should be a library with built-in support, not a new language design. We need to be removing the broken "type-checking" logic rather that making it more complicated.

I see no reason why this program shouldn't work:

typedef DoCallbackNative = Void Function(Pointer);
typedef DoCallback = void Function(Pointer);

final DoCallback doCallback =
  testLibrary.lookupFunction<DoCallbackNative, DoCallback>("callCallback");

Pointer<Int8> intPointer = allocate();
doCallback(intPointer); // segfaults

[dcharkes]

I see no reason why this program shouldn't work

It should work, just like List inferred as List<dynamic> should work. However, some people might want to select a lint to disallow that in their code base. Idem Pointer being inferred as Pointer<NativeType>. This should not be an error, it should be an opt-in or opt-out lint.

[dcharkes]

As it stands, all lints are opt-out since they each need to be explicitly enabled.

@pq You mean opt-in?

[pq]

@dcharkes: agh, yes. opt-in! (time for more ☕️ !)

(EDITED ABOVE)

[bwilkerson]

Yes, all lints are opt-in.

But as you pointed out, this is similar to no-implicit-dynamic, which is a general analysis option rather than a lint. I haven't gone searching for it, but I seem to recall that @srawlins and @leafpetersen have both made proposals for a coherent set of such options, and I'd be surprised if this case wasn't covered by those.

[srawlins]

@dcharkes rather than a special case for NativeType, what about analysis that warns on any instantiate-to-bounds?

This is effectively strict-raw-types: https://github.com/dart-lang/language/blob/master/resources/type-system/strict-raw-types.md