-
Notifications
You must be signed in to change notification settings - Fork 133
[ffigen] Cpp unique ptr ownership #3513
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
96134f0
f4ef610
2100b8e
1b7cac4
f1fc38d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,6 +194,17 @@ class $name implements $ffiPrefix.Finalizable { | |
| _activeFinalizerFn = null; | ||
| } | ||
|
|
||
| /// Detaches the finalizer and invalidates this object, returning the | ||
| /// underlying C++ pointer. | ||
| /// | ||
| /// Throws a [StateError] if the object has already been disposed, or if | ||
| /// this object does not own the pointer. | ||
| $ptrVoid detachPointer() { | ||
| final rawPtr = _ptr; | ||
| releaseOwnership(); | ||
| _ptr = $ffiPrefix.nullptr; | ||
| return rawPtr; | ||
| } | ||
| '''); | ||
|
|
||
| for (final ctor in constructors) { | ||
|
|
@@ -202,22 +213,42 @@ class $name implements $ffiPrefix.Finalizable { | |
|
|
||
| final dartParams = dartParamList(ctor.parameters); | ||
|
|
||
| final ownedParams = ctor.parameters | ||
| .where((p) => p.type is CppUniquePtrType) | ||
| .toList(); | ||
|
|
||
| final localVars = LocalVariables(ctor.localScope); | ||
|
|
||
| final rawPtrVars = <String, String>{}; | ||
| for (final p in ownedParams) { | ||
| rawPtrVars[p.name] = '_raw_${p.name}'; | ||
| } | ||
|
|
||
| final callArgs = ctor.parameters | ||
| .map( | ||
| (p) => p.type.convertDartTypeToFfiDartType( | ||
| .map((p) { | ||
| if (rawPtrVars.containsKey(p.name)) { | ||
| return rawPtrVars[p.name]!; | ||
| } | ||
| return p.type.convertDartTypeToFfiDartType( | ||
| ctx, | ||
| p.name, | ||
| objCRetain: false, | ||
| objCAutorelease: false, | ||
| localVariables: localVars, | ||
| ), | ||
| ) | ||
| ); | ||
| }) | ||
| .join(', '); | ||
|
|
||
| final ownershipChecks = StringBuffer(); | ||
| for (final p in ownedParams) { | ||
| final raw = rawPtrVars[p.name]!; | ||
| ownershipChecks.write(' final $raw = ${p.name}.detachPointer();\n'); | ||
| } | ||
|
|
||
| s.write(''' | ||
| factory $name($dartParams) { | ||
| ${localVars.generateDeclarations()} | ||
| ${ownershipChecks.toString().trimLeft()} | ||
| return $name.fromPointer($privateName($callArgs), takeOwnership: true); | ||
| } | ||
| '''); | ||
|
|
@@ -228,18 +259,31 @@ class $name implements $ffiPrefix.Finalizable { | |
| final dartReturn = method.returnType.getDartType(ctx); | ||
| final dartParams = dartParamList(method.parameters); | ||
|
|
||
| final ownedParams = method.parameters | ||
| .where((p) => p.type is CppUniquePtrType) | ||
| .toList(); | ||
|
|
||
| final localVars = LocalVariables(method.localScope); | ||
|
|
||
| final rawPtrVars = <String, String>{}; | ||
| for (final p in ownedParams) { | ||
| rawPtrVars[p.name] = '_raw_${p.name}'; | ||
| } | ||
|
|
||
| final callArgs = [ | ||
| if (!method.isStatic) '_ptr', | ||
| ...method.parameters.map( | ||
| (p) => p.type.convertDartTypeToFfiDartType( | ||
| ...method.parameters.map((p) { | ||
| if (rawPtrVars.containsKey(p.name)) { | ||
| return rawPtrVars[p.name]!; | ||
| } | ||
| return p.type.convertDartTypeToFfiDartType( | ||
| ctx, | ||
| p.name, | ||
| objCRetain: false, | ||
| objCAutorelease: false, | ||
| localVariables: localVars, | ||
| ), | ||
| ), | ||
| ); | ||
| }), | ||
| ].join(', '); | ||
| final decls = localVars.generateDeclarations(); | ||
|
|
||
|
|
@@ -249,21 +293,38 @@ class $name implements $ffiPrefix.Finalizable { | |
| objCRetain: false, | ||
| ); | ||
|
|
||
| // Build the ownership-transfer preamble for owned parameters. | ||
| final ownershipChecks = StringBuffer(); | ||
| for (final p in ownedParams) { | ||
| final raw = rawPtrVars[p.name]!; | ||
| ownershipChecks.write(' final $raw = ${p.name}.detachPointer();\n'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| } | ||
|
|
||
| final hasReturn = method.returnType != voidType; | ||
|
|
||
| if (method.isStatic) { | ||
| final callLine = hasReturn | ||
| ? 'return $returnExpr;' | ||
| : '$glue($callArgs);'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can |
||
| s.write('''\ | ||
| static $dartReturn ${method.originalName}($dartParams) { | ||
| $decls | ||
| return $returnExpr; | ||
| ${ownershipChecks.toString().trimLeft()} | ||
| $callLine | ||
| } | ||
| '''); | ||
| } else { | ||
| final callLine = hasReturn | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
| ? 'return $returnExpr;' | ||
| : '$glue($callArgs);'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can |
||
| s.write('''\ | ||
| $dartReturn ${method.originalName}($dartParams) { | ||
| if (_ptr == $ffiPrefix.nullptr) { | ||
| throw StateError('This object has already been disposed.'); | ||
| } | ||
| $decls | ||
| return $returnExpr; | ||
| ${ownershipChecks.toString().trimLeft()} | ||
| $callLine | ||
| } | ||
| '''); | ||
| } | ||
|
|
@@ -365,12 +426,13 @@ FFIGEN_EXPORT void ${name}_delete($originalName* self) { | |
| final methodBindings = methods | ||
| .map((method) { | ||
| final symbol = method.name.name; | ||
| final callArgs = method.parameters.map((p) => p.name).join(', '); | ||
|
|
||
| final String returnTypeString; | ||
| final String params; | ||
| final String body; | ||
|
|
||
| final callArgs = method.parameters.map(_cppCallArg).join(', '); | ||
|
|
||
| if (method.isConstructor) { | ||
| returnTypeString = '$originalName*'; | ||
| params = method.parameters.map(paramDecl).join(', '); | ||
|
|
@@ -396,7 +458,11 @@ FFIGEN_EXPORT void ${name}_delete($originalName* self) { | |
| selfType = originalName; | ||
| } | ||
| params = ['$selfType* self', ...otherParams].join(', '); | ||
| body = '${returnPrefix}self->${method.originalName}($callArgs);'; | ||
| final methodName = method.originalName; | ||
| final suffix = method.returnType is CppUniquePtrType | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine for now, but you're probably going to run into more places where you need to reuse this return type conversion (and the I filed a bug to clean this up later, don't worry about it at the moment: #3523 |
||
| ? '.release()' | ||
| : ''; | ||
| body = '${returnPrefix}self->$methodName($callArgs)$suffix;'; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -436,3 +502,12 @@ FFIGEN_EXPORT $returnTypeString $symbol($params) { | |
| visitor.visit(ffiImport); | ||
| } | ||
| } | ||
|
|
||
| String _cppCallArg(Parameter p) { | ||
| final type = p.type; | ||
| if (type is CppUniquePtrType) { | ||
| final className = type.cppClass.originalName; | ||
| return 'std::unique_ptr<$className>(${p.name})'; | ||
| } | ||
| return p.name; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,8 +255,6 @@ class ObjCObjectPointerWithProtocols extends ObjCObjectPointer { | |
| } | ||
|
|
||
| /// A pointer to a C++ class wrapper object. | ||
| /// Returned pointers are always unowned by default. The developer must call | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restore this deleted comment? |
||
| /// `retainOwnership()` explicitly if ownership has been transferred. | ||
| class CppClassPointerType extends PointerType { | ||
| final CppClass cppClass; | ||
|
|
||
|
|
@@ -314,3 +312,22 @@ class CppClassPointerType extends PointerType { | |
| visitor.visit(ffiImport); | ||
| } | ||
| } | ||
|
|
||
| /// A type representing `std::unique_ptr<T>` ownership transfer. | ||
| class CppUniquePtrType extends CppClassPointerType { | ||
| CppUniquePtrType(super.cppClass); | ||
|
|
||
| @override | ||
| String convertFfiDartTypeToDartType( | ||
| Context context, | ||
| String value, { | ||
| required bool objCRetain, | ||
| String? objCEnclosingClass, | ||
| }) => '${cppClass.name}.fromPointer($value, takeOwnership: true)'; | ||
|
|
||
| @override | ||
| String toString() => 'unique_ptr<${cppClass.name}>'; | ||
|
|
||
| @override | ||
| String cacheKey() => 'unique_ptr<${cppClass.cacheKey()}>'; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,27 @@ Type getCodeGenType( | |
| return getCodeGenType(context, clang.clang_Type_getNamedType(cxtype)); | ||
| } | ||
|
|
||
| // Handle C++ templates like std::unique_ptr. | ||
| if (context.config.cpp?.classes != null) { | ||
| final numTemplateArgs = clang.clang_Type_getNumTemplateArguments(cxtype); | ||
| if (numTemplateArgs >= 1) { | ||
| final declCursor = clang.clang_getTypeDeclaration(cxtype); | ||
| final usr = clang.clang_getCursorUSR(declCursor).toStringAndDispose(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have extension methods for this. You can use |
||
| final isStdUniquePtr = usr.contains('std@') && usr.contains('unique_ptr'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you show me what one of these USRs looks like? Doing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. libclang produces USRs in the following format:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Can you do
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
| if (isStdUniquePtr) { | ||
| final spelling = clang | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| .clang_getTypeSpelling(cxtype) | ||
| .toStringAndDispose(); | ||
| return _extractUniquePtrType( | ||
| context, | ||
| cxtype, | ||
| numTemplateArgs, | ||
| spelling, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // These basic Objective C types skip the cache, and are conditional on the | ||
| // language flag. | ||
| if (context.config.objectiveC != null) { | ||
|
|
@@ -287,6 +308,41 @@ Type? _extractfromRecord( | |
| return UnimplementedType('${cxtype.kindSpelling()} not implemented'); | ||
| } | ||
|
|
||
| Type _extractUniquePtrType( | ||
| Context context, | ||
| clang_types.CXType cxtype, | ||
| int numTemplateArgs, | ||
| String spelling, | ||
| ) { | ||
| final logger = context.logger; | ||
|
|
||
| if (numTemplateArgs != 1) { | ||
| logger.warning( | ||
| 'std::unique_ptr with a custom deleter is not supported ' | ||
| '($numTemplateArgs template args in "$spelling"). Skipping.', | ||
| ); | ||
| return UnimplementedType('unique_ptr with custom deleter not supported'); | ||
| } | ||
|
|
||
| final innerCXType = clang.clang_Type_getTemplateArgumentAsType(cxtype, 0); | ||
| final innerType = getCodeGenType(context, innerCXType); | ||
|
|
||
| if (innerType is CppClass) { | ||
| logger.fine( | ||
| ' unique_ptr<${innerType.originalName}> is an owned CppUniquePtrType', | ||
| ); | ||
| return CppUniquePtrType(innerType); | ||
| } | ||
|
|
||
| logger.warning( | ||
| 'std::unique_ptr inner type is not a known C++ class ' | ||
| '(got ${innerType.runtimeType} from "$spelling"). Skipping.', | ||
| ); | ||
| return UnimplementedType( | ||
| 'unique_ptr inner type is not a supported C++ class', | ||
| ); | ||
| } | ||
|
|
||
| // Used for function pointer arguments. | ||
| Type _extractFromFunctionProto( | ||
| Context context, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating these sorts of variables is exactly what
LocalVariablesis for. It would be better to use that. But actually, now that you've got thedetachPointermethod, you can probably inline it, and get rid of these local variables.