From 57c80fb96f8bcf3e1cecc21193c11419c95cc5d8 Mon Sep 17 00:00:00 2001
From: Tim Smith <tsmith84@proton.me>
Date: Thu, 27 Aug 2026 22:14:14 -0700
Subject: [PATCH] Load pp only when XPath debug tracing is enabled

xpath_parser.rb requires pp at the top of the file, but PP is used in
exactly one place: XPathParser#trace, reached only through enter and
leave, and every one of those call sites is guarded by `if @debug`.
@debug comes from DEBUG, which is false unless the environment variable
REXML_XPATH_PARSER_DEBUG is set to "true".

So every REXML user loads pp and prettyprint for a debugging aid that
is off by default.

Measured on Ruby 4.0.6 (arm64-darwin), best of seven runs:

              require "rexml/document"   files loaded
  before      23.85 ms                   38
  after       21.72 ms                   36
               -2.13 ms                   -2   (9% faster)

Small in absolute terms, but REXML sits underneath a lot of the
ecosystem and the change carries no behavior risk.

Debug tracing still works: running with REXML_XPATH_PARSER_DEBUG=true
produces the same trace output as before.

Test suite: 811 tests, 0 failures, unchanged. The three added tests
cover that requiring rexml/document does not load pp, that XPath
matching works without it, and that evaluating an XPath does not pull
it in. Two of them fail against the previous code.

Signed-off-by: Tim Smith <tsmith84@proton.me>
---
 lib/rexml/xpath_parser.rb |  6 +++++-
 test/test_require.rb      | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 test/test_require.rb

diff --git a/lib/rexml/xpath_parser.rb b/lib/rexml/xpath_parser.rb
index 761b5281..20bdd14d 100644
--- a/lib/rexml/xpath_parser.rb
+++ b/lib/rexml/xpath_parser.rb
@@ -1,6 +1,5 @@
 # frozen_string_literal: false
 
-require "pp"
 require "set"
 
 require_relative 'namespace'
@@ -801,6 +800,11 @@ def evaluate_predicate(expression, nodesets)
     end
 
     def trace(*args)
+      # Loaded here rather than at the top of the file because this method is
+      # only reached when REXML_XPATH_PARSER_DEBUG is set. Requiring pp
+      # eagerly made every REXML user pay for a debugging aid.
+      require "pp"
+
       indent = "  " * @nest
       PP.pp(args, "").each_line do |line|
         puts("#{indent}#{line}")
diff --git a/test/test_require.rb b/test/test_require.rb
new file mode 100644
index 00000000..71e11461
--- /dev/null
+++ b/test/test_require.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: false
+
+require "test/unit"
+
+module REXMLTests
+  # Guards the deferred `require "pp"` in REXML::XPathParser#trace. The checks
+  # run in a subprocess because this one has pp loaded already.
+  class TestRequire < Test::Unit::TestCase
+    PP_LOADED = '$LOADED_FEATURES.any? { |f| File.basename(f) == "pp.rb" }'
+
+    def subprocess(script)
+      lib = File.join(File.dirname(File.expand_path(__dir__)), "lib")
+      IO.popen([RbConfig.ruby, "-I", lib, "-e", script], &:read)
+    end
+
+    def test_requiring_document_does_not_load_pp
+      assert_equal("false", subprocess("require 'rexml/document'; print #{PP_LOADED}"))
+    end
+
+    def test_xpath_works_without_pp
+      script = "require 'rexml/document'; " \
+               "d = REXML::Document.new('<r><a>x</a><a>y</a></r>'); " \
+               "print REXML::XPath.match(d, '//a').map(&:text).join(',')"
+      assert_equal("x,y", subprocess(script))
+    end
+
+    def test_xpath_does_not_load_pp
+      script = "require 'rexml/document'; " \
+               "d = REXML::Document.new('<r><a>x</a></r>'); " \
+               "REXML::XPath.match(d, '//a'); " \
+               "print #{PP_LOADED}"
+      assert_equal("false", subprocess(script))
+    end
+  end
+end