Ensure that printed arguments in verify_pred are evaluated exactly once by assigning intermediate values to variables.

PiperOrigin-RevId: 690026827

Author: Googler

googletest/tests/assertions_test.rs

@@ -67,6 +67,57 @@ mod verify_pred {
         )
     }
 
+    #[test]
+    fn evaluates_functions_and_arguments_exactly_once() -> Result<()> {
+        let mut a = 0;
+        let mut foo = |_b: u32| {
+            a += 1;
+            false
+        };
+        let mut b = 0;
+        let mut bar = || {
+            b += 10;
+            b
+        };
+
+        let res = verify_pred!(foo(bar()));
+        verify_that!(
+            res,
+            err(displays_as(contains_substring(indoc! {"
+                foo(bar()) was false with
+                  bar() = 10,
+                  at"
+            })))
+        )?;
+
+        verify_that!((a, b), eq((1, 10)))
+    }
+
+    #[test]
+    fn evaluates_methods_and_arguments_exactly_once() -> Result<()> {
+        struct Apple(u32);
+        impl Apple {
+            fn c(&mut self, _b: bool) -> bool {
+                self.0 += 1;
+                false
+            }
+        }
+        let mut a = Apple(0);
+        let mut b = Apple(10);
+
+        let res = verify_pred!(a.c(b.c(false)));
+        verify_that!(
+            res,
+            err(displays_as(contains_substring(indoc! {"
+                a.c(b.c(false)) was false with
+                  b.c(false) = false,
+                  at"
+            })))
+        )?;
+
+        verify_that!((a.0, b.0), eq((1, 11)))
+    }
+
     #[test]
     fn supports_chained_method_calls() -> Result<()> {
         struct Foo;

googletest_macro/src/verify_pred.rs

@@ -13,10 +13,11 @@
 // limitations under the License.
 
 use quote::quote;
-use syn::{parse_macro_input, punctuated::Punctuated, token::Comma, Expr, Ident};
+use syn::{parse_macro_input, punctuated::Punctuated, spanned::Spanned, token::Comma, Expr, Ident};
 
 struct AccumulatePartsState {
     error_message_ident: Ident,
+    var_defs: Vec<proc_macro2::TokenStream>,
     formats: Vec<proc_macro2::TokenStream>,
 }
 
@@ -31,54 +32,92 @@ impl AccumulatePartsState {
                 "__googletest__verify_pred__error_message",
                 ::proc_macro2::Span::call_site(),
             ),
+            var_defs: vec![],
             formats: vec![],
         }
     }
 
-    /// Accumulates error message formating parts for various parts of the
-    /// expression.
-    fn accumulate_parts(&mut self, expr: &Expr) {
-        let expr_string = expr_to_string(expr);
-        match expr {
-            Expr::Group(group) => {
+    /// Takes an expression with chained field accesses and method calls and
+    /// accumulates intermediate expressions used for computing `verify_pred!`'s
+    /// expression, including intermediate variable assignments to evaluate
+    /// parts of the expression exactly once, and the format string used to
+    /// output intermediate values on condition failure. It returns the new form
+    /// of the input expression with parts of it potentially replaced by the
+    /// intermediate variables.
+    fn accumulate_parts(&mut self, expr: Expr) -> Expr {
+        let expr_string = expr_to_string(&expr);
+        let new_expr = match expr {
+            Expr::Group(mut group) => {
                 // This is an invisible group added for correct precedence in the AST. Just pass
                 // through without having a separate printing result.
-                return self.accumulate_parts(&group.expr);
+                *group.expr = self.accumulate_parts(*group.expr);
+                return Expr::Group(group);
             }
-            Expr::Call(call) => {
-                // Format the args into the error message.
-                self.format_args(&call.args);
+            Expr::Call(mut call) => {
+                // Cache args into intermediate variables.
+                call.args = self.define_variables_for_args(call.args);
+                // Cache function value into an intermediate variable.
+                self.define_variable(&Expr::Call(call))
             }
-            Expr::MethodCall(method_call) => {
-                // Format the args into the error message.
-                self.format_args(&method_call.args);
+            Expr::MethodCall(mut method_call) => {
+                // Cache args into intermediate variables.
+                method_call.args = self.define_variables_for_args(method_call.args);
+                // Cache method value into an intermediate variable.
+                self.define_variable(&Expr::MethodCall(method_call))
             }
-            _ => {}
-        }
+            // By default, assume it's some expression that needs to be cached to avoid
+            // double-evaluation.
+            _ => self.define_variable(&expr),
+        };
         let error_message_ident = &self.error_message_ident;
         self.formats.push(quote! {
             ::googletest::fmt::internal::__googletest__write_expr_value!(
                 &mut #error_message_ident,
                 #expr_string,
-                #expr,
+                #new_expr,
             );
         });
+        new_expr
     }
 
-    // Formats each argument expression into the error message.
-    fn format_args(&mut self, args: &Punctuated<Expr, Comma>) {
-        for pair in args.pairs() {
-            let error_message_ident = &self.error_message_ident;
-            let expr_string = expr_to_string(pair.value());
-            let expr = pair.value();
-            self.formats.push(quote! {
-                ::googletest::fmt::internal::__googletest__write_expr_value!(
-                    &mut #error_message_ident,
-                    #expr_string,
-                    #expr,
-                );
-            });
-        }
+    // Defines a variable for each argument expression so that it's evaluated
+    // exactly once.
+    fn define_variables_for_args(
+        &mut self,
+        args: Punctuated<Expr, Comma>,
+    ) -> Punctuated<Expr, Comma> {
+        args.into_pairs()
+            .map(|mut pair| {
+                let var_expr = self.define_variable(pair.value());
+                let error_message_ident = &self.error_message_ident;
+                let expr_string = expr_to_string(pair.value());
+                self.formats.push(quote! {
+                    ::googletest::fmt::internal::__googletest__write_expr_value!(
+                        &mut #error_message_ident,
+                        #expr_string,
+                        #var_expr,
+                    );
+                });
+
+                *pair.value_mut() = var_expr;
+                pair
+            })
+            .collect()
+    }
+
+    /// Defines a new variable assigned to the expression and returns the
+    /// variable as an expression to be used in place of the passed-in
+    /// expression.
+    fn define_variable(&mut self, value: &Expr) -> Expr {
+        let var_name = Ident::new(
+            &format!("__googletest__verify_pred__var{}", self.var_defs.len()),
+            value.span(),
+        );
+        self.var_defs.push(quote! {
+            #[allow(non_snake_case)]
+            let mut #var_name = #value;
+        });
+        syn::parse::<Expr>(quote!(#var_name).into()).unwrap()
     }
 }
 
@@ -87,13 +126,14 @@ pub fn verify_pred_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStre
     let error_message = quote!(#parsed).to_string() + " was false with";
 
     let mut state = AccumulatePartsState::new();
-    state.accumulate_parts(&parsed);
-    let AccumulatePartsState { error_message_ident, mut formats, .. } = state;
+    let pred_value = state.accumulate_parts(parsed);
+    let AccumulatePartsState { error_message_ident, var_defs, mut formats, .. } = state;
 
     let _ = formats.pop(); // The last one is the full expression itself.
     quote! {
         {
-            if (#parsed) {
+            #(#var_defs)*
+            if (#pred_value) {
                 Ok(())
             } else {
                 let mut #error_message_ident = #error_message.to_string();