Skip to content

Commit

Permalink
Merge pull request #3736 from tokiwa-software/fix_3734
Browse files Browse the repository at this point in the history
jvm: Use special value instead of `null` for unit type effect to fix #3734
  • Loading branch information
michaellilltokiwa authored Sep 7, 2024
2 parents 64529cf + bf44c86 commit 3020023
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 12 deletions.
4 changes: 3 additions & 1 deletion src/dev/flang/be/jvm/Intrinsix.java
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,9 @@ else if (in.equals("fuzion.sys.internal_array.setel"))
var arg = args.get(0);
if (jvm._types.resultType(ecl) == ClassFileConstants.PrimitiveType.type_void)
{
arg = arg.drop().andThen(Expr.ACONST_NULL);
arg = arg.drop().andThen(Expr.getstatic(Names.RUNTIME_CLASS,
"_UNIT_TYPE_EFFECT_",
Names.ANYI_TYPE));
}
if (call_t instanceof ClassType call_ct)
{
Expand Down
1 change: 1 addition & 0 deletions src/dev/flang/be/jvm/JVM.java
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ void prepare(JVM jvm)
"dev/flang/be/jvm/runtime/Runtime$2.class",
"dev/flang/be/jvm/runtime/Runtime$3.class",
"dev/flang/be/jvm/runtime/Runtime$4.class",
"dev/flang/be/jvm/runtime/Runtime$5.class",
"dev/flang/be/jvm/runtime/Runtime$Abort.class",
"dev/flang/util/ANY.class",
"dev/flang/util/Errors.class",
Expand Down
33 changes: 22 additions & 11 deletions src/dev/flang/be/jvm/runtime/Runtime.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ private SystemErrNo(final int errno)
public static final String CLASS_NAME_TO_FUZION_CLAZZ_NAME = "CLASS_NAME_TO_FUZION_CLAZZ_NAME.txt";


/**
* Value used for `FuzionThread.effect_store` and `FuzionThread.effect_load`
* to distinguish a unit value effect from a not existing effect
*/
public static final AnyI _UNIT_TYPE_EFFECT_ = new AnyI() { };


/*-------------------------- static fields --------------------------*/


Expand Down Expand Up @@ -513,11 +520,11 @@ public static void effect_default(int id, AnyI instance)


/**
* Helper method to implement intrinsic effect.type.is_installed.
* Helper method to implement intrinsic effect.type.is_instated.
*
* @param id an effect id.
*
* @return true iff an effect with that id was installed.
* @return true iff an effect with that id was instated.
*/
public static boolean effect_is_instated(int id)
{
Expand Down Expand Up @@ -573,7 +580,7 @@ else if (o != null)


/**
* Helper method to implement effect.abort. Abort the currently installed
* Helper method to implement effect.abort. Abort the currently instated
* effect with given id. Helper to implement intrinsic effect.abort.
*
* @param id the id of the effect type that is aborted.
Expand All @@ -585,20 +592,24 @@ public static void effect_abort(int id)


/**
* Helper method to implement effect.type.instante0. Install an instance of effect
* type specified by id and run f.call while it is installed. Helper to
* Helper method to implement effect.type.instante0. Instate an instance of effect
* type specified by id and run f.call while it is instated. Helper to
* implement intrinsic effect.abort.
*
* @param id the id of the effect that is installed
* @param id the id of the effect that is instated
*
* @param instance the effect instance that is installed
* @param instance the effect instance that is instated, NOTE: This is `_UNIT_TYPE_EFFECT_`
* for a unit type effect.
*
* @param code the Unary instance to be executed
*
* @param call the Java clazz of the Unary instance to be executed.
*/
public static void effect_instate(int id, AnyI instance, Any code, Class call)
{
if (PRECONDITIONS) require
(instance != null);

var t = currentThread();

var old = t.effect_load(id);
Expand Down Expand Up @@ -647,14 +658,14 @@ public static void effect_instate(int id, AnyI instance, Any code, Class call)
}

/**
* Helper method to implement `effect.env` expressions. Returns the installed
* Helper method to implement `effect.env` expressions. Returns the instated
* effect with the given id. Causes an error in case no such effect exists.
*
* @param id the id of the effect that should be loaded.
*
* @return the instance that was installed for this id
* @return the instance that was instated for this id
*
* @throws Error in case no instance was installed.
* @throws Error in case no instance was instated.
*/
public static AnyI effect_get(int id)
{
Expand All @@ -663,7 +674,7 @@ public static AnyI effect_get(int id)
var result = t.effect_load(id);
if (result == null)
{
throw new Error("No effect of "+id+" installed");
throw new Error("No effect of "+id+" instated");
}
return result;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/reg_issue3734/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# This file is part of the Fuzion language implementation.
#
# The Fuzion language implementation is free software: you can redistribute it
# and/or modify it under the terms of the GNU General Public License as published
# by the Free Software Foundation, version 3 of the License.
#
# The Fuzion language implementation is distributed in the hope that it will be
# useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
# License for more details.
#
# You should have received a copy of the GNU General Public License along with The
# Fuzion language implementation. If not, see <https://www.gnu.org/licenses/>.


# -----------------------------------------------------------------------
#
# Tokiwa Software GmbH, Germany
#
# Source code of Fuzion test Makefile
#
# -----------------------------------------------------------------------

override NAME = reg_issue3734
include ../simple.mk
58 changes: 58 additions & 0 deletions tests/reg_issue3734/reg_issue3734.fz
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# This file is part of the Fuzion language implementation.
#
# The Fuzion language implementation is free software: you can redistribute it
# and/or modify it under the terms of the GNU General Public License as published
# by the Free Software Foundation, version 3 of the License.
#
# The Fuzion language implementation is distributed in the hope that it will be
# useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
# License for more details.
#
# You should have received a copy of the GNU General Public License along with The
# Fuzion language implementation. If not, see <https://www.gnu.org/licenses/>.


# -----------------------------------------------------------------------
#
# Tokiwa Software GmbH, Germany
#
# Source code of Fuzion test reg_issue3734
#
# -----------------------------------------------------------------------

# This uses an effect without any state, i.e. a unit type value. This used to cause
# problems with the JVM backend since the unit type value is optimized out and replaced by
# `null` when calling `Runtime.effect_instate()`, while `null` is used to identify the lack
# of an installed effect.
#
# The fix is to use a special value `Runtime._UUNIT_TYPE_EFFECT_`.
#
reg_issue3734 =>

# the original code from #3734
e : effect is
stop => e.type.abort e

codea1 => "hi"
codea2 => e.env.stop
defa => "ho"

codeb1 => 42
codeb2 => e.env.stop
defb => 666

say (e.instate String e codea1 defa)
say (e.instate String e codea2 defa)
say (e.instate i32 e codeb1 defb)
say (e.instate i32 e codeb2 defb)

# the simplified example from #3734
f : effect is

f.instate f ()->

# note that `(3%%4)^` is the identity operation for bool in disguise such that DFA
# no longer considers the result `e.is_instated` the constant `true`
#
say "codea1: "+((3%%4)^f.is_instated)
Empty file.
5 changes: 5 additions & 0 deletions tests/reg_issue3734/reg_issue3734.fz.expected_out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
hi
ho
42
666
codea1: true

0 comments on commit 3020023

Please sign in to comment.