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

Adding hashmap and map initialization functions with data #1402

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

chamaeleon
Copy link
Contributor

@chamaeleon chamaeleon commented Sep 2, 2024

Adding some functions to initialize HashMap and Map structures with data.

def StringHashMap = HashMap(<String, int>);
def IntHashMap = HashMap(<int, String>);
def StringMap = Map(<String, int>);
def IntMap = Map(<int, String>);

int[] keys = {1, 2, 3};
String[] values = {"one", "two", "three"};

fn void main(String[] args)
{
    StringHashMap m1;
    IntHashMap m2;

    m1.new_init_with_keyvalues("abc", 123, "def", 456, "ghi", 789);
    defer m1.free();
    m1.@each(; String k, int v) { io::printf("%s -> %s\n", k, v); };

    m2.new_init_with_arrays(keys, values);
    defer m2.free();
    m2.@each(; int k, String v) { io::printf("%s -> %s\n", k, v); };

    StringMap m3;
    IntMap m4;

    m3 = map::new_with_keyvalues(<String, int>)("abc", 123, "def", 456, "ghi", 789);
    defer m3.free();
    m3.@each(; String k, int v) { io::printf("%s -> %s\n", k, v); };

    m4 = map::new_with_arrays(<int, String>)(keys, values);
    defer m4.free();
    m4.@each(; int k, String v) { io::printf("%s -> %s\n", k, v); };
}

@lerno
Copy link
Collaborator

lerno commented Sep 3, 2024

Can we do the following changes:

  1. The variant with keys and values as list, call that new_init_from_keys_and_values
  2. The one with key, value: new_init_with_key_values
  3. Check that the lists are the same length in the contract for (1)
  4. Check that $vacount is even for (2)

@chamaeleon
Copy link
Contributor Author

chamaeleon commented Sep 3, 2024

I'm running into a bit of a strange issue when I rename the functions, in the form of some kind of name clash. When I rename HashMap.new_init_with_keyvalues and map::new_with_keyvalues to HashMap.new_init_with_key_values and map::new_init_keys_and_values, I am greeted by

PS C:\Projects\C3\playground> c3c run
27:     StringMap m3;
28:     IntMap m4;
29:
30:     m3 = map::new_init_with_key_values(<String, int>)("abc", 123, "def", 456, "ghi", 789);
                  ^^^^^^^^^^^^^^^^^^^^^^^^
(C:/Projects/C3/playground/src/main.c3:30:15) Error: The macro 'map::new_init_with_key_values' is defined in both 'std::collections::map' and 'std::collections::map', please use either std::collections::map::new_init_with_key_values or std::collections::map::new_init_with_key_values to resolve the ambiguity.

If I temporarily change either HashMap.new_init_with_key_values or the macro new_init_with_key_values to something else (and the calling code accordinly), the collision disappear. As to why the message seemingly saying it's colliding with itself, I'm not sure.

I can replicate this in this manner:
main.c3:

module playground;
import std::io;
import playground::bug;

def MyFoo = Foo(<int>);

fn void main(String[] args)
{
    MyFoo foo = { 123 };
    foo.print_it();
    bug::print_it(<int>)();
}

foo.c3

module playground::bug(<Ty>);
import std::io;

struct Foo
{
    Ty x;
}

fn void Foo.print_it(&self)
{
    io::printf("Method %s\n", self.x);
}

bar.c3

module playground::bug(<Ty>);
import std::io;

macro void print_it(...)
{
    io::printn("Macro 456");
}

Result is

PS C:\Projects\C3\playground> c3c run
 46: {
 47:     MyFoo foo = { 123 };
 48:     foo.print_it();
 49:     bug::print_it(<int>)();
              ^^^^^^^^
(C:/Projects/C3/playground/src/main.c3:49:10) Error: The macro 'bug::print_it' is defined in both 'playground::bug' and 'playground::bug', please use either playground::bug::print_it or playground::bug::print_it to resolve the ambiguity.

If I remove the generic part of module playground::bug(<Ty>) and make it module playground::bug in both files and change the struct to have an int explicitly, there is no name collision.

PS C:\Projects\C3\playground> c3c run
Launching build\playground.exe
Method 123
Macro 456
Program completed with exit code 0.

I realize this could very well deserve a bug report and I can certainly file one with the small test case.

@lerno
Copy link
Collaborator

lerno commented Sep 4, 2024

Do you need to have those in different files, or can you reproduce it with module sections, so basically pasting everything in a single file one after the other?

@chamaeleon
Copy link
Contributor Author

Single file seems to reproduce it as well.

module playground::bug(<Ty>);
import std::io;

struct Foo
{
    Ty x;
}

fn void Foo.print_it(&self)
{
    io::printf("Method %s\n", self.x);
}

module playground::bug(<Ty>);
import std::io;

macro void print_it(...)
{
    io::printn("Macro 456");
}

module playground;
import playground::bug;

def MyFoo = Foo(<int>);

fn void main(String[] args)
{
    MyFoo foo = { 123 };
    foo.print_it();
    bug::print_it(<int>)();
}
PS C:\Projects\C3\playground> c3c run
28: {
29:     MyFoo foo = { 123 };
30:     foo.print_it();
31:     bug::print_it(<int>)();
             ^^^^^^^^
(C:/Projects/C3/playground/src/main.c3:31:10) Error: The function 'bug::print_it' is defined in both 'playground::bug' and 'playground::bug', please use either playground::bug::print_it or playground::bug::print_it to resolve the ambiguity.

@chamaeleon
Copy link
Contributor Author

If I remove the generic aspect it works for the single file version (and that behavior is the same for the multi file version).

module playground::bug;
import std::io;

struct Foo
{
    int x;
}

fn void Foo.print_it(&self)
{
    io::printf("Method %s\n", self.x);
}

module playground::bug;
import std::io;

macro void print_it(...)
{
    io::printn("Macro 456");
}

module playground;
import playground::bug;

def MyFoo = Foo;

fn void main(String[] args)
{
    MyFoo foo = { 123 };
    foo.print_it();
    bug::print_it();
}
PS C:\Projects\C3\playground> c3c run
Launching build\playground.exe
Method 123
Macro 456
Program completed with exit code 0.

lerno added a commit that referenced this pull request Sep 4, 2024
@lerno lerno added Bug Something isn't working Fixed Needs Verification Fixed, but needs verification that it works labels Sep 4, 2024
@lerno lerno self-assigned this Sep 4, 2024
@lerno
Copy link
Collaborator

lerno commented Sep 4, 2024

Please try it now, it ought to work.

@chamaeleon
Copy link
Contributor Author

Good news/bad news.

Good news, the changes to c3c makes the single-file sample code work (like your added test case). Follow-up to this. Is it desirable that a macro can shadow a function with the same name?

Bad news, I was looking at too many functions at a time, and mistook my first problem to be between a fn definition and a macro definition. In reality, the actual HashMap and Map code are a method and a function, respectively. Here is a very similar code sample showing the issue, like for the macro conflict.

module playground::foo(<Ty>);
import std::io;
struct Foo { Ty x; }
fn void Foo.testing(&self)
{
    io::printn("method");
}

module playground::foo(<Ty>);
import std::io;
fn void testing(...)
{
    io::printn("function");
}

module playground;
import playground::foo;

def MyFoo = Foo(<int>);

fn void main(String[] args)
{
    Foo foo;
    foo.testing();
    MyFoo.testing(&foo);
    foo::testing(<int>)();
}
PS C:\Projects\C3\playground> c3c run
23:     Foo foo;
24:     foo.testing();
25:     MyFoo.testing(&foo);
26:     foo::testing(<int>)();
             ^^^^^^^
(C:/Projects/C3/playground/src/main.c3:26:10) Error: The function 'foo::testing' is defined 
in both 'playground::foo' and 'playground::foo', please use either playground::foo::testing 
or playground::foo::testing to resolve the ambiguity.

The desired call would be to print function, if this is supposed to work at all. If I comment it out, both versions of calling the method will print method.

What had intended to ask in my first comment showing the original problem was whether the _init part should be part of the names in the functions and macros in map.c3, as I don't see that for other new and temp functions there (only the _init() function contains that, none of the functions that call it do, like new() and temp(), unlike hashmap.c3). If I were to remove that part from my function names, there would be no conflict. However, then we're avoiding the problem of whether the same function name should be possible to use as a method in a generic module as well as a plain function in the same generic module or not.

@lerno
Copy link
Collaborator

lerno commented Sep 4, 2024

I thought right, but just had a total brain meltdown when I made the "solution".

@lerno
Copy link
Collaborator

lerno commented Sep 4, 2024

It's fixed now from what I can tell, and I also fixed another issue as well.

@chamaeleon
Copy link
Contributor Author

It works successfully now for the real case and my test case to narrow down the issue. However, I'm unhappy with how my updated commit happened here which pulled in your changes and makes it look like I added a bunch more when most of it are your changes. :( Let me know if you want me to redo something.

@lerno
Copy link
Collaborator

lerno commented Sep 5, 2024

You could rebase on origin-master and use squash on all your changes, then force push that to you map-init- branch. That should remove all duplicate changes.

This is my method when I run into something and it's turned out to be very simple and safe.

@chamaeleon
Copy link
Contributor Author

I have hopefully fixed up the branch now. I see some build errors in CI, but I don't think it's related. Some CMake issue with finding curl stuff.

@chamaeleon
Copy link
Contributor Author

I have a lot to learn still about the language.

@ is required on macros that use # and & parameters.

Should I therefore make one more change to the names of my macros that takes keys and values using $vaargs when &self`` is used in the beginning (HashMap.new_init_with_key_values and HashMap.temp_init_with_key_values does not start with @...)

Sorry again for the need to clean up the branch with squash

@lerno lerno merged commit 04c85eb into c3lang:master Sep 6, 2024
35 of 41 checks passed
@lerno
Copy link
Collaborator

lerno commented Sep 6, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Fixed Needs Verification Fixed, but needs verification that it works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants