Skip to content

Commit af86097

Browse files
vifergaviferga
committed
Solve duplicated names bug, at least for same loader, it needs more testing.
1 parent bd987a3 commit af86097

File tree

7 files changed

+58
-19
lines changed

7 files changed

+58
-19
lines changed

‎source/loader/source/loader_impl.c‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -629,9 +629,12 @@ int loader_impl_load_from_file(loader_impl impl, const loader_naming_path paths[
629629
{
630630
if (interface_impl->discover(impl, handle_impl->module, handle_impl->ctx) == 0)
631631
{
632-
// TODO: Check if is contained in the context
633-
// if (context_contains(impl->ctx, handle_impl->ctx) == 0 && context_append...)
634-
if (context_append(impl->ctx, handle_impl->ctx) == 0)
632+
if (context_contains(impl->ctx, handle_impl->ctx) == 0)
633+
{
634+
/* TODO: This still does not protect duplicated names between different loaders global scope */
635+
log_write("metacall", LOG_LEVEL_ERROR, "There are duplicated symbols already loaded in the global scope conflicting with handle: %s", module_name);
636+
}
637+
else if (context_append(impl->ctx, handle_impl->ctx) == 0)
635638
{
636639
static const char func_init_name[] = LOADER_IMPL_FUNCTION_INIT;
637640

@@ -734,9 +737,12 @@ int loader_impl_load_from_memory(loader_impl impl, const char *buffer, size_t si
734737
{
735738
if (interface_impl->discover(impl, handle_impl->module, handle_impl->ctx) == 0)
736739
{
737-
// TODO: Check if is contained in the context
738-
// if (context_contains(impl->ctx, handle_impl->ctx) == 0 && context_append...)
739-
if (context_append(impl->ctx, handle_impl->ctx) == 0)
740+
if (context_contains(impl->ctx, handle_impl->ctx) == 0)
741+
{
742+
/* TODO: This still does not protect duplicated names between different loaders global scope */
743+
log_write("metacall", LOG_LEVEL_ERROR, "There are duplicated symbols already loaded in the global scope conflicting with handle: %s", name);
744+
}
745+
else if (context_append(impl->ctx, handle_impl->ctx) == 0)
740746
{
741747
static const char func_init_name[] = LOADER_IMPL_FUNCTION_INIT;
742748

@@ -808,9 +814,12 @@ int loader_impl_load_from_package(loader_impl impl, const loader_naming_path pat
808814
{
809815
if (interface_impl->discover(impl, handle_impl->module, handle_impl->ctx) == 0)
810816
{
811-
// TODO: Check if is contained in the context
812-
// if (context_contains(impl->ctx, handle_impl->ctx) == 0 && context_append...)
813-
if (context_append(impl->ctx, handle_impl->ctx) == 0)
817+
if (context_contains(impl->ctx, handle_impl->ctx) == 0)
818+
{
819+
/* TODO: This still does not protect duplicated names between different loaders global scope */
820+
log_write("metacall", LOG_LEVEL_ERROR, "There are duplicated symbols already loaded in the global scope conflicting with handle: %s", package_name);
821+
}
822+
else if (context_append(impl->ctx, handle_impl->ctx) == 0)
814823
{
815824
static const char func_init_name[] = LOADER_IMPL_FUNCTION_INIT;
816825

‎source/reflect/include/reflect/reflect_context.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ REFLECT_API scope context_scope(context ctx);
4141

4242
REFLECT_API int context_append(context dest, context src);
4343

44+
REFLECT_API int context_contains(context dest, context src);
45+
4446
REFLECT_API int context_remove(context dest, context src);
4547

4648
REFLECT_API void context_destroy(context ctx);

‎source/reflect/include/reflect/reflect_scope.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ REFLECT_API value scope_undef(scope sp, const char *key);
5252

5353
REFLECT_API int scope_append(scope dest, scope src);
5454

55+
REFLECT_API int scope_contains(scope dest, scope src);
56+
5557
REFLECT_API int scope_remove(scope dest, scope src);
5658

5759
REFLECT_API size_t *scope_stack_return(scope sp);

‎source/reflect/source/reflect_context.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ int context_append(context dest, context src)
8787
return scope_append(dest->sp, src->sp);
8888
}
8989

90+
int context_contains(context dest, context src)
91+
{
92+
return scope_contains(dest->sp, src->sp);
93+
}
94+
9095
int context_remove(context dest, context src)
9196
{
9297
return scope_remove(dest->sp, src->sp);

‎source/reflect/source/reflect_scope.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -454,17 +454,14 @@ value scope_undef(scope sp, const char *key)
454454

455455
int scope_append(scope dest, scope src)
456456
{
457-
/* TODO: ¿Implement name mangling and/or scope of functions per handle and remove this? */
458-
if (set_contains_any(dest->objects, src->objects) == 0)
459-
{
460-
log_write("metacall", LOG_LEVEL_ERROR, "One or more objects are already defined in the scope");
461-
462-
return 1;
463-
}
464-
465457
return set_append(dest->objects, src->objects);
466458
}
467459

460+
int scope_contains(scope dest, scope src)
461+
{
462+
return set_contains_any(dest->objects, src->objects);
463+
}
464+
468465
int scope_remove(scope dest, scope src)
469466
{
470467
return set_disjoint(dest->objects, src->objects);

‎source/tests/metacall_duplicated_symbols_test/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Check if this loader is enabled
2-
if(NOT OPTION_BUILD_LOADERS OR NOT OPTION_BUILD_LOADERS_RB OR NOT OPTION_BUILD_SCRIPTS OR NOT OPTION_BUILD_SCRIPTS_RB)
2+
if(NOT OPTION_BUILD_LOADERS OR NOT OPTION_BUILD_LOADERS_RB OR NOT OPTION_BUILD_LOADERS_PY OR NOT OPTION_BUILD_SCRIPTS OR NOT OPTION_BUILD_SCRIPTS_RB)
33
return()
44
endif()
55

@@ -128,6 +128,7 @@ add_test(NAME ${target}
128128
#
129129

130130
add_dependencies(${target}
131+
py_loader
131132
rb_loader
132133
)
133134

‎source/tests/metacall_duplicated_symbols_test/source/metacall_duplicated_symbols_test.cpp‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,29 @@ TEST_F(metacall_duplicated_symbols_test, DefaultConstructor)
3434

3535
ASSERT_EQ((int)0, (int)metacall_initialize());
3636

37+
/* Python */
38+
#if defined(OPTION_BUILD_LOADERS_PY)
39+
{
40+
static const char bufferA[] =
41+
"#!/usr/bin/env python3\n"
42+
"def multmem(left: int, right: int) -> int:\n"
43+
"\tresult = left * right;\n"
44+
"\tprint(left, ' * ', right, ' = ', result);\n"
45+
"\treturn result;";
46+
47+
static const char bufferB[] =
48+
"#!/usr/bin/env python3\n"
49+
"def multmem(left: int, right: int) -> int:\n"
50+
"\tresult = left * right;\n"
51+
"\tprint(left, ' * ', right, ' = ', result);\n"
52+
"\treturn result;";
53+
54+
EXPECT_EQ((int)0, (int)metacall_load_from_memory("py", bufferA, sizeof(bufferA), NULL));
55+
56+
EXPECT_EQ((int)1, (int)metacall_load_from_memory("py", bufferB, sizeof(bufferB), NULL));
57+
}
58+
#endif /* OPTION_BUILD_LOADERS_PY */
59+
3760
/* Ruby */
3861
#if defined(OPTION_BUILD_LOADERS_RB)
3962
{
@@ -49,7 +72,7 @@ TEST_F(metacall_duplicated_symbols_test, DefaultConstructor)
4972

5073
EXPECT_EQ((int)0, (int)metacall_load_from_file("rb", rb_ducktype_scripts, sizeof(rb_ducktype_scripts) / sizeof(rb_ducktype_scripts[0]), NULL));
5174

52-
EXPECT_NE((int)0, (int)metacall_load_from_file("rb", rb_second_scripts, sizeof(rb_second_scripts) / sizeof(rb_second_scripts[0]), NULL));
75+
EXPECT_EQ((int)1, (int)metacall_load_from_file("rb", rb_second_scripts, sizeof(rb_second_scripts) / sizeof(rb_second_scripts[0]), NULL));
5376
}
5477
#endif /* OPTION_BUILD_LOADERS_RB */
5578

0 commit comments

Comments
 (0)