Skip to content

Commit 9944d19

Browse files
headiusheadius
authored
Merge pull request #9112 from headius/final_proc_file_line
Prevent modifying state in RubyProc
2 parents 846f601 + 5d51024 commit 9944d19

File tree

3 files changed

+96
-61
lines changed

3 files changed

+96
-61
lines changed

‎core/src/main/java/org/jruby/RubyMethod.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,7 @@ public IRubyObject to_proc(ThreadContext context) {
239239
receiver, originModule, originName, getFilename(), line == -1 ? -1 : line - 1);
240240
Block b = MethodBlockBody.createMethodBlock(body);
241241

242-
RubyProc proc = RubyProc.newProc(context.runtime, b, Block.Type.LAMBDA);
243-
proc.setFromMethod();
242+
RubyProc proc = RubyProc.newMethodProc(context.runtime, b);
244243
return proc;
245244
}
246245

‎core/src/main/java/org/jruby/RubyModule.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3030,9 +3030,7 @@ private DynamicMethod createProcMethod(Ruby runtime, String name, Visibility vis
30303030
block.getBinding().getFrame().setName(name);
30313031

30323032
// a normal block passed to define_method changes to do arity checking; make it a lambda
3033-
RubyProc proc = runtime.newProc(Block.Type.LAMBDA, block);
3034-
3035-
proc.setFromMethod();
3033+
RubyProc proc = RubyProc.newMethodProc(runtime, block);
30363034

30373035
// various instructions can tell this scope is not an ordinary block but a block representing
30383036
// a method definition.

‎core/src/main/java/org/jruby/RubyProc.java‎

Lines changed: 94 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@
3939
import org.jruby.anno.JRubyMethod;
4040

4141
import org.jruby.ast.util.ArgsUtil;
42-
import org.jruby.common.IRubyWarnings;
4342
import org.jruby.ir.runtime.IRRuntimeHelpers;
44-
import org.jruby.lexer.yacc.ISourcePosition;
4543
import org.jruby.parser.StaticScope;
4644
import org.jruby.runtime.Binding;
4745
import org.jruby.runtime.Block;
@@ -62,40 +60,90 @@
6260
import static org.jruby.api.Define.defineClass;
6361
import static org.jruby.api.Error.argumentError;
6462
import static org.jruby.api.Warn.warn;
63+
import static org.jruby.runtime.Block.Type.LAMBDA;
64+
import static org.jruby.runtime.Block.Type.PROC;
6565
import static org.jruby.runtime.ObjectAllocator.NOT_ALLOCATABLE_ALLOCATOR;
6666
import static org.jruby.runtime.ThreadContext.resetCallInfo;
6767
import static org.jruby.util.RubyStringBuilder.types;
6868

6969
@JRubyClass(name="Proc")
7070
public class RubyProc extends RubyObject implements DataType {
71-
private Block block = Block.NULL_BLOCK;
71+
private final Block block;
7272
private final Block.Type type;
73-
private String file = null;
74-
private int line = -1;
75-
private boolean fromMethod;
73+
private final String file;
74+
private final int line;
75+
private final boolean fromMethod;
7676

77-
protected RubyProc(Ruby runtime, RubyClass rubyClass, Block.Type type) {
77+
/**
78+
* Construct a new RubyProc with the given parameters. The Block's proc object will be set to this RubyProc.
79+
*
80+
* @param runtime
81+
* @param rubyClass
82+
* @param block
83+
* @param file
84+
* @param line
85+
* @param fromMethod
86+
*/
87+
private RubyProc(Ruby runtime, RubyClass rubyClass, Block block, Block.Type type, String file, int line, boolean fromMethod) {
7888
super(runtime, rubyClass);
7989

90+
this.block = block;
8091
this.type = type;
81-
}
92+
this.file = file;
93+
this.line = line;
94+
this.fromMethod = fromMethod;
8295

83-
@Deprecated(since = "9.0.0.0")
84-
protected RubyProc(Ruby runtime, RubyClass rubyClass, Block.Type type, ISourcePosition sourcePosition) {
85-
this(runtime, rubyClass, type, sourcePosition.getFile(), sourcePosition.getLine());
96+
block.setProcObject(this);
8697
}
8798

88-
protected RubyProc(Ruby runtime, RubyClass rubyClass, Block.Type type, String file, int line) {
89-
this(runtime, rubyClass, type);
99+
/**
100+
* Construct a new RubyProc with the given parameters. The Block's proc object will be set to this RubyProc.
101+
*
102+
* @param runtime
103+
* @param rubyClass
104+
* @param block
105+
* @param file
106+
* @param line
107+
*/
108+
private RubyProc(Ruby runtime, RubyClass rubyClass, Block block, Block.Type type, String file, int line) {
109+
this(runtime, rubyClass, block, type, file, line, false);
110+
}
90111

91-
this.file = file;
92-
this.line = line;
112+
/**
113+
* Construct a new RubyProc with the given parameters. The Block's proc object will be set to this RubyProc.
114+
*
115+
* @param runtime
116+
* @param rubyClass
117+
* @param block
118+
* @param file
119+
* @param line
120+
*/
121+
public RubyProc(Ruby runtime, RubyClass rubyClass, Block block, String file, int line) {
122+
this(runtime, rubyClass, block, block.type, file, line, false);
93123
}
94124

125+
/**
126+
* Construct a new RubyProc with the given parameters. The Block's proc object will be set to this RubyProc.
127+
*
128+
* @param runtime
129+
* @param rubyClass
130+
* @param block
131+
* @param fromMethod
132+
*/
133+
public RubyProc(Ruby runtime, RubyClass rubyClass, Block block, Block.Type type, boolean fromMethod) {
134+
this(runtime, rubyClass, block, type, null, -1, fromMethod);
135+
}
95136

96-
public RubyProc(Ruby runtime, RubyClass rubyClass, Block block, String file, int line) {
97-
this(runtime, rubyClass, block.type, file, line);
98-
this.block = block;
137+
/**
138+
* Construct a new RubyProc with the given parameters. The Block's proc object will be set to this RubyProc.
139+
*
140+
* @param runtime
141+
* @param rubyClass
142+
* @param block
143+
* @param type
144+
*/
145+
private RubyProc(Ruby runtime, RubyClass rubyClass, Block block, Block.Type type) {
146+
this(runtime, rubyClass, block, type, null, -1, false);
99147
}
100148

101149
public static RubyClass createProcClass(ThreadContext context, RubyClass Object) {
@@ -119,20 +167,13 @@ public static RubyProc newProc(Ruby runtime, Block.Type type) {
119167
public static RubyProc newProc(Ruby runtime, Block block, Block.Type type) {
120168
// The three valid types of execution here are PROC/LAMBDA/THREAD. NORMAL should not normally
121169
// be passed but when it is we just assume it will be a PROC.
122-
if (type == Block.Type.NORMAL) type = Block.Type.PROC;
170+
if (type == Block.Type.NORMAL) type = PROC;
123171

124-
RubyProc proc = new RubyProc(runtime, runtime.getProc(), type);
125-
proc.setup(runtime, block);
126-
127-
return proc;
172+
return new RubyProc(runtime, runtime.getProc(), prepareBlock(runtime, block, type), type);
128173
}
129174

130-
@Deprecated(since = "9.0.0.0")
131-
public static RubyProc newProc(Ruby runtime, Block block, Block.Type type, ISourcePosition sourcePosition) {
132-
RubyProc proc = new RubyProc(runtime, runtime.getProc(), type, sourcePosition);
133-
proc.setup(runtime, block);
134-
135-
return proc;
175+
public static RubyProc newMethodProc(Ruby runtime, Block block) {
176+
return new RubyProc(runtime, runtime.getProc(), prepareBlock(runtime, block, LAMBDA), LAMBDA, true);
136177
}
137178

138179
public static RubyProc newProc(Ruby runtime, Block block, Block.Type type, String file, int line) {
@@ -141,10 +182,8 @@ public static RubyProc newProc(Ruby runtime, Block block, Block.Type type, Strin
141182
}
142183

143184
public static RubyProc newProc(Ruby runtime, RubyClass clazz, Block block, Block.Type type, String file, int line) {
144-
RubyProc proc = new RubyProc(runtime, clazz, type, file, line);
145-
proc.setup(runtime, block);
146185

147-
return proc;
186+
return new RubyProc(runtime, clazz, prepareBlock(runtime, block, type), type, file, line);
148187
}
149188

150189
/**
@@ -163,23 +202,19 @@ public static IRubyObject newInstance(ThreadContext context, IRubyObject recv, I
163202
return block.getProcObject();
164203
}
165204

166-
RubyProc obj = new RubyProc(context.runtime, (RubyClass)recv, Block.Type.PROC);
167-
obj.setup(context.runtime, block);
205+
RubyProc obj = new RubyProc(context.runtime, (RubyClass)recv, prepareBlock(context.runtime, block, PROC), PROC);
168206

169207
obj.callMethod(context, "initialize", args, block);
170208
return obj;
171209
}
172210

173-
private void setup(Ruby runtime, Block procBlock) {
174-
if (!procBlock.isGiven()) throw argumentError(runtime.getCurrentContext(), "tried to create Proc object without a block");
211+
private static Block prepareBlock(Ruby runtime, Block block, Block.Type type) {
212+
if (!block.isGiven()) throw argumentError(runtime.getCurrentContext(), "tried to create Proc object without a block");
175213

176-
if (isLambda()) {
177-
// TODO: warn "tried to create Proc object without a block"
178-
}
179-
180-
if (isThread()) {
214+
Block procBlock;
215+
if (type == Block.Type.THREAD) {
181216
// binding for incoming proc must not share frame
182-
Binding oldBinding = procBlock.getBinding();
217+
Binding oldBinding = block.getBinding();
183218
Binding newBinding = new Binding(
184219
oldBinding.getSelf(),
185220
oldBinding.getFrame().duplicate(),
@@ -188,32 +223,32 @@ private void setup(Ruby runtime, Block procBlock) {
188223
oldBinding.getMethod(),
189224
oldBinding.getFile(),
190225
oldBinding.getLine());
191-
block = new Block(procBlock.getBody(), newBinding, type);
226+
procBlock = new Block(block.getBody(), newBinding, type);
192227

193228
// Mark as escaped, so non-local flow errors immediately
194-
block.escape();
229+
procBlock.escape();
195230

196231
// modify the block with a new backref/lastline-grabbing scope
197-
StaticScope oldScope = block.getBody().getStaticScope();
232+
StaticScope oldScope = procBlock.getBody().getStaticScope();
198233
StaticScope newScope = oldScope.duplicate();
199-
block.getBody().setStaticScope(newScope);
234+
procBlock.getBody().setStaticScope(newScope);
200235
} else {
201236
// just use as is unless type differs
202-
if (type != procBlock.type) {
203-
block = procBlock.cloneBlockAsType(type);
237+
if (type != block.type) {
238+
procBlock = block.cloneBlockAsType(type);
204239
} else {
205-
block = procBlock;
240+
procBlock = block;
206241
}
207242
}
208243

209244
// force file/line info into the new block's binding
210-
block.getBinding().setFile(block.getBody().getFile());
211-
block.getBinding().setLine(block.getBody().getLine());
212-
213-
block.setProcObject(this);
245+
procBlock.getBinding().setFile(procBlock.getBody().getFile());
246+
procBlock.getBinding().setLine(procBlock.getBody().getLine());
214247

215248
// pre-request dummy scope to avoid clone overhead in lightweight blocks
216-
block.getBinding().getDummyScope(block.getBody().getStaticScope());
249+
procBlock.getBinding().getDummyScope(procBlock.getBody().getStaticScope());
250+
251+
return procBlock;
217252
}
218253

219254
@JRubyMethod(name = "clone")
@@ -242,7 +277,7 @@ public IRubyObject to_s(ThreadContext context) {
242277
boolean isSymbolProc = block.getBody() instanceof RubySymbol.SymbolProcBody;
243278
if (isSymbolProc) {
244279
string.catStringUnsafe("(&:" + ((RubySymbol.SymbolProcBody) block.getBody()).getId() + ")");
245-
} else if ((file = block.getBody().getFile()) != null) {
280+
} else if (block.getBody().getFile() instanceof String file) {
246281
string.catStringUnsafe(" " + file + ":" + (block.getBody().getLine() + 1));
247282
}
248283

@@ -487,7 +522,10 @@ public IRubyObject dup() {
487522
return dup(getRuntime().getCurrentContext());
488523
}
489524

525+
/**
526+
* @deprecated fromMethod is now final
527+
*/
528+
@Deprecated(since = "10.0.3.0")
490529
public void setFromMethod() {
491-
fromMethod = true;
492530
}
493531
}

0 commit comments

Comments
 (0)