Skip to content

Eliminate contextless PythonTuple constructor #1946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 20, 2025
Merged

Eliminate contextless PythonTuple constructor #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/IronPython.Modules/IterTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ public class product : IterBase {
private PythonTuple[] tuples;

public product(CodeContext context, [NotNone] params object[] iterables) {
tuples = ArrayUtils.ConvertAll(iterables, x => new PythonTuple(PythonOps.GetEnumerator(x)));
tuples = ArrayUtils.ConvertAll(iterables, x => new PythonTuple(context, PythonOps.GetEnumerator(x)));
InnerEnumerator = Yielder(tuples);
}

Expand All @@ -788,7 +788,7 @@ public product(CodeContext context, [ParamDictionary] IDictionary<object, object
tuples = new PythonTuple[iterables.Length * iRepeat];
for (int i = 0; i < iRepeat; i++) {
for (int j = 0; j < iterables.Length; j++) {
tuples[i * iterables.Length + j] = new PythonTuple(iterables[j]);
tuples[i * iterables.Length + j] = new PythonTuple(context, iterables[j]);
}
}
InnerEnumerator = Yielder(tuples);
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython.Modules/nt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1389,7 +1389,7 @@ public PythonTuple __reduce__() {

return MakeTuple(
DynamicHelpers.GetPythonTypeFromType(typeof(stat_result)),
MakeTuple(new PythonTuple(this), timeDict)
MakeTuple(new PythonTuple(DefaultContext.Default, this), timeDict)
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython.Modules/resource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private struct_rusage(object?[] sequence) : base(sequence) {
throw PythonOps.TypeError("resource.struct_rusage() takes a {0}-sequence ({1}-sequence given)", n_sequence_fields, __len__());
}

private struct_rusage(object o) : base(o) {
private struct_rusage(object o) : base(DefaultContext.Default, o) {
if (__len__() != n_sequence_fields)
throw PythonOps.TypeError("resource.struct_rusage() takes a {0}-sequence ({1}-sequence given)", n_sequence_fields, __len__());
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython.Modules/time.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ internal struct_time(int year, int month, int day, int hour, int minute, int sec
}

internal struct_time(PythonTuple sequence)
: base(sequence) {
: base(DefaultContext.Default, sequence) {
}

public static struct_time __new__(CodeContext context, [NotNone] PythonType cls, int year, int month, int day, int hour, int minute, int second, int dayOfWeek, int dayOfYear, int isDst) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython/Runtime/ClrModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,7 @@ private static void SortModules(List<string> modules) {
/// All times are expressed in the same unit of measure as DateTime.Ticks
/// </summary>
public static PythonTuple GetProfilerData(CodeContext/*!*/ context, bool includeUnused = false) {
return new PythonTuple(Profiler.GetProfiler(context.LanguageContext).GetProfile(includeUnused));
return new PythonTuple(DefaultContext.Default, Profiler.GetProfiler(context.LanguageContext).GetProfile(includeUnused));
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython/Runtime/Exceptions/PythonExceptions.BaseException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public static object __new__([NotNone] PythonType/*!*/ cls, [NotNone] params obj
} else {
res = (BaseException)Activator.CreateInstance(cls.UnderlyingSystemType, cls)!;
}
res._args = new PythonTuple(args);
res._args = new PythonTuple(DefaultContext.Default, args);
return res;
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/IronPython/Runtime/Operations/PythonOps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ internal static bool TryInvokeLengthHint(CodeContext context, object? sequence,
List<object?> largs;

if (argsTuple != null && args.Length == names.Length) {
if (!(argsTuple is PythonTuple tuple)) tuple = new PythonTuple(argsTuple);
if (!(argsTuple is PythonTuple tuple)) tuple = new PythonTuple(DefaultContext.Default, argsTuple);

largs = new List<object?>(tuple);
largs.AddRange(args);
Expand Down Expand Up @@ -1813,7 +1813,7 @@ public static void DictUpdate(CodeContext context, PythonDictionary dict, object
/// LIST_TO_TUPLE
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public static PythonTuple ListToTuple(PythonList list) => new PythonTuple(list);
public static PythonTuple ListToTuple(PythonList list) => new PythonTuple(DefaultContext.Default, list);

/// <summary>
/// SET_ADD
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython/Runtime/PythonFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public PythonTuple __defaults__ {
get {
if (_defaults.Length == 0) return null;

return new PythonTuple(_defaults);
return new PythonTuple(DefaultContext.Default, _defaults);
}
set {
_defaults = value == null ? [] : value.ToArray();
Expand Down
2 changes: 1 addition & 1 deletion src/core/IronPython/Runtime/PythonList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1302,7 +1302,7 @@ int IStructuralEquatable.GetHashCode(IEqualityComparer comparer) {
int res;
CompareUtil.Push(this);
try {
res = ((IStructuralEquatable)new PythonTuple(this)).GetHashCode(comparer);
res = ((IStructuralEquatable)PythonTuple.Make(GetObjectArray())).GetHashCode(comparer);
} finally {
CompareUtil.Pop(this);
}
Expand Down
24 changes: 12 additions & 12 deletions src/core/IronPython/Runtime/PythonTuple.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public class PythonTuple : IList, IList<object?>, ICodeFormattable, IExpressionS

internal static readonly PythonTuple EMPTY = new PythonTuple();

public PythonTuple([AllowNull]object o) {
_data = MakeItems(o);
public PythonTuple(CodeContext context, [AllowNull]object o) {
Copy link
Contributor

@slozier slozier Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is technically a breaking change since the constructor was public. Doesn't seem unreasonable that someone could have been calling this from outside the codebase? Maybe we could keep it with the default context (we can add the Obsolete attribute).

Copy link
Member Author

@BCSharp BCSharp Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a breaking change, albeit a straightforward one: all is needed in user code is to add DefaultContext.Default as the first argument to have the same behaviour as before, although I was hoping that by making the parameter explicit it will prompt the user to some deliberation which context to use.

I don't think that adding another constructor with the signature of the old one will work — since they will only differ by CodeContext, the method resolver will not be able to pick one when dynamic code (i.e. Python code) is being run.

Copy link
Contributor

@slozier slozier Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the overload is a conflict then go ahead and break it. 😊

Can't say I fully understand the whole concept of the context.

Copy link
Member Author

@BCSharp BCSharp Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure, I ran a test with the old constructor added back, and sure enough, the resolver fails when trying to instantiate subclasses of tuple.

Perhaps I'll start a wiki page outlining all breaking changes like this one since 3.4.2 and the recommended workarounds, kind of what we have for upgrading from 2.x to 3.x. Hopefully it will be very short. In the release notes of 3.4.3 you could add a link to it.

Copy link
Member Author

@BCSharp BCSharp Apr 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/IronLanguages/ironpython3/wiki/Differences-between-versions

_data = MakeItems(context, o);
}

protected PythonTuple(object?[] items) {
Expand All @@ -55,19 +55,19 @@ public static PythonTuple __new__(CodeContext context, [NotNone] PythonType cls)
if (cls == TypeCache.PythonTuple) {
return EMPTY;
} else {
if (!(cls.CreateInstance(context) is PythonTuple tupObj)) throw PythonOps.TypeError("{0} is not a subclass of tuple", cls);
if (cls.CreateInstance(context) is not PythonTuple tupObj) throw PythonOps.TypeError("{0} is not a subclass of tuple", cls);
return tupObj;
}
}

public static PythonTuple __new__(CodeContext context, [NotNone] PythonType cls, object? sequence) {
if (sequence == null) return new PythonTuple(sequence); // this will throw the proper exception
if (sequence == null) return new PythonTuple(context, sequence); // this will throw the proper exception

if (cls == TypeCache.PythonTuple) {
if (sequence.GetType() == typeof(PythonTuple)) return (PythonTuple)sequence;
return new PythonTuple(sequence);
return new PythonTuple(context, sequence);
} else {
if (!(cls.CreateInstance(context, sequence) is PythonTuple tupObj)) throw PythonOps.TypeError("{0} is not a subclass of tuple", cls);
if (cls.CreateInstance(context, sequence) is not PythonTuple tupObj) throw PythonOps.TypeError("{0} is not a subclass of tuple", cls);
return tupObj;
}
}
Expand Down Expand Up @@ -115,15 +115,15 @@ public int count(object? obj) {

internal static PythonTuple Make(object o) {
if (o is PythonTuple t) return t;
return new PythonTuple(o);
return new PythonTuple(DefaultContext.Default, o);
}

internal static PythonTuple MakeTuple(params object?[] items) {
if (items.Length == 0) return EMPTY;
return new PythonTuple(items);
return new PythonTuple(DefaultContext.Default, items);
}

private static object?[] MakeItems(object? o) {
private static object?[] MakeItems(CodeContext context, object? o) {
var t = o?.GetType();
// Only use fast paths if we have an exact tuple/list, otherwise use iter
if (t == typeof(PythonTuple)) {
Expand All @@ -142,7 +142,7 @@ internal static PythonTuple MakeTuple(params object?[] items) {
PerfTrack.NoteEvent(PerfTrack.Categories.OverAllocate, "TupleOA: " + PythonOps.GetPythonTypeName(o));

var l = new List<object?>();
IEnumerator i = PythonOps.GetEnumerator(o);
IEnumerator i = PythonOps.GetEnumerator(context, o);
while (i.MoveNext()) {
l.Add(i.Current);
}
Expand Down Expand Up @@ -315,8 +315,8 @@ public IEnumerator GetEnumerator() {
}

public object __getnewargs__() {
// Call "new Tuple()" to force result to be a Tuple (otherwise, it could possibly be a Tuple subclass)
return PythonTuple.MakeTuple(new PythonTuple(this));
// Call "MakeTuple()" to force result to be specifically a Tuple (otherwise, it could possibly be a Tuple subclass)
return PythonTuple.MakeTuple(_data);
}

#region IEnumerable<object> Members
Expand Down
6 changes: 3 additions & 3 deletions src/extensions/IronPython.SQLite/Cursor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,10 @@ private object queryExecute(CodeContext context, bool multiple, object operation
(bool?)null
};

new_description[i] = new PythonTuple(descriptor);
new_description[i] = new PythonTuple(DefaultContext.Default, descriptor);
}

this.description = new PythonTuple(new_description);
this.description = new PythonTuple(DefaultContext.Default, new_description);
}
}

Expand Down Expand Up @@ -362,7 +362,7 @@ private object fetchOneRow(CodeContext context)
row[i] = converted;
}

return new PythonTuple(row);
return new PythonTuple(DefaultContext.Default, row);
}

public Cursor executescript(string operation)
Expand Down
4 changes: 2 additions & 2 deletions src/extensions/IronPython.SQLite/Statement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ value is string ||
object proto = DynamicHelpers.GetPythonTypeFromType(typeof(PythonSQLite.PrepareProtocol));
object type = DynamicHelpers.GetPythonType(value);

object key = new PythonTuple(new[] { type, proto });
object key = new PythonTuple(DefaultContext.Default, new[] { type, proto });
Copy link
Contributor

@slozier slozier Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these are not PythonTuple.MakeTuple because IronPython.SQLite only has access to the public API surface?

Copy link
Member Author

@BCSharp BCSharp Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. This is also the reason why in #1938 I directly included FakeInteropServices.cs. We could make this assembly a friend of IronPython.dll and gain access, but it is all-or-nothing internal access. I kind of like that IronPython.SQLite.cs has access only to the public API — it is a good test that the API is usable for programming.

Here, using DefaultContext.Default is not a problem because the type to enumerate is a CLR array. Just slightly less efficient.


return PythonSQLite.adapters.ContainsKey(key);
}
Expand All @@ -228,7 +228,7 @@ private object adaptValue(CodeContext context, object value)
object proto = DynamicHelpers.GetPythonTypeFromType(typeof(PythonSQLite.PrepareProtocol));
object type = DynamicHelpers.GetPythonType(value);

object key = new PythonTuple(new[] { type, proto });
object key = new PythonTuple(DefaultContext.Default, new[] { type, proto });

object adapter;
if(PythonSQLite.adapters.TryGetValue(key, out adapter))
Expand Down
2 changes: 1 addition & 1 deletion src/extensions/IronPython.SQLite/_sqlite.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static object connect(CodeContext context,
public static void register_adapter(CodeContext context, PythonType type, object adapter)
{
object proto = DynamicHelpers.GetPythonTypeFromType(typeof(PrepareProtocol));
object key = new PythonTuple(new object[] { type, proto });
object key = new PythonTuple(DefaultContext.Default, new object[] { type, proto });
adapters[key] = adapter;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/IronPython.Tests/EngineTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,7 @@ public void ScenarioEvaluateInAnonymousEngineModule() {
[Test]
public void ScenarioObjectOperations() {
var ops = _pe.Operations;
Assert.That(ops.Format(new PythonTuple(new object[] { 1, 2, 3 })), Is.EqualTo("(1, 2, 3)"));
Assert.That(ops.Format(PythonTuple.Make(new object[] { 1, 2, 3 })), Is.EqualTo("(1, 2, 3)"));

var scope = _pe.CreateScope();
scope.SetVariable("ops", ops);
Expand Down