Skip to content

Commit

Permalink
[GR-45621] [GR-47996] [GR-45678] Follow-up on "Ruby 3.2.2 import" and…
Browse files Browse the repository at this point in the history
… implement performance warnings

PullRequest: truffleruby/3946
  • Loading branch information
andrykonchin authored and eregon committed Aug 16, 2023
2 parents 66ca27b + 4b9a371 commit 14e0cc5
Show file tree
Hide file tree
Showing 18 changed files with 77 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
4 changes: 2 additions & 2 deletions spec/ruby/core/warning/element_reference_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions spec/ruby/core/warning/element_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/tags/core/warning/element_reference_tags.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
slow:Warning.[] returns default values for categories :deprecated and :experimental
slow:Warning.[] returns default values for :performance category
34 changes: 2 additions & 32 deletions spec/truffleruby.next-specs
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +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/array/slice_spec.rb
spec/ruby/core/array/element_reference_spec.rb

spec/ruby/core/hash/shift_spec.rb
spec/ruby/core/range/size_spec.rb

spec/ruby/core/string/dedup_spec.rb

spec/ruby/core/string/bytesplice_spec.rb

spec/ruby/core/string/byteindex_spec.rb
spec/ruby/core/string/byterindex_spec.rb

spec/ruby/core/queue/deq_spec.rb
spec/ruby/core/queue/pop_spec.rb
spec/ruby/core/queue/shift_spec.rb

spec/ruby/core/sizedqueue/deq_spec.rb
spec/ruby/core/sizedqueue/pop_spec.rb
spec/ruby/core/sizedqueue/shift_spec.rb
spec/ruby/core/sizedqueue/append_spec.rb
spec/ruby/core/sizedqueue/enq_spec.rb
spec/ruby/core/sizedqueue/push_spec.rb

spec/ruby/core/module/const_added_spec.rb
spec/ruby/core/module/refinements_spec.rb
spec/ruby/core/module/undefined_instance_methods_spec.rb
spec/ruby/core/refinement/refined_class_spec.rb
spec/ruby/core/module/used_refinements_spec.rb

spec/ruby/core/thread/each_caller_location_spec.rb
spec/ruby/core/enumerator/product_spec.rb
spec/ruby/core/warning/element_reference_spec.rb
spec/ruby/core/warning/element_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/truffleruby/RubyContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -758,6 +763,10 @@ public AssumedValue<Boolean> getWarningCategoryExperimental() {
return warningCategoryExperimental;
}

public AssumedValue<Boolean> getWarningCategoryPerformance() {
return warningCategoryPerformance;
}

public PrintStream getEnvOutStream() {
return outStream;
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/truffleruby/core/kernel/KernelNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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");
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/truffleruby/core/symbol/CoreSymbols.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/org/truffleruby/options/Options.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/main/ruby/truffleruby/core/warning.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
16 changes: 14 additions & 2 deletions src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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("")
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions test/mri/excludes/TestFiddle.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
exclude :test_nil_true_etc, "NameError: uninitialized constant Fiddle::Qtrue"
exclude :test_dlopen_linker_script_group_linux, "NotImplementedError: NotImplementedError"
exclude :test_dlopen_linker_script_input_linux, "NotImplementedError: NotImplementedError"
1 change: 1 addition & 0 deletions tool/generate-core-symbols.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 14e0cc5

Please sign in to comment.