Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: bc809f751761
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 63c2d530327d
Choose a head ref
  • 2 commits
  • 7 files changed
  • 1 contributor

Commits on Mar 13, 2018

  1. RubySymbol.getRawString() -> idString() - Point that it is raw should… 

    … not the

important part of the API as much as consolidating our vocabulary to know we
need an identifier(id) string.

RubySymbol.retrieveIDSymbol() - We know we need an identifier reserved in the
symbol table but we don't know if it is there already.  This will make or
retrieve one.

Some minor changes to use newer vocab and fix up some constant methods (not
finished).
    @enebo
    enebo committed Mar 13, 2018

    Verified

    This commit was signed with the committer’s verified signature.
    headius Charles Oliver Nutter
    GPG key ID: 8E26FF248BC7AEAD
    Verified
    Learn about vigilant mode
    Copy the full SHA
    a3bc4ed View commit details

  2. Pass all but one regression in MRI test vs master.

    @enebo
    enebo committed Mar 13, 2018
    Copy the full SHA
    63c2d53 View commit details
49 changes: 26 additions & 23 deletions core/src/main/java/org/jruby/RubyBasicObject.java
View file
Original file line number Diff line number Diff line change
@@ -28,7 +28,6 @@
package org.jruby;

import org.jcodings.Encoding;
import org.jcodings.specific.UTF8Encoding;
import org.jruby.ast.util.ArgsUtil;
import org.jruby.ir.interpreter.Interpreter;
import org.jruby.runtime.JavaSites;
@@ -74,6 +73,7 @@
import org.jruby.util.ConvertBytes;
import org.jruby.util.IdUtil;
import org.jruby.util.TypeConverter;
import org.jruby.util.io.EncodingUtils;
import org.jruby.util.unsafe.UnsafeHolder;

import static org.jruby.runtime.Helpers.invokedynamic;
@@ -1223,15 +1223,15 @@ private RubyString inspectObj(final Ruby runtime, RubyString part) {
boolean first = true;
for (Map.Entry<String, VariableAccessor> entry : metaClass.getVariableTableManager().getVariableAccessorsForRead().entrySet()) {
Object value = entry.getValue().get(this);
String id = entry.getKey();
// FIXME: bytelist_love: This probably should be symbol ala newSymbol(id).validInstanceVariableName()
if (!(value instanceof IRubyObject) || !IdUtil.isInstanceVariable(id)) continue;
RubySymbol symbol = runtime.newSymbol(entry.getKey());
if (!(value instanceof IRubyObject) || !symbol.validInstanceVariableName()) continue;

IRubyObject obj = (IRubyObject) value;

if (!first) encStrBufCat(runtime, part, INSPECT_COMMA);
encStrBufCat(runtime, part, INSPECT_SPACE);
encStrBufCat(runtime, part, runtime.newSymbol(id).getBytes());
// FIXME: bytelist_love: EPICLY wrong but something in MRI gets around identifiers of arbitrary encoding.
encStrBufCat(runtime, part, symbol.asString().encode(context, runtime.getEncodingService().convertEncodingToRubyEncoding(part.getEncoding())).asString().getByteList());
encStrBufCat(runtime, part, INSPECT_EQUALS);
encStrBufCat(runtime, part, sites(context).inspect.call(context, obj, obj).convertToString().getByteList());

@@ -2489,7 +2489,7 @@ public RubyArray singleton_methods(ThreadContext context, IRubyObject[] args) {

public IRubyObject singleton_method(IRubyObject name) {
RubySymbol symbol = TypeConverter.checkID(name);
final String methodName = symbol.getRawString();
final String methodName = symbol.idString();
final RubyClass klass = metaClass;
if (klass.isSingleton()) {
DynamicMethod method = klass.searchMethod(methodName);
@@ -2834,10 +2834,7 @@ public IRubyObject op_not_match(ThreadContext context, IRubyObject arg) {
* fred.instance_variable_defined?("@c") #=> false
*/
public IRubyObject instance_variable_defined_p(ThreadContext context, IRubyObject name) {
if (variableTableContains(validateInstanceVariable(name, name.asJavaString()))) {
return context.runtime.getTrue();
}
return context.runtime.getFalse();
return context.runtime.newBoolean(variableTableContains(validateInstanceVariable(name)));
}

/** rb_obj_ivar_get
@@ -2861,11 +2858,9 @@ public IRubyObject instance_variable_defined_p(ThreadContext context, IRubyObjec
* fred.instance_variable_get("@b") #=> 99
*/
public IRubyObject instance_variable_get(ThreadContext context, IRubyObject name) {
Object value;
if ((value = variableTableFetch(validateInstanceVariable(name, name.asJavaString()))) != null) {
return (IRubyObject)value;
}
return context.runtime.getNil();
Object value = variableTableFetch(validateInstanceVariable(name));

return value != null ? (IRubyObject) value : context.nil;
}

/** rb_obj_ivar_set
@@ -2890,7 +2885,7 @@ public IRubyObject instance_variable_get(ThreadContext context, IRubyObject name
*/
public IRubyObject instance_variable_set(IRubyObject name, IRubyObject value) {
// no need to check for ensureInstanceVariablesSettable() here, that'll happen downstream in setVariable
return (IRubyObject)variableTableStore(validateInstanceVariable(name, name.asJavaString()), value);
return (IRubyObject) variableTableStore(validateInstanceVariable(name), value);
}

/** rb_obj_remove_instance_variable
@@ -2917,10 +2912,8 @@ public IRubyObject instance_variable_set(IRubyObject name, IRubyObject value) {
*/
public IRubyObject remove_instance_variable(ThreadContext context, IRubyObject name, Block block) {
ensureInstanceVariablesSettable();
IRubyObject value;
if ((value = (IRubyObject)variableTableRemove(validateInstanceVariable(name, name.asJavaString()))) != null) {
return value;
}
IRubyObject value = (IRubyObject) variableTableRemove(validateInstanceVariable(name));
if (value != null) return value;
throw context.runtime.newNameError("instance variable %1$s not defined", this, name);
}

@@ -3019,16 +3012,26 @@ protected static int nonFixnumHashCode(IRubyObject hashValue) {
/**
* Checks if the name parameter represents a legal instance variable name, and otherwise throws a Ruby NameError
*/
@Deprecated
protected String validateInstanceVariable(String name) {
if (IdUtil.isValidInstanceVariableName(name)) return name;

throw getRuntime().newNameError("`%1$s' is not allowable as an instance variable name", this, name);
}

protected String validateInstanceVariable(IRubyObject nameObj, String name) {
if (IdUtil.isValidInstanceVariableName(name)) return name;
@Deprecated
protected String validateInstanceVariable(IRubyObject name, String _unused_) {
return validateInstanceVariable(name);
}

protected String validateInstanceVariable(IRubyObject name) {
RubySymbol symbol = RubySymbol.retrieveIDSymbol(name);

if (!symbol.validInstanceVariableName()) {
throw getRuntime().newNameError("`%1$s' is not allowable as an instance variable name", this, name);
}

throw getRuntime().newNameError("`%1$s' is not allowable as an instance variable name", this, nameObj);
return symbol.idString();
}

/**
100 changes: 49 additions & 51 deletions core/src/main/java/org/jruby/RubyModule.java
View file
Original file line number Diff line number Diff line change
@@ -136,6 +136,7 @@

import static org.jruby.util.RubyStringBuilder.str;
import static org.jruby.util.RubyStringBuilder.ids;
import static org.jruby.util.RubyStringBuilder.types;


/**
@@ -3016,7 +3017,7 @@ public RubyModule alias_method(ThreadContext context, IRubyObject newId, IRubyOb
RubySymbol newSym = TypeConverter.checkID(newId);
RubySymbol oldSym = TypeConverter.checkID(oldId); // MRI uses rb_to_id but we return existing symbol

defineAlias(newSym.getRawString(), oldSym.getRawString());
defineAlias(newSym.idString(), oldSym.idString());

if (isSingleton()) {
((MetaClass)this).getAttached().callMethod(context, "singleton_method_added", newSym);
@@ -3396,11 +3397,10 @@ public RubyBoolean const_defined_p(ThreadContext context, IRubyObject symbol) {

private RubyBoolean constantDefined(Ruby runtime, RubySymbol symbol, boolean inherit) {
if (symbol.validConstantName()) {
return runtime.newBoolean(getConstantSkipAutoload(symbol.getRawString(), inherit, inherit) != null);
return runtime.newBoolean(getConstantSkipAutoload(symbol.idString(), inherit, inherit) != null);
}

// FIXME: bytelist_love: nameError should use symbol and not Java String.
throw runtime.newNameError(str(runtime, "wrong constant name", ids(runtime, symbol)), symbol.getRawString());
throw runtime.newNameError(str(runtime, "wrong constant name", ids(runtime, symbol)), symbol.idString());
}

@JRubyMethod(name = "const_defined?", required = 1, optional = 1)
@@ -3418,7 +3418,7 @@ public RubyBoolean const_defined_p19(ThreadContext context, IRubyObject[] args)
module = this;
}

String id = RubySymbol.newConstantSymbol(runtime, fullName, segment).getRawString();
String id = RubySymbol.newConstantSymbol(runtime, fullName, segment).idString();
IRubyObject obj = ((RubyModule) module).getConstantNoConstMissing(id, inherit, inherit);

if (obj == null) return null;
@@ -3428,7 +3428,7 @@ public RubyBoolean const_defined_p19(ThreadContext context, IRubyObject[] args)
}, (index, segment, module) -> {
if (module == null) module = this; // Bare 'Foo'

String id = RubySymbol.newConstantSymbol(runtime, fullName, segment).getRawString();
String id = RubySymbol.newConstantSymbol(runtime, fullName, segment).idString();
return ((RubyModule) module).getConstantSkipAutoload(id, inherit, inherit);
});

@@ -3492,44 +3492,28 @@ public IRubyObject const_get_2_0(ThreadContext context, IRubyObject[] args) {
*/
@JRubyMethod(name = "const_set", required = 2)
public IRubyObject const_set(IRubyObject name, IRubyObject value) {
RubySymbol symbol;

// FIXME: bytelist_love: we need some mechanism for new values to construct a symbol so the id-equivalent can look stuff up like these exprs
if (name instanceof RubySymbol) {
symbol = (RubySymbol) name;
} else {
symbol = RubySymbol.newHardSymbol(getRuntime(), name.convertToString().getByteList());
}

if (!symbol.validConstantName()) {
// FIXME: bytelist_love: nameError should use symbol and not Java String.
throw getRuntime().newNameError(str(getRuntime(), "wrong constant name ", name), symbol.getRawString());

}

return setConstant(symbol.getRawString(), value);
return setConstant(validateConstant(name), value);
}

@JRubyMethod(name = "remove_const", required = 1, visibility = PRIVATE)
public IRubyObject remove_const(ThreadContext context, IRubyObject rubyName) {
String name = validateConstant(rubyName);
IRubyObject value;
if ((value = deleteConstant(name)) != null) {
invalidateConstantCache(name);
if (value != UNDEF) {
return value;
String id = validateConstant(rubyName);
IRubyObject value = deleteConstant(id);

if (value != null) { // found it!
invalidateConstantCache(id);

if (value == UNDEF) { // autoload entry
removeAutoload(id);
return context.nil;
}
removeAutoload(name);
// FIXME: I'm not sure this is right, but the old code returned
// the undef, which definitely isn't right...
return context.runtime.getNil();
}

if (hasConstantInHierarchy(name)) {
throw cannotRemoveError(name);
return value;
}

throw context.runtime.newNameError("constant " + name + " not defined for " + getName(), name);
if (hasConstantInHierarchy(id)) throw cannotRemoveError(id);

throw context.runtime.newNameError("constant " + id + " not defined for " + getName(), id);
}

private boolean hasConstantInHierarchy(final String name) {
@@ -3631,37 +3615,37 @@ public Collection<String> constantsCommon(Ruby runtime, boolean replaceModule, b
public void deprecateConstant(Ruby runtime, String name) {
ConstantEntry entry = getConstantMap().get(name);
if (entry == null) {
throw runtime.newNameError("constant " + getName() + "::" + name + " not defined", name);
throw runtime.newNameError(str(runtime, "constant ", types(runtime, this), "::", ids(runtime, name), " not defined"), name);
}

storeConstant(name, entry.value, entry.hidden, true);
invalidateConstantCache(name);
}

@JRubyMethod
public IRubyObject deprecate_constant(ThreadContext context, IRubyObject rname) {
public IRubyObject deprecate_constant(ThreadContext context, IRubyObject name) {
checkFrozen();

deprecateConstant(context.runtime, validateConstant(rname));
deprecateConstant(context.runtime, validateConstant(name));
return this;
}

@JRubyMethod(rest = true)
public IRubyObject deprecate_constant(ThreadContext context, IRubyObject[] names) {
for (IRubyObject rname : names) {
deprecate_constant(context, rname);
for (IRubyObject name: names) {
deprecate_constant(context, name);
}
return this;
}

@JRubyMethod
public IRubyObject private_constant(ThreadContext context, IRubyObject rubyName) {
public IRubyObject private_constant(ThreadContext context, IRubyObject name) {
checkFrozen();

String name = validateConstant(rubyName);
String id = validateConstant(name);

setConstantVisibility(context.runtime, name, true);
invalidateConstantCache(name);
setConstantVisibility(context.runtime, id, true);
invalidateConstantCache(id);

return this;
}
@@ -3675,13 +3659,13 @@ public IRubyObject private_constant(ThreadContext context, IRubyObject[] rubyNam
}

@JRubyMethod
public IRubyObject public_constant(ThreadContext context, IRubyObject rubyName) {
public IRubyObject public_constant(ThreadContext context, IRubyObject name) {
checkFrozen();

String name = validateConstant(rubyName);
String id = validateConstant(name);

setConstantVisibility(context.runtime, name, false);
invalidateConstantCache(name);
setConstantVisibility(context.runtime, id, false);
invalidateConstantCache(id);
return this;
}

@@ -4182,7 +4166,9 @@ public boolean fastIsConstantDefined19(String internedName, boolean inherit) {
//

private RaiseException cannotRemoveError(String id) {
return getRuntime().newNameError("cannot remove " + id + " for " + getName(), id);
Ruby runtime = getRuntime();

return getRuntime().newNameError(str(runtime, "cannot remove ", ids(runtime, id), " for ", types(runtime, this)), id);
}


@@ -4483,10 +4469,22 @@ public Collection<String> getConstantNames(boolean includePrivate) {
return publicNames;
}

/**
* Validates name is a valid constant name and returns its id string.
* @param name object to verify as a valid constant.
* @return the id for this valid constant name.
*/
protected final String validateConstant(IRubyObject name) {
return validateConstant(name.asJavaString(), name);
RubySymbol symbol = RubySymbol.retrieveIDSymbol(name);

if (!symbol.validConstantName()) {
throw getRuntime().newNameError(str(getRuntime(), "wrong constant name ", name), symbol.idString());
}

return symbol.idString();
}

@Deprecated
protected final String validateConstant(String name, IRubyObject errorName) {
if (IdUtil.isValidConstantName19(name)) return name;

Loading