[GR-47996] [GR-45678] Implement performance warnings (Warning[:performance])

eregon · eregon · commit 4b9a371cb08e · 2023-08-16T16:58:50.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,6 +39,7 @@ Compatibility:
 * Add `Enumerator#product` (#3039, @itarato).
 * Add `Module#const_added` (#3039,  @itarato).
 * Show the pointer size information (if available) in `FFI::Pointer#inspect` (@nirvdrum).
+* Implement performance warnings (`Warning[:performance]`) like in CRuby 3.3 (@eregon).
 
 Performance:
 
diff --git a/spec/ruby/core/warning/element_reference_spec.rb b/spec/ruby/core/warning/element_reference_spec.rb
@@ -3,8 +3,8 @@
 describe "Warning.[]" do
   ruby_version_is '2.7.2' do
     it "returns default values for categories :deprecated and :experimental" do
-      ruby_exe('p Warning[:deprecated]').chomp.should == "false"
-      ruby_exe('p Warning[:experimental]').chomp.should == "true"
+      ruby_exe('p [Warning[:deprecated], Warning[:experimental]]').chomp.should == "[false, true]"
+      ruby_exe('p [Warning[:deprecated], Warning[:experimental]]', options: "-w").chomp.should == "[true, true]"
     end
   end
 
diff --git a/spec/ruby/core/warning/element_set_spec.rb b/spec/ruby/core/warning/element_set_spec.rb
@@ -17,6 +17,18 @@
     end
   end
 
+  ruby_version_is '3.3' do
+    it "enables or disables performance warnings" do
+      original = Warning[:performance]
+      begin
+        Warning[:performance] = !original
+        Warning[:performance].should == !original
+      ensure
+        Warning[:performance] = original
+      end
+    end
+  end
+
   it "raises for unknown category" do
     -> { Warning[:noop] = false }.should raise_error(ArgumentError, /unknown category: noop/)
   end
diff --git a/spec/tags/core/warning/element_reference_tags.txt b/spec/tags/core/warning/element_reference_tags.txt
@@ -1 +1,2 @@
 slow:Warning.[] returns default values for categories :deprecated and :experimental
+slow:Warning.[] returns default values for :performance category
diff --git a/spec/truffleruby.next-specs b/spec/truffleruby.next-specs
@@ -9,3 +9,5 @@
 # Use spec/ruby/core/nil/nil_spec.rb as a dummy file to avoid being empty (what causes mspec to error)
 spec/ruby/core/nil/nil_spec.rb
 
+spec/ruby/core/warning/element_reference_spec.rb
+spec/ruby/core/warning/element_set_spec.rb
diff --git a/src/launcher/java/org/truffleruby/launcher/CommandLineParser.java b/src/launcher/java/org/truffleruby/launcher/CommandLineParser.java
@@ -361,6 +361,12 @@ private void processArgument() throws CommandLineException {
                             case ":no-experimental":
                                 config.setOption(OptionsCatalog.WARN_EXPERIMENTAL, false);
                                 break;
+                            case ":performance":
+                                config.setOption(OptionsCatalog.WARN_PERFORMANCE, true);
+                                break;
+                            case ":no-performance":
+                                config.setOption(OptionsCatalog.WARN_PERFORMANCE, false);
+                                break;
                             default:
                                 LOGGER.warning("unknown warning category: `" + temp.substring(1) + "'");
                                 break;
@@ -494,6 +500,7 @@ private void processArgument() throws CommandLineException {
     private void setAllWarningCategories(boolean value) {
         config.setOption(OptionsCatalog.WARN_DEPRECATED, value);
         config.setOption(OptionsCatalog.WARN_EXPERIMENTAL, value);
+        // WARN_PERFORMANCE is excluded here, it is not set by -w/-W2 on CRuby
     }
 
     private void enableDisableFeature(String name, boolean enable) {
diff --git a/src/launcher/java/org/truffleruby/launcher/RubyLauncher.java b/src/launcher/java/org/truffleruby/launcher/RubyLauncher.java
@@ -437,6 +437,7 @@ private static void printHelp(PrintStream out) {
         out.println("Warning categories:");
         out.println("  deprecated      deprecated features");
         out.println("  experimental    experimental features");
+        out.println("  performance     performance issues");
     }
 
     // Same as above, but with "ruby -h"
diff --git a/src/main/java/org/truffleruby/RubyContext.java b/src/main/java/org/truffleruby/RubyContext.java
@@ -166,6 +166,7 @@ public final class RubyContext {
 
     private final AssumedValue<Boolean> warningCategoryDeprecated;
     private final AssumedValue<Boolean> warningCategoryExperimental;
+    private final AssumedValue<Boolean> warningCategoryPerformance;
 
     private ImmutableRubyString mainScriptName;
 
@@ -189,6 +190,7 @@ public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
 
         warningCategoryDeprecated = new AssumedValue<>(options.WARN_DEPRECATED);
         warningCategoryExperimental = new AssumedValue<>(options.WARN_EXPERIMENTAL);
+        warningCategoryPerformance = new AssumedValue<>(options.WARN_PERFORMANCE);
 
         safepointManager = new SafepointManager(this);
         coreExceptions = new CoreExceptions(this, language);
@@ -305,6 +307,9 @@ protected boolean patch(Env newEnv) {
         if (newOptions.WARN_EXPERIMENTAL != oldOptions.WARN_EXPERIMENTAL) {
             warningCategoryExperimental.set(newOptions.WARN_EXPERIMENTAL);
         }
+        if (newOptions.WARN_PERFORMANCE != oldOptions.WARN_PERFORMANCE) {
+            warningCategoryPerformance.set(newOptions.WARN_PERFORMANCE);
+        }
 
         // Re-read the value of $TZ as it can be different in the new process
         GetTimeZoneNode.invalidateTZ();
@@ -758,6 +763,10 @@ public AssumedValue<Boolean> getWarningCategoryExperimental() {
         return warningCategoryExperimental;
     }
 
+    public AssumedValue<Boolean> getWarningCategoryPerformance() {
+        return warningCategoryPerformance;
+    }
+
     public PrintStream getEnvOutStream() {
         return outStream;
     }
diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java
@@ -1851,6 +1851,11 @@ protected boolean getCategoryExperimental(RubySymbol category) {
             return getContext().getWarningCategoryExperimental().get();
         }
 
+        @Specialization(guards = "category == coreSymbols().PERFORMANCE")
+        protected boolean getCategoryPerformance(RubySymbol category) {
+            return getContext().getWarningCategoryPerformance().get();
+        }
+
     }
 
     @Primitive(name = "warning_set_category")
@@ -1864,6 +1869,8 @@ protected boolean setCategory(RubySymbol category, boolean newValue) {
                 existingValue = getContext().getWarningCategoryDeprecated();
             } else if (category == coreSymbols().EXPERIMENTAL) {
                 existingValue = getContext().getWarningCategoryExperimental();
+            } else if (category == coreSymbols().PERFORMANCE) {
+                existingValue = getContext().getWarningCategoryPerformance();
             } else {
                 throw CompilerDirectives.shouldNotReachHere("unexpected warning category");
             }
diff --git a/src/main/java/org/truffleruby/core/symbol/CoreSymbols.java b/src/main/java/org/truffleruby/core/symbol/CoreSymbols.java
@@ -40,6 +40,7 @@ public final class CoreSymbols {
     public final RubySymbol ON_BLOCKING = createRubySymbol("on_blocking");
     public final RubySymbol DEPRECATED = createRubySymbol("deprecated");
     public final RubySymbol EXPERIMENTAL = createRubySymbol("experimental");
+    public final RubySymbol PERFORMANCE = createRubySymbol("performance");
     public final RubySymbol BIG = createRubySymbol("big");
     public final RubySymbol LITTLE = createRubySymbol("little");
     public final RubySymbol NATIVE = createRubySymbol("native");
diff --git a/src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java b/src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java
@@ -57,6 +57,11 @@ protected void warnOnce(String message) throws Warned {
             return;
         }
 
+        // Only warn if Warning[:performance] is true
+        if (!getContext().getWarningCategoryPerformance().get()) {
+            return;
+        }
+
         log(message);
         throw new Warned();
     }
diff --git a/src/main/java/org/truffleruby/options/Options.java b/src/main/java/org/truffleruby/options/Options.java
@@ -131,6 +131,8 @@ public final class Options {
     public final boolean WARN_DEPRECATED;
     /** --warn-experimental=true */
     public final boolean WARN_EXPERIMENTAL;
+    /** --warn-performance=false */
+    public final boolean WARN_PERFORMANCE;
     /** --use-truffle-regex=true */
     public final boolean USE_TRUFFLE_REGEX;
     /** --warn-truffle-regex-compile-fallback=false */
@@ -262,6 +264,7 @@ public Options(Env env, OptionValues options, LanguageOptions languageOptions) {
         CEXTS_LOG_WARNINGS = options.get(OptionsCatalog.CEXTS_LOG_WARNINGS_KEY);
         WARN_DEPRECATED = options.get(OptionsCatalog.WARN_DEPRECATED_KEY);
         WARN_EXPERIMENTAL = options.get(OptionsCatalog.WARN_EXPERIMENTAL_KEY);
+        WARN_PERFORMANCE = options.get(OptionsCatalog.WARN_PERFORMANCE_KEY);
         USE_TRUFFLE_REGEX = options.get(OptionsCatalog.USE_TRUFFLE_REGEX_KEY);
         WARN_TRUFFLE_REGEX_COMPILE_FALLBACK = options.get(OptionsCatalog.WARN_TRUFFLE_REGEX_COMPILE_FALLBACK_KEY);
         WARN_TRUFFLE_REGEX_MATCH_FALLBACK = options.get(OptionsCatalog.WARN_TRUFFLE_REGEX_MATCH_FALLBACK_KEY);
@@ -410,6 +413,8 @@ public Object fromDescriptor(OptionDescriptor descriptor) {
                 return WARN_DEPRECATED;
             case "ruby.warn-experimental":
                 return WARN_EXPERIMENTAL;
+            case "ruby.warn-performance":
+                return WARN_PERFORMANCE;
             case "ruby.use-truffle-regex":
                 return USE_TRUFFLE_REGEX;
             case "ruby.warn-truffle-regex-compile-fallback":
diff --git a/src/main/ruby/truffleruby/core/truffle/warning_operations.rb b/src/main/ruby/truffleruby/core/truffle/warning_operations.rb
@@ -11,7 +11,7 @@
 module Truffle
   module WarningOperations
     def self.check_category(category)
-      return if category == :deprecated || category == :experimental
+      return if category == :deprecated || category == :experimental || category == :performance
 
       raise ArgumentError, "unknown category: #{category}"
     end
diff --git a/src/main/ruby/truffleruby/core/warning.rb b/src/main/ruby/truffleruby/core/warning.rb
@@ -42,6 +42,8 @@ def self.[](category)
       Primitive.warning_get_category(:deprecated)
     when :experimental
       Primitive.warning_get_category(:experimental)
+    when :performance
+      Primitive.warning_get_category(:performance)
     else
       raise ArgumentError, "unknown category: #{category}"
     end
@@ -55,6 +57,8 @@ def self.[]=(category, value)
       Primitive.warning_set_category(:deprecated, Primitive.as_boolean(value))
     when :experimental
       Primitive.warning_set_category(:experimental, Primitive.as_boolean(value))
+    when :performance
+      Primitive.warning_set_category(:performance, Primitive.as_boolean(value))
     else
       raise ArgumentError, "unknown category: #{category}"
     end
diff --git a/src/options.yml b/src/options.yml
@@ -142,8 +142,9 @@ EXPERT:
     CEXTS_LOG_LOAD: [cexts-log-load, boolean, false, Log loading of cexts]
     CEXTS_LOG_WARNINGS: [cexts-log-warnings, boolean, false, Log cexts warnings]
 
-    WARN_DEPRECATED: [[warn-deprecated, -W], boolean, false, 'Sets deprecated Warning category']
-    WARN_EXPERIMENTAL: [[warn-experimental, -W], boolean, true, 'Sets experimental Warning category']
+    WARN_DEPRECATED: [[warn-deprecated, -W], boolean, false, 'Sets the deprecated Warning category']
+    WARN_EXPERIMENTAL: [[warn-experimental, -W], boolean, true, 'Sets the experimental Warning category']
+    WARN_PERFORMANCE: [[warn-performance, -W], boolean, false, 'Sets the performance Warning category']
 
     # Controlling the regular expression engines
     USE_TRUFFLE_REGEX: [use-truffle-regex, boolean, true, 'Use the Truffle regular expression engine when possible and fallback to Joni otherwise']
diff --git a/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java b/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java
@@ -83,6 +83,7 @@ public final class OptionsCatalog {
     public static final OptionKey<Boolean> CEXTS_LOG_WARNINGS_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> WARN_DEPRECATED_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> WARN_EXPERIMENTAL_KEY = new OptionKey<>(true);
+    public static final OptionKey<Boolean> WARN_PERFORMANCE_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> USE_TRUFFLE_REGEX_KEY = new OptionKey<>(true);
     public static final OptionKey<Boolean> WARN_TRUFFLE_REGEX_COMPILE_FALLBACK_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> WARN_TRUFFLE_REGEX_MATCH_FALLBACK_KEY = new OptionKey<>(false);
@@ -654,15 +655,23 @@ public final class OptionsCatalog {
 
     public static final OptionDescriptor WARN_DEPRECATED = OptionDescriptor
             .newBuilder(WARN_DEPRECATED_KEY, "ruby.warn-deprecated")
-            .help("Sets deprecated Warning category (configured by the -W Ruby option)")
+            .help("Sets the deprecated Warning category (configured by the -W Ruby option)")
             .category(OptionCategory.EXPERT)
             .stability(OptionStability.EXPERIMENTAL)
             .usageSyntax("")
             .build();
 
     public static final OptionDescriptor WARN_EXPERIMENTAL = OptionDescriptor
             .newBuilder(WARN_EXPERIMENTAL_KEY, "ruby.warn-experimental")
-            .help("Sets experimental Warning category (configured by the -W Ruby option)")
+            .help("Sets the experimental Warning category (configured by the -W Ruby option)")
+            .category(OptionCategory.EXPERT)
+            .stability(OptionStability.EXPERIMENTAL)
+            .usageSyntax("")
+            .build();
+
+    public static final OptionDescriptor WARN_PERFORMANCE = OptionDescriptor
+            .newBuilder(WARN_PERFORMANCE_KEY, "ruby.warn-performance")
+            .help("Sets the performance Warning category (configured by the -W Ruby option)")
             .category(OptionCategory.EXPERT)
             .stability(OptionStability.EXPERIMENTAL)
             .usageSyntax("")
@@ -1436,6 +1445,8 @@ public static OptionDescriptor fromName(String name) {
                 return WARN_DEPRECATED;
             case "ruby.warn-experimental":
                 return WARN_EXPERIMENTAL;
+            case "ruby.warn-performance":
+                return WARN_PERFORMANCE;
             case "ruby.use-truffle-regex":
                 return USE_TRUFFLE_REGEX;
             case "ruby.warn-truffle-regex-compile-fallback":
@@ -1666,6 +1677,7 @@ public static OptionDescriptor[] allDescriptors() {
             CEXTS_LOG_WARNINGS,
             WARN_DEPRECATED,
             WARN_EXPERIMENTAL,
+            WARN_PERFORMANCE,
             USE_TRUFFLE_REGEX,
             WARN_TRUFFLE_REGEX_COMPILE_FALLBACK,
             WARN_TRUFFLE_REGEX_MATCH_FALLBACK,
diff --git a/tool/generate-core-symbols.rb b/tool/generate-core-symbols.rb
@@ -59,6 +59,7 @@
     public final RubySymbol ON_BLOCKING = createRubySymbol("on_blocking");
     public final RubySymbol DEPRECATED = createRubySymbol("deprecated");
     public final RubySymbol EXPERIMENTAL = createRubySymbol("experimental");
+    public final RubySymbol PERFORMANCE = createRubySymbol("performance");
     public final RubySymbol BIG = createRubySymbol("big");
     public final RubySymbol LITTLE = createRubySymbol("little");
     public final RubySymbol NATIVE = createRubySymbol("native");

Original file line number	Diff line number	Diff line change
`@@ -1 +1,2 @@`
`1`	`1`	`slow:Warning.[] returns default values for categories :deprecated and :experimental`
	`2`	`+slow:Warning.[] returns default values for :performance category`
Original file line number	Diff line number	Diff line change
`@@ -437,6 +437,7 @@ private static void printHelp(PrintStream out) {`
`437`	`437`	`out.println("Warning categories:");`
`438`	`438`	`out.println(" deprecated deprecated features");`
`439`	`439`	`out.println(" experimental experimental features");`
	`440`	`+ out.println(" performance performance issues");`
`440`	`441`	`}`
`441`	`442`
`442`	`443`	`// Same as above, but with "ruby -h"`
Original file line number	Diff line number	Diff line change
`@@ -1851,6 +1851,11 @@ protected boolean getCategoryExperimental(RubySymbol category) {`
`1851`	`1851`	`return getContext().getWarningCategoryExperimental().get();`
`1852`	`1852`	`}`
`1853`	`1853`
	`1854`	`+ @Specialization(guards = "category == coreSymbols().PERFORMANCE")`
	`1855`	`+ protected boolean getCategoryPerformance(RubySymbol category) {`
	`1856`	`+ return getContext().getWarningCategoryPerformance().get();`
	`1857`	`+ }`
	`1858`	`+`
`1854`	`1859`	`}`
`1855`	`1860`
`1856`	`1861`	`@Primitive(name = "warning_set_category")`
`@@ -1864,6 +1869,8 @@ protected boolean setCategory(RubySymbol category, boolean newValue) {`
`1864`	`1869`	`existingValue = getContext().getWarningCategoryDeprecated();`
`1865`	`1870`	`} else if (category == coreSymbols().EXPERIMENTAL) {`
`1866`	`1871`	`existingValue = getContext().getWarningCategoryExperimental();`
	`1872`	`+ } else if (category == coreSymbols().PERFORMANCE) {`
	`1873`	`+ existingValue = getContext().getWarningCategoryPerformance();`
`1867`	`1874`	`} else {`
`1868`	`1875`	`throw CompilerDirectives.shouldNotReachHere("unexpected warning category");`
`1869`	`1876`	`}`