From f2f2a5b06cbfd6a3bfbce1c14d243d885cd701c3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 13 Jul 2024 05:44:32 -0700 Subject: [PATCH] jsinspector: Fix various build warnings (#45423) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45423 I noticed while fixing some CI issues that `jsinspector` emits a bunch of warnings, making finding errors (especially in CI logs) awkward. Also fix up a couple of stale comments from earlier designs of `NetworkIOAgent`. Changelog: [Internal] Differential Revision: D59693730 --- .../ReactCommon/jsinspector-modern/CdpJson.h | 2 +- .../jsinspector-modern/ExecutionContext.cpp | 7 +++---- .../FallbackRuntimeAgentDelegate.h | 10 +++++----- .../ReactCommon/jsinspector-modern/HostAgent.h | 2 +- .../jsinspector-modern/NetworkIOAgent.h | 16 +++++++--------- .../jsinspector-modern/RuntimeTargetConsole.cpp | 4 ++-- 6 files changed, 19 insertions(+), 22 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/CdpJson.h b/packages/react-native/ReactCommon/jsinspector-modern/CdpJson.h index dba85f39bec..348ade26874 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/CdpJson.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/CdpJson.h @@ -85,7 +85,7 @@ using ParseError = folly::json::parse_error; /** * Returns a JSON-formatted string representing an error. * - * {"id": , "error": { "code": , "message": }} + * {"id": , "error": { "code": , "message": }} * * \param id Request ID. Mandatory, null only if the request omitted it or * could not be parsed. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/ExecutionContext.cpp b/packages/react-native/ReactCommon/jsinspector-modern/ExecutionContext.cpp index a298816d3af..4a0168bfde2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/ExecutionContext.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/ExecutionContext.cpp @@ -11,8 +11,11 @@ namespace facebook::react::jsinspector_modern { namespace { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-const-variable" template inline constexpr bool always_false_v = false; +#pragma clang diagnostic pop } // namespace @@ -34,10 +37,6 @@ bool ExecutionContextSelector::matches( } }, value_); - - // Prevent the compiler from thinking always_false_v is unused when the - // visitor is (correctly) exhaustive. - (void)always_false_v; } ExecutionContextSelector ExecutionContextSelector::byId(int32_t id) { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h index 56e3e895959..1c9934f02e4 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeAgentDelegate.h @@ -51,11 +51,11 @@ class FallbackRuntimeAgentDelegate : public RuntimeAgentDelegate { void sendFallbackRuntimeWarning(); /** - * Send a simple Log.entryAdded notification with the given - * \param text. You must ensure that the frontend has enabled Log - * notifications (using Log.enable) prior to calling this function. In Chrome - * DevTools, the message will appear in the Console tab along with regular - * console messages. + * Send a simple Log.entryAdded notification with the given text. + * You must ensure that the frontend has enabled Log notifications (using + * Log.enable) prior to calling this function. In Chrome DevTools, the message + * will appear in the Console tab along with regular console messages. + * \param text The text to send. */ void sendWarningLogEntry(std::string_view text); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h index 8fea9da662e..d6c63d03241 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h @@ -41,7 +41,7 @@ class HostAgent final { * HostTargetDelegate and underlying HostTarget both outlive the agent. * \param hostMetadata Metadata about the host that created this agent. * \param sessionState The state of the session that created this agent. - * \param exector A void executor to be used by async-aware handlers. + * \param executor A void executor to be used by async-aware handlers. */ HostAgent( FrontendChannel frontendChannel, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h index ebc58d44a37..a9aa61b49c6 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h @@ -166,20 +166,18 @@ class LoadNetworkResourceDelegate { * Called by NetworkIOAgent on handling a * `Network.loadNetworkResource` CDP request. Platform implementations should * override this to perform a network request of the given URL, and use - * listener's callbacks (on any thread) on receipt of headers, data chunks, + * listener's callbacks (via the executor) on receipt of headers, data chunks, * and errors. * * \param params A LoadNetworkResourceRequest, including the url. - * \param listener The listener to call on headers, data chunks, and errors. - * Implementations must ensure that they retain a shared_ptr to listener for - * as long as its callbacks may be called, and should release it once the - * network request is complete or cancelled. Implementations *should* call - * listener->setCancelFunction() to provide a lambda that can be called to - * abort any in-flight network operation that is no longer needed. + * \param executor A listener-scoped executor used by the delegate to execute + * listener callbacks on headers, data chunks, and errors. Implementations + * *should* call listener->setCancelFunction() to provide a lambda that can be + * called to abort any in-flight network operation that is no longer needed. */ virtual void loadNetworkResource( - const LoadNetworkResourceRequest& /*params*/, - ScopedExecutor /*executor*/) = 0; + [[maybe_unused]] const LoadNetworkResourceRequest& params, + [[maybe_unused]] ScopedExecutor executor) = 0; }; /** diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp index f0479e9de46..a129d662339 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp @@ -369,8 +369,8 @@ void RuntimeTarget::installConsoleHandler() { }; /** - * Call \param innerFn and forward any arguments to the original console - * method named \param methodName, if possible. + * Call innerFn and forward any arguments to the original console method + * named methodName, if possible. */ auto forwardToOriginalConsole = [originalConsole]( const char* methodName,