Merge pull request #926 from swiftlang/jan_svoboda/better-isResultValid-API

Provide more powerful `isResultValid` API

Author: jansvoboda11

examples/c-api/buildsystem/main.c

@@ -100,6 +100,7 @@ fancy_tool_create_command(void *context, const llb_data_t* name) {
   delegate.execute_command_detached = NULL;
   delegate.cancel_detached_command = NULL;
   delegate.is_result_valid = NULL;
+  delegate.is_result_valid_with_fallback = NULL;
   return llb_buildsystem_external_command_create(name, delegate);
 }
 

products/libllbuild/BuildSystem-C-API.cpp

@@ -1146,6 +1146,34 @@ class CAPIExternalCommand : public ExternalCommand {
   }
 
   bool isResultValid(BuildSystem& system, const BuildValue& value) override {
+    if (cAPIDelegate.is_result_valid_with_fallback) {
+      struct FallbackContext {
+        BuildSystem *buildSystem;
+        
+        static bool handle(void* c_ctx,
+                           llb_buildsystem_command_t* c_command,
+                           llb_build_value* c_value) {
+          auto* command = (CAPIExternalCommand*)c_command;
+          auto* value = (CAPIBuildValue*)c_value;
+          
+          auto* ctx = static_cast<FallbackContext*>(c_ctx);
+          BuildSystem& system = *ctx->buildSystem;
+          delete ctx;
+          
+          return command->ExternalCommand::isResultValid(system, value->getInternalBuildValue());
+        }
+      };
+      
+      auto value_p = (llb_build_value *)new CAPIBuildValue(BuildValue(value));
+      return cAPIDelegate.is_result_valid_with_fallback(
+        cAPIDelegate.context,
+        (llb_buildsystem_command_t*)this,
+        value_p,
+        new FallbackContext{&system},
+        FallbackContext::handle
+      );
+    }
+    
     if (cAPIDelegate.is_result_valid) {
       auto value_p = (llb_build_value *)new CAPIBuildValue(BuildValue(value));
       return cAPIDelegate.is_result_valid(

products/libllbuild/include/llbuild/buildsystem.h

@@ -776,6 +776,12 @@ typedef struct llb_buildsystem_external_command_delegate_t_ {
                           llb_buildsystem_command_t* command,
                           const llb_build_value* value);
 
+  bool (*is_result_valid_with_fallback)(void* context,
+                                        llb_buildsystem_command_t* command,
+                                        const llb_build_value* value,
+                                        void* fallback_ctx,
+                                        bool (*fallback)(void* fallback_ctx, llb_buildsystem_command_t*, llb_build_value*));
+  
   /// Callback a client may use to tear down data structures associated with the context
   /// pointer.
   void (*destroy_context)(void* context);

products/libllbuild/include/llbuild/llbuild-defines.h

@@ -84,6 +84,8 @@
 /// compile for multiple versions of the API.
 ///
 /// Version History:
+/// 19: Added isResultValid API with a fallback to CAPIExternalCommand.
+///
 /// 18: Added support for configuring outputs of dynamic tasks via the C API.
 ///
 /// 17: Added `llb_buildsystem_dependency_data_format_makefile_ignoring_subsequent_outputs`
@@ -121,6 +123,6 @@
 /// 1: Added `environment` parameter to llb_buildsystem_invocation_t.
 ///
 /// 0: Pre-history
-#define LLBUILD_C_API_VERSION 18
+#define LLBUILD_C_API_VERSION 19
 
 #endif

products/llbuildSwift/BuildSystemBindings.swift

@@ -220,6 +220,9 @@ private final class ToolWrapper {
                 _delegate.is_result_valid = {
                     return BuildSystem.toCommandWrapper($0!).isResultValid($1!, $2!)
                 }
+                _delegate.is_result_valid_with_fallback = {
+                    return BuildSystem.toCommandWrapper($0!).isResultValid($1!, $2!, $3!, $4!)
+                }
             } else {
                 _delegate.execute_command_ex = nil
                 _delegate.is_result_valid = nil
@@ -362,6 +365,20 @@ public protocol ProducesCustomBuildValue: AnyObject {
     /// - command: A handle to the executing command.
     /// - buildValue: The most recently computed build value.
     func isResultValid(_ command: Command, _ buildValue: BuildValue) -> Bool
+    
+    /// Called to check if the current result for this command remains valid.
+    ///
+    /// - command: A handle to the executing command.
+    /// - buildValue: The most recently computed build value.
+    /// - fallback: The default implementation: llbuild::buildsystem::ExternalCommand::isResultValid().
+    func isResultValid(_ command: Command, _ buildValue: BuildValue, _ fallback: @escaping (Command, BuildValue) -> Bool) -> Bool
+}
+
+public extension ProducesCustomBuildValue {
+    func isResultValid(_ command: Command, _ buildValue: BuildValue, _ fallback: @escaping (Command, BuildValue) -> Bool) -> Bool {
+        // This should default to the fallback, but instead we defer to ProducesCustomBuildValue.isResultValid(_:_:) for backward compatibility.
+        return isResultValid(command, buildValue)
+    }
 }
 
 // Extension to provide a default implementation of execute(_ Command, _ commandInterface) and
@@ -545,6 +562,23 @@ private final class CommandWrapper {
 
         return (command as! ProducesCustomBuildValue).isResultValid(_command, buildValue)
     }
+    
+    func isResultValid(
+        _: OpaquePointer,
+        _ value: OpaquePointer,
+        _ fallbackCtx: UnsafeMutableRawPointer?,
+        _ fallback: @escaping (UnsafeMutableRawPointer?, OpaquePointer?, OpaquePointer?) -> Bool
+    ) -> Bool {
+        guard let buildValue = BuildValue.construct(from: value) else {
+            fatalError("Could not decode incoming build value.")
+        }
+        
+        func fallbackWrapper(_ command: Command, value: BuildValue) -> Bool {
+            return fallback(fallbackCtx, command.handle, BuildValue.clone(value))
+        }
+        
+        return (command as! ProducesCustomBuildValue).isResultValid(_command, buildValue, fallbackWrapper)
+    }
 }
 
 /// Encapsulates a diagnostic as reported by the build system.

products/llbuildSwift/BuildValue.swift

@@ -173,6 +173,10 @@ public class BuildValue: CustomStringConvertible, Equatable, Hashable {
 
         return llb_build_value_clone(value.internalBuildValue)
     }
+    
+    static func clone(_ value: BuildValue) -> OpaquePointer {
+        return llb_build_value_clone(value.internalBuildValue)
+    }
 
     /// The kind of the build value.
     /// The kind also defines the subclass, so kind == .invalid means the instance should be of type Invalid

unittests/CAPI/BuildSystem-C-API.cpp

@@ -151,6 +151,7 @@ depinfo_tester_tool_create_command(void *context, const llb_data_t* name) {
   delegate.execute_command_detached = NULL;
   delegate.cancel_detached_command = NULL;
   delegate.is_result_valid = NULL;
+  delegate.is_result_valid_with_fallback = NULL;
   return llb_buildsystem_external_command_create(name, delegate);
 }