[ffi] Support non-trivial const expressions in exceptional returns

Bug: #54417
Change-Id: Iddafe896f7c070b7ef4205347b4b454febbd164a
Fixes: #54417
TEST=tests/ffi/exceptional_return_const_test.dart for the new behavior
TEST=tests/ffi/vmspecific_static_checks_test.dart tests that the existing error messages still work.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397220
Commit-Queue: Liam Appelbe <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: liamappelbe

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -486,6 +486,8 @@ class LibraryAnalyzer {
       FfiVerifier(
         _typeSystem,
         diagnosticReporter,
+        _libraryElement,
+        _declaredVariables,
         strictCasts: _analysisOptions.strictCasts,
       ),
     );

pkg/analyzer/lib/src/generated/ffi_verifier.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/dart/analysis/declared_variables.dart';
 import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
@@ -12,6 +13,8 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/ast/extensions.dart';
+import 'package:analyzer/src/dart/constant/evaluation.dart';
+import 'package:analyzer/src/dart/constant/value.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
@@ -106,15 +109,24 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
   /// Subclass of `Struct` or `Union` we are currently visiting, or `null`.
   ClassDeclaration? compound;
 
+  final LibraryElementImpl _currentLibrary;
+
+  final ConstantEvaluationEngine _evaluationEngine;
+
   /// The `Void` type from `dart:ffi`, or `null` if unresolved.
   InterfaceTypeImpl? ffiVoidType;
 
   /// Initialize a newly created verifier.
   FfiVerifier(
     this.typeSystem,
-    this._diagnosticReporter, {
+    this._diagnosticReporter,
+    this._currentLibrary,
+    DeclaredVariables declaredVariables, {
     required this.strictCasts,
-  });
+  }) : _evaluationEngine = ConstantEvaluationEngine(
+         declaredVariables: declaredVariables,
+         configuration: ConstantEvaluationConfiguration(),
+       );
 
   @override
   void visitClassDeclaration(covariant ClassDeclarationImpl node) {
@@ -744,21 +756,17 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
   }
 
   bool _isConst(Expression expr) {
-    if (expr is Literal) {
-      return true;
-    }
-    if (expr is Identifier) {
-      var element = expr.element;
-      if (element is VariableElement && element.isConst) {
-        return true;
-      }
-      if (element is PropertyAccessorElement) {
-        if (element.variable.isConst) {
-          return true;
-        }
-      }
-    }
-    return false;
+    var subDiagnosticReporter = DiagnosticReporter(
+      RecordingDiagnosticListener(),
+      _diagnosticReporter.source,
+    );
+    var constantVisitor = ConstantVisitor(
+      _evaluationEngine,
+      _currentLibrary,
+      subDiagnosticReporter,
+    );
+    var result = constantVisitor.evaluateConstant(expr);
+    return result is! InvalidConstant;
   }
 
   bool _isLeaf(NodeList<Expression>? args) {

pkg/dart2wasm/lib/target.dart

@@ -340,24 +340,34 @@ class WasmTarget extends Target {
       logger?.call("Skipped ffi transformation");
     } else {
       wasmFfiNativeAddressTrans.transformLibraries(
-          component,
-          coreTypes,
-          hierarchy,
-          transitiveImportingDartFfi,
-          diagnosticReporter,
-          referenceFromIndex);
+        component,
+        coreTypes,
+        hierarchy,
+        transitiveImportingDartFfi,
+        diagnosticReporter,
+        referenceFromIndex,
+      );
       wasmFfiNativeTrans.transformLibraries(component, coreTypes, hierarchy,
           transitiveImportingDartFfi, diagnosticReporter, referenceFromIndex);
       transformFfiDefinitions.transformLibraries(
-          component,
-          coreTypes,
-          hierarchy,
-          transitiveImportingDartFfi,
-          diagnosticReporter,
-          referenceFromIndex,
-          changedStructureNotifier);
-      transformFfiUseSites.transformLibraries(component, coreTypes, hierarchy,
-          transitiveImportingDartFfi, diagnosticReporter, referenceFromIndex);
+        component,
+        coreTypes,
+        hierarchy,
+        transitiveImportingDartFfi,
+        diagnosticReporter,
+        referenceFromIndex,
+        changedStructureNotifier,
+      );
+      transformFfiUseSites.transformLibraries(
+        this,
+        component,
+        coreTypes,
+        hierarchy,
+        transitiveImportingDartFfi,
+        diagnosticReporter,
+        referenceFromIndex,
+        environmentDefines,
+      );
       logger?.call("Transformed ffi annotations");
     }
 

pkg/vm/lib/modular/target/vm.dart

@@ -217,12 +217,14 @@ class VmTarget extends Target {
       // VM pragma attached to valid targets in the native transformer. Hence,
       // it can only run after `@Native` targets have been transformed.
       transformFfiUseSites.transformLibraries(
+        this,
         component,
         coreTypes,
         hierarchy,
         transitiveImportingDartFfi,
         diagnosticReporter,
         referenceFromIndex,
+        environmentDefines,
       );
       logger?.call("Transformed ffi use sites");
     }

pkg/vm/lib/modular/transformations/ffi/use_sites.dart

@@ -4,8 +4,11 @@
 
 // This imports 'codes/cfe_codes.dart' instead of 'api_prototype/codes.dart' to
 // avoid cyclic dependency between `package:vm/modular` and `package:front_end`.
+import 'package:front_end/src/api_prototype/constant_evaluator.dart'
+    show ConstantEvaluator, SimpleErrorReporter;
 import 'package:front_end/src/codes/cfe_codes.dart'
     show
+        LocatedMessage,
         codeFfiAddressOfMustBeNative,
         codeFfiAddressPosition,
         codeFfiAddressReceiver,
@@ -28,7 +31,7 @@ import 'package:kernel/kernel.dart';
 import 'package:kernel/library_index.dart' show LibraryIndex;
 import 'package:kernel/names.dart';
 import 'package:kernel/reference_from_index.dart';
-import 'package:kernel/target/targets.dart' show DiagnosticReporter;
+import 'package:kernel/target/targets.dart' show DiagnosticReporter, Target;
 import 'package:kernel/type_algebra.dart'
     show FunctionTypeInstantiator, Substitution;
 import 'package:kernel/type_environment.dart';
@@ -40,18 +43,27 @@ import 'finalizable.dart';
 import 'native.dart' as native;
 import 'native_type_cfe.dart';
 
+class _SilentErrorReporter extends SimpleErrorReporter {
+  const _SilentErrorReporter();
+
+  @override
+  void report(LocatedMessage message, [List<LocatedMessage>? context]) {}
+}
+
 /// Checks and replaces calls to dart:ffi compound fields and methods.
 ///
 /// To reliably lower calls to methods like `sizeOf` and `Native.addressOf`,
 /// this requires that the [definitions] and the [native] transformer already
 /// ran on the libraries to transform.
 void transformLibraries(
+  Target target,
   Component component,
   CoreTypes coreTypes,
   ClassHierarchy hierarchy,
   List<Library> libraries,
   DiagnosticReporter diagnosticReporter,
   ReferenceFromIndex? referenceFromIndex,
+  Map<String, String>? environmentDefines,
 ) {
   final index = LibraryIndex(component, [
     "dart:ffi",
@@ -76,6 +88,15 @@ void transformLibraries(
     hierarchy,
     diagnosticReporter,
     referenceFromIndex,
+    ConstantEvaluator(
+      target.dartLibrarySupport,
+      target.constantsBackend,
+      component,
+      environmentDefines,
+      TypeEnvironment(coreTypes, hierarchy),
+      const _SilentErrorReporter(),
+      errorOnUnevaluatedConstant: true,
+    ),
   );
   libraries.forEach(transformer.visitLibrary);
 }
@@ -88,12 +109,14 @@ void transformLibraries(
 /// respectively. This means one cannot do `visitX() { super.visitX() as X }`.
 class _FfiUseSiteTransformer2 extends FfiTransformer
     with _FfiUseSiteTransformer, FinalizableTransformer {
+  final ConstantEvaluator _constantEvaluator;
   _FfiUseSiteTransformer2(
     LibraryIndex index,
     CoreTypes coreTypes,
     ClassHierarchy hierarchy,
     DiagnosticReporter diagnosticReporter,
     ReferenceFromIndex? referenceFromIndex,
+    this._constantEvaluator,
   ) : super(
         index,
         coreTypes,
@@ -115,6 +138,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
   StaticTypeContext? get staticTypeContext;
 
   bool _inFfiTearoff = false;
+  ConstantEvaluator get _constantEvaluator;
 
   bool get isFfiLibrary => currentLibrary == ffiLibrary;
 
@@ -1210,9 +1234,11 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
 
       // The exceptional return value must be a constant so that it can be
       // referenced by precompiled trampoline's object pool.
-      if (exceptionalReturn is! BasicLiteral &&
-          !(exceptionalReturn is ConstantExpression &&
-              exceptionalReturn.constant is PrimitiveConstant)) {
+      final exceptionalReturnValue = _constantEvaluator.evaluate(
+        staticTypeContext!,
+        exceptionalReturn,
+      );
+      if (exceptionalReturnValue is UnevaluatedConstant) {
         diagnosticReporter.report(
           codeFfiExpectedConstant,
           node.fileOffset,
@@ -1223,9 +1249,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
       }
 
       // Moreover it may not be null.
-      if (exceptionalReturn is NullLiteral ||
-          (exceptionalReturn is ConstantExpression &&
-              exceptionalReturn.constant is NullConstant)) {
+      if (exceptionalReturnValue is NullConstant) {
         diagnosticReporter.report(
           codeFfiExceptionalReturnNull,
           node.fileOffset,
@@ -1251,6 +1275,11 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
         );
         return node;
       }
+
+      exceptionalReturn = ConstantExpression(
+        exceptionalReturnValue,
+        returnType,
+      );
     }
 
     final compoundClasses =

pkg/vm/testcases/transformations/ffi/native_callable.dart.aot.expect

@@ -98,7 +98,7 @@ static method negateInt(core::int i) → core::int
 
 [@vm.inferred-return-type.metadata=dart.core::Null? (value: null)]
 static method testNativeCallableIsolateLocalInt() → void {
-  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>([@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeCallbackFunction<(ffi::Int32) → ffi::Int>(#C3, 123), null, true));
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>([@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeCallbackFunction<(ffi::Int32) → ffi::Int>(#C3, #C4), null, true));
   core::print([@vm.direct-call.metadata=dart.ffi::_NativeCallableBase.nativeFunction] [@vm.inferred-type.metadata=dart.ffi::Pointer] callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>});
   [@vm.direct-call.metadata=dart.ffi::_NativeCallableBase.close] [@vm.inferred-type.metadata=? (skip check)] callback.{ffi::NativeCallable::close}(){() → void};
 }
@@ -108,12 +108,13 @@ static method testNativeCallableIsolateLocalIntClosure() → void {
   [@vm.inferred-type.metadata=dart.core::_Smi (value: 123)] core::int j = 123;
   function closure(core::int i) → core::int
     return [@vm.direct-call.metadata=dart.core::_IntegerImplementation.+] [@vm.inferred-type.metadata=int (skip check)] i.{core::num::+}(j){(core::num) → core::int};
-  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>([@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeIsolateLocalCallbackFunction<(ffi::Int32) → ffi::Int>(123), closure, true));
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>([@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeIsolateLocalCallbackFunction<(ffi::Int32) → ffi::Int>(#C4), closure, true));
   core::print([@vm.direct-call.metadata=dart.ffi::_NativeCallableBase.nativeFunction] [@vm.inferred-type.metadata=dart.ffi::Pointer] callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>});
   [@vm.direct-call.metadata=dart.ffi::_NativeCallableBase.close] [@vm.inferred-type.metadata=? (skip check)] callback.{ffi::NativeCallable::close}(){() → void};
 }
 constants  {
   #C1 = static-tearoff self::printInt
   #C2 = static-tearoff self::intToPointer
   #C3 = static-tearoff self::negateInt
+  #C4 = 123
 }

pkg/vm/testcases/transformations/ffi/native_callable.dart.expect

@@ -72,20 +72,21 @@ static method testNativeCallableIsolateLocalPointerClosure() → void {
 static method negateInt(core::int i) → core::int
   return i.{core::int::unary-}(){() → core::int};
 static method testNativeCallableIsolateLocalInt() → void {
-  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>(ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeCallbackFunction<(ffi::Int32) → ffi::Int>(#C3, 123), null, true));
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>(ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeCallbackFunction<(ffi::Int32) → ffi::Int>(#C3, #C4), null, true));
   core::print(callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>});
   callback.{ffi::NativeCallable::close}(){() → void};
 }
 static method testNativeCallableIsolateLocalIntClosure() → void {
   core::int j = 123;
   function closure(core::int i) → core::int
     return i.{core::num::+}(j){(core::num) → core::int};
-  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>(ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeIsolateLocalCallbackFunction<(ffi::Int32) → ffi::Int>(123), closure, true));
+  final ffi::NativeCallable<(ffi::Int32) → ffi::Int> callback = new ffi::_NativeCallableIsolateLocal::•<(ffi::Int32) → ffi::Int>(ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>(ffi::_nativeIsolateLocalCallbackFunction<(ffi::Int32) → ffi::Int>(#C4), closure, true));
   core::print(callback.{ffi::NativeCallable::nativeFunction}{ffi::Pointer<ffi::NativeFunction<(ffi::Int32) → ffi::Int>>});
   callback.{ffi::NativeCallable::close}(){() → void};
 }
 constants  {
   #C1 = static-tearoff self::printInt
   #C2 = static-tearoff self::intToPointer
   #C3 = static-tearoff self::negateInt
+  #C4 = 123
 }

pkg/vm/testcases/transformations/type_flow/transformer/ffi_struct_constructors.dart.expect

@@ -70,7 +70,7 @@ final class Struct12 extends ffi::Struct {
 }
 
 [@vm.inferred-type.metadata=dart.ffi::Pointer]
-static final field ffi::Pointer<ffi::NativeFunction<(self::Struct3) → ffi::Int32>> _#ffiCallback0 = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(self::Struct3) → ffi::Int32>>(ffi::_nativeCallbackFunction<(self::Struct3) → ffi::Int32>(#C17, 0), null, false);
+static final field ffi::Pointer<ffi::NativeFunction<(self::Struct3) → ffi::Int32>> _#ffiCallback0 = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<(self::Struct3) → ffi::Int32>>(ffi::_nativeCallbackFunction<(self::Struct3) → ffi::Int32>(#C17, #C9), null, false);
 
 [@vm.inferred-type.metadata=dart.ffi::Pointer]
 static final field ffi::Pointer<ffi::NativeFunction<() → self::Struct7>> _#ffiCallback1 = [@vm.inferred-type.metadata=dart.ffi::Pointer] ffi::_createNativeCallableIsolateLocal<ffi::NativeFunction<() → self::Struct7>>(ffi::_nativeCallbackFunction<() → self::Struct7>(#C18, null), null, false);