Skip to content
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.

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

Creating vector.kk with an identifier named empty gives a strange compiler error #586

Open
chtenb opened this issue Oct 10, 2024 · 4 comments

Comments

@chtenb
Copy link
Contributor

chtenb commented Oct 10, 2024

  1. Create vector.kk
  2. Add pub val empty : forall<a> vector<a> = unit/vector()
  3. Compile

(Changing the name of the file, or adding a qualifier to empty makes the error disappear)

check   : vector
In file included from .koka/v3.1.3/clang-cl-debug-29abf8/vector.c:2:
.koka/v3.1.3/clang-cl-debug-29abf8\vector.h(31,20): error: redefinition of 'kk_vector_empty' as different kind of
      symbol
   31 | extern kk_vector_t kk_vector_empty;
      |                    ^
c:/Users/ChieltenBrinke/prj/koka/kklib/include\kklib/vector.h(23,41): note: previous definition is here
   23 | static inline kk_decl_const kk_vector_t kk_vector_empty(void) {
      |                                         ^
.koka/v3.1.3/clang-cl-debug-29abf8/vector.c(4,13): error: redefinition of 'kk_vector_empty' as different kind of symbol
    4 | kk_vector_t kk_vector_empty;
      |             ^
c:/Users/ChieltenBrinke/prj/koka/kklib/include\kklib/vector.h(23,41): note: previous definition is here
   23 | static inline kk_decl_const kk_vector_t kk_vector_empty(void) {
      |                                         ^
.koka/v3.1.3/clang-cl-debug-29abf8/vector.c(35,21): error: non-object type 'kk_vector_t ()' (aka 'kk_datatype_s ()') is
      not assignable
   35 |     kk_vector_empty = kk_vector_empty(); /*forall<a> vector<a>*/
      |     ~~~~~~~~~~~~~~~ ^
.koka/v3.1.3/clang-cl-debug-29abf8/vector.c(47,3): error: no matching function for call to 'kk_vector_drop'
   47 |   kk_vector_drop(kk_vector_empty, _ctx);
      |   ^~~~~~~~~~~~~~
c:/Users/ChieltenBrinke/prj/koka/kklib/include\kklib/vector.h(36,20): note: candidate function not viable: no known
      conversion from 'kk_vector_t ()' (aka 'kk_datatype_s ()') to 'kk_vector_t' (aka 'kk_datatype_s') for 1st argument
   36 | static inline void kk_vector_drop(kk_vector_t v, kk_context_t* ctx) {
      |                    ^              ~~~~~~~~~~~~~
4 errors generated.
vector(1, 1): build error: user error (error  : command failed (exit code 1)
command: "c:/Program Files/LLVM/bin/clang-cl.exe" -DWIN32 -nologo -Wno-everything -Wall -Wextra -Wpointer-arith -Wshadow -Wstrict-aliasing -Wno-unknown-pragmas -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-variable -Wno-unused-value -Wno-cast-qual -Wno-undef -Wno-reserved-id-macro -Wno-unused-macros -Wno-cast-align -Wno-extra-semi-stmt -Wno-extra-semi -Wno-float-equal -MDd -Zi -FS -O1 -D__clang_msvc__ -EHs -TP -c -I c:/Users/ChieltenBrinke/prj/koka/kklib/include -DKK_MIMALLOC=8 -Fo.koka/v3.1.3/clang-cl-debug-29abf8/vector.obj .koka/v3.1.3/clang-cl-debug-29abf8/vector.c)

Failed to compile vector.kk
@TimWhiting
Copy link
Collaborator

Unfortunately this is a pretty hard thing to fix.

We don't want to mangle all the names in the program, because that would make C interop harder. At the same time, we do have some c definitions already. So you just happened to give your function a name that translates to kk_vector_empty (kk + file name + definition name). which is already defined in our core libraries for use with the unit/vector constructor:

// Create an empty vector.
pub inline extern unit/vector : forall<a> () -> vector<a>
  c inline "kk_vector_empty()"
  cs inline "new ##1[0]"
  js inline "[]"

To fix this we have a few options:

  • we rely on people parsing the C error message (do nothing),
  • we have to keep a list of all the C definitions in our core libraries and dependencies in the compiler and throw a message,
  • or we have to adopt a different naming system for core libraries versus for user generated code

Both of the last two are possible, but would require lots of renaming and fixing our extern definitions, or a large initial effort to document all C definitions, and a continual effort to keep that list maintained. (Also we have no guarantee that other people won't add their own C libraries and extern definitions that might conflict).

@daanx
Copy link
Member

daanx commented Oct 22, 2024

... I guess we need to prevent clashes with the runtime defined functions; maybe generate kk__xxx for user functions, or rename all primitives to kkp_xxx; I am embarrassed to not have addressed this sooner. I think changing the generated C is easiest but I'll ponder a bit on it.

@TimWhiting
Copy link
Collaborator

I think changing the generated C code is probably the best option.

We also should probably document the naming / calling convention for generated code and possibly allow some sort of function modifier or annotation to use a different raw name / (and possibly calling convention - for example to allow passing to a C function pointer without requiring a ctx parameter). This will make C library integration easier.

@anfelor
Copy link
Collaborator

anfelor commented Oct 23, 2024

How would our name generation work once Koka gets a proper package manager? I could imagine that several packages will have the same modules and identifiers in them. To avoid the same error in that case, we could prepend each name with the name of its package. The identifier in our standard library would then get a uniform prefix like kk_base_ that distinguishes them from identifiers in user projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants