Small changes for correctness by ShawSumma · Pull Request #372 · temperlang/temper

ShawSumma · 2026-03-13T15:37:43Z

I used the AI to identify and fix small problems in the backends, mostly focusing on correctness, but also doing some inlining.

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

…fixes-rust

mikesamuel · 2026-03-14T23:25:55Z

Could you add to functional-test-suite/src/commonMain/resources/types/float/ops/ ?

That'd make sure the corner cases are exercised for all backends.

notactuallytreyanastasio · 2026-03-19T15:12:48Z

Worth noting that in theory this solves #373 in theory and maybe should add a specific test there. Leaving this as a note so its linked in GH history.

tjpalmer

Some of these changes are good, some are meh, some are bad, and some I still haven't looked at enough.

If you have different opinions on any, let me know.

Minimally, this maybe can serve as an example of why not to just do everything that AI says. Which should help for some talking points of why Temper still matters.

tjpalmer · 2026-03-19T16:48:52Z

be-csharp/src/commonMain/resources/lang/temper/be/csharp/temper-core/Float64.cs

            var abs = absTol ?? 0.0;
            var margin = Math.Max(Math.Max(Math.Abs(x), Math.Abs(y)) * rel, abs);
-            return Math.Abs(x - y) < margin;
+            return Math.Abs(x - y) <= margin;


This does match Python's math.isclose better. I'll plan to see if everything else also gets updated in this pr.

tjpalmer · 2026-03-19T16:52:31Z

be-csharp/src/commonMain/resources/lang/temper/be/csharp/temper-core/Float64.cs

                : trimmed == "-Infinity"
                    ? double.NegativeInfinity
-                    : double.Parse(s);
+                    : double.Parse(trimmed);


In Remarks here, it says that double.Parse allows for surrounding whitespace, so it doesn't matter much whether s or trimmed goes in. Maybe worth a comment either way, and maybe trimmed is less surprising. So probably an ok change to make, even if s also works.

tjpalmer · 2026-03-19T16:53:48Z

be-csharp/src/commonMain/resources/lang/temper/be/csharp/temper-core/StringUtil.cs

            {
                count += 1;
-                if (i + 1 < right)
+                if (i + 1 < limit)


Because the loop prevents i from going past limit, either works fine here, so again, a needless change but also not a breaking one. And maybe limit is less surprising to some people.

tjpalmer · 2026-03-19T16:55:19Z

be-csharp/src/commonMain/resources/lang/temper/be/csharp/temper-core/StringUtil.cs

            {
-                return -1;
+                return i;
            }


I personally feel a lot better always using -1 for no string index rather than arbitrary negative values. Is there a good reason for this change that I'm missing? If not, I'd like to revert this.

I agree with Tom on this.

tjpalmer · 2026-03-19T16:55:41Z

be-js/src/commonMain/resources/lang/temper/be/js/temper-core/float.js

  }
  const margin = Math.max(Math.max(Math.abs(x), Math.abs(y)) * relTol, absTol);
-  return Math.abs(x - y) < margin;
+  return Math.abs(x - y) <= margin;


So we've updated cs and js so far.

tjpalmer · 2026-03-19T17:33:39Z

frontend/src/commonMain/kotlin/lang/temper/frontend/typestage/InlineToRepairUnrealizedGoals.kt

            val calleeTree = callToInline.definition
            val formalNameToArg = callToInline.formalNameToFunArg
            val thisBindings = callToInline.thisTypeBindings
-            callToInline.thisTypeBindings


This looks like good cleanup at a glance.

tjpalmer · 2026-03-19T17:34:35Z

frontend/src/commonMain/kotlin/lang/temper/frontend/typestage/Typer.kt

                else -> type
            }
-            is OrType -> type.members.firstNotNullOf { findNominal(it) }
+            is OrType -> type.members.firstNotNullOfOrNull { findNominal(it) }


I haven't analyzed to know if this is a good change or not.

tjpalmer · 2026-03-19T17:36:02Z

lexer/src/commonMain/kotlin/lang/temper/lexer/Lexer.kt

                        // larger shift or comparison operator, or close tag start marker.
                        val before = text[end - 1]
-                        val after = if (end + 1 < limit) { text[end - 1] } else { '\u0000' }
+                        val after = if (end + 1 < limit) { text[end + 1] } else { '\u0000' }


The old code looks sus, but I don't know why it wouldn't have triggered problems. No test changes from this?

Nope, no changes, it's one of those "how did it ever work?!" things.

result-helpers/src/commonMain/kotlin/lang/temper/result/junit/JUnitResultParser.kt

tjpalmer · 2026-03-19T17:37:06Z

tooling/src/commonMain/kotlin/lang/temper/tooling/buildrun/Runner.kt

    fun append(str: String) { buffer.append(str) }

+    @Synchronized
    override fun toString(): String = "$buffer"


Again, some analysis here would be good, which I haven't done right now.

It makes sure the buffer is never read at the same the above append is happening, as that would be bad and give you a toString() that never actually existed.

It makes sure the buffer is never read at the same the above append is happening, as that would be bad and give you a toString() that never actually existed.

I guess the question is if it's ever used from multithreaded contexts. I still haven't looked, though.

For the time being it feels like it could be, as the @Synchronized on the other method is preexisting. If it weren't used multithreaded then both should be removed.

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

tjpalmer · 2026-03-19T18:45:18Z

Could you add to functional-test-suite/src/commonMain/resources/types/float/ops/ ?

That'd make sure the corner cases are exercised for all backends.

Maybe a corner case test for near? Maybe also some large int64 cases that might trigger the old Python int things? I already forget if other additional testing would be good.

Signed-off-by: Tom <tom@contextfree.info>

tjpalmer · 2026-03-25T00:11:00Z

be-lua/src/commonMain/resources/lang/temper/be/lua/temper-core/init.lua


 function temper.string_indexof(str, target, i)
-    return string.find(str, target, i, true) or 0
+    return string.find(str, target, i, true) or -1


For lua, I think we're using 0 as no string index, since indices start at 1?

I liked having it be -1 everywhere, tho maybe this needs more thought

Does this affect == with NoStringIndex

tjpalmer · 2026-03-25T16:17:24Z

be-rust/src/commonMain/kotlin/lang/temper/be/rust/RustTranslator.kt

-                TmpLOperator.AmpAmp -> TODO() // RustOperator.LogicalAnd
-                TmpLOperator.BarBar -> TODO() // RustOperator.LogicalOr
+                TmpLOperator.AmpAmp -> RustOperator.LogicalAnd
+                TmpLOperator.BarBar -> RustOperator.LogicalOr


I see here's where they were being ignored before. Looks like we don't actually translate to using these. Instead we assign bool values from if/else sequences. But probably good to implement these anyway, just in case, and even if not easy to test, the implementations look ok.

ShawSumma added 9 commits March 9, 2026 20:23

ai

60e6da2

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes to type stuff

4b1b76d

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes for rust

d71c325

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes for rust

00660a4

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes for lua

8d42701

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes to rust

8eaf9fd

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

ai fixes to lua

f06fc9f

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

Merge remote-tracking branch 'origin/main' into shaw-ai-fixes-rust

0ec6083

Merge remote-tracking branch 'origin/shaw-ai-fixes-lua' into shaw-ai-…

4137463

…fixes-rust

ShawSumma marked this pull request as ready for review March 14, 2026 11:57

tjpalmer mentioned this pull request Mar 14, 2026

Temper has Float64 division that generates NaN / Infinity in Python backend #373

Open

tjpalmer reviewed Mar 19, 2026

View reviewed changes

revert things as per tom's suggestions

94d7e19

Signed-off-by: Shaw Summa <shawsumma@gmail.com>

Finish changing and testing near maybe (#386)

368455d

Signed-off-by: Tom <tom@contextfree.info>

tjpalmer reviewed Mar 25, 2026

View reviewed changes

Conversation

ShawSumma commented Mar 13, 2026

Uh oh!

mikesamuel commented Mar 14, 2026

Uh oh!

notactuallytreyanastasio commented Mar 19, 2026

Uh oh!

tjpalmer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjpalmer commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants