Use of `padding` identifier causes compilation errors under some circumstances

[mtoohey31]

It seems like the use of the "padding" prefix to signify a padding name in src/Common/Name.hs is problematic. If I attempt to build the following (somewhat contrived) source:

module base64

fun decode-char(c : char) : exn int
  throw("decode: invalid character")

pub fun decode(s : sslice, padding : bool = True) : <div, exn> list<int>
  val _ = decode-char('a')
  val _ = padding && s.count == 2 && s.string == "=="
  val _ = padding && s.count == 1 && s.string == "="
  decode(s, padding)

I get this error:

compile: base64.kk
loading: std/core
loading: std/core/types
loading: std/core/hnd
check  : base64
.koka/v2.4.3/gcc-debug/base64.c: In function ‘kk_base64_new_decode_fun280’:
.koka/v2.4.3/gcc-debug/base64.c:128:25: error: incompatible types when assigning to type ‘_Bool’ from type ‘kk_box_t’ {aka ‘struct kk_box_s’}
  128 |   _self->_padding_270 = kk_box_null();
      |                         ^~~~~~~~~~~
.koka/v2.4.3/gcc-debug/base64.c: In function ‘kk_base64_decode_fun280’:
.koka/v2.4.3/gcc-debug/base64.c:137:56: error: ‘_padding_270’ undeclared (first use in this function)
  137 |   kk_std_core__list _x281 = kk_base64__mlift165_decode(_padding_270, s0, _pat13_218, _ctx); /*list<int>*/
      |                                                        ^~~~~~~~~~~~
.koka/v2.4.3/gcc-debug/base64.c:137:56: note: each undeclared identifier is reported only once for each function it appears in
failure during code generation:
  error  : command failed (exit code 1)
  command: /nix/store/90h6k8ylkgn81k10190v5c9ldyjpzgl9-gcc-wrapper-12.3.0/bin/gcc -Wall -Wextra -Wpointer-arith -Wshadow -Wstrict-aliasing -Wno-unknown-pragmas -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-variable -Wno-unused-value -Wno-unused-but-set-variable -g -Og -c -I /home/mtoohey/repos/koka/kklib/include -DKK_MIMALLOC=8 -o .koka/v2.4.3/gcc-debug/base64.o .koka/v2.4.3/gcc-debug/base64.c

However, if I apply the following patch:

diff --git a/src/Common/Name.hs b/src/Common/Name.hs
index 642036d1..593b42e3 100644
--- a/src/Common/Name.hs
+++ b/src/Common/Name.hs
@@ -324,11 +324,11 @@ toHiddenUniqueName i s name

 newPaddingName i
-  = newHiddenName ("padding" ++ show i)
+  = newHiddenName ("padding.." ++ show i)

 isPaddingName name
-  = -- hiddenNameStartsWith name "padding"
-    nameId name `startsWith` (".padding")
+  = -- hiddenNameStartsWith name "padding.."
+    nameId name `startsWith` (".padding..")

 isCCtxName name
   = -- hiddenNameStartsWith name "padding"

...then the error is resolved. Is the approach in the patch an acceptable solution, or is there a more elegant way to fix this? Also, could the same problem arise with cctx names (mentioned right below padding names in Name.hs)? I haven't attempted to reproduce the issue for that case yet but it seems like it might be possible.

[TimWhiting]

I think the proper solution would probably be to make sure hidden names translate to C names that are distinguished from normal variable names in all cases. This particular probably has to do with ascii encoding ..

[mtoohey31]

My understanding of what's happening, and why the patch above seems to fix things, is that a name is made into a "hidden" name by prepending ., or made into a "padding" name by prepending .padding. But when the identifier already begins with padding, names that are meant to be considered just plain "hidden" are also considered "padding", which breaks things.

Do you know if there's a reason why names are marked as "hidden" or "padding" by prepending strings instead of something else like extra field(s) in the Name type? I don't know much about how the compiler works so there might be a reason I'm not aware of. But if there isn't, the prefix approach does seem kind of brittle, so maybe a better way to fix the issue would be to add extra field(s) to the Name type to distinguish between "hidden", "padding", etc. instead of the prefixes?

[TimWhiting]

But my question is why is it hiding padding?

I think the intent is that everywhere a name is hidden it is also prefixed, so that it is unique. A name shouldn't be hidden without being also prefixed. (i.e. your variable padding should never become .padding). I think this invariant isn't respected in src/Common/Unique.hs at line 59. Adding the prefix unique to the baseName there seems to fix the problem in general.

[mtoohey31]

Ah, I see. Yeah, if we ensured that hidden names were prefixed then it sounds like that would prevent the issue.

[daanx]

Thanks for the report ! This is fixed in the latest dev .

[mtoohey31]

Great, thanks for the fix!