| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
| |
Not sure if we should go this route or wrap in a callback, I'd prefer
if we can go this route to avoid adding JS wrapping
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The callback trampoline is in fact a GClosure, but we were just composing
one. We can instead just inherit from it now, given that the
Gjs::Closure will allocate the needed memory via the closure allocator
and handle the references via native closure reference system (avoid to
duplicate that).
As per this, some less memory used as we save a pointer and the
reference counting.
The only bit we need to handle is the destruction as we need to be sure
to call the proper type destructor on closure finalization.
So, adding an utility function that ensures this happens for each type
and that we won't call the base destructor twice.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
To create closures we used to create an internal closure structure that
for which we were handling construction and destruction manually.
C++ allows us to use some nicer features though, and we can take
advantage of them once we leave the role of allocating and deallocating
the memory to GClosure functions.
This can be nicely done overriding the new and delete operators that
will give us the size to allocate exactly as the closure creator
function expects to receive.
|
| |
|
|
|
|
|
|
|
| |
Add a simple structure that wraps GValue that allows us handling
lifetime of GValue's in a nicer way and redefine AutoGValueVector
with it transparently.
In this way the allocated data keeps being structured as GValues but we
handle its initializations without errors.
|
| | |
|
| |
|
|
| |
This is a prepartory work to get the unity builds possible
|
| |
|
|
|
|
| |
This was a more recent change than the branch that was just merged.
Unreviewed, pushing to fix build.
|
| |\
| |
| |
| |
| |
| |
| | |
arg-cache: Never throw when building the argument cache
Closes #386
See merge request GNOME/gjs!590
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, the methods for building the argument cache for a function
could fail, and throw an exception. Normally this was not a problem, since
the argument cache was built when creating the Function object, and that
was generally created right before it was about to be called.
However, static methods are created eagerly when defining a type's
constructor, so if a type contained a method that would throw when
building the argument cache, then merely resolving that type would throw.
(For example, resolving `GLib.ByteArray` would throw.)
Instead, change all the argument cache building methods to be infallible.
If an argument is not supported in GJS, then its in-marshaller throws
instead. This causes the error to happen when the Function object is
called, not when it is created.
In order to give good exception messages, we add a "reason" field to the
GjsArgumentCache union, used by the new gjs_marshal_not_introspectable_in
marshaller, and a display_name() method to GjsFunctionCallState.
Closes: #386
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
While it's true that a void function has no return value, this is not
true in the case it's a void pointer.
Not doing this, will cause a crash at ffi level because we're passing to
it a null return value pointer, while it's expecting to fill it with
the returned data.
Fixes: #406
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In some cases we're handing interfaces with unsed GType, a case is
GTypeInstance. When this happen we should rely on fallback marshaller
and try to figure out the actual C container for this type getting the
gtype from the JS object.
This allows to use g_value_init_from_instance() correctly between the
others cases.
|
| |/
|
|
|
|
|
| |
cppcheck is not smart enough to determine that when we save an allocated
pointer plus an offset, and then free the saved pointer minus that offset,
it amounts to the same thing. But, maybe it is clearer anyway to save the
original pointers and add the offset in an accessor.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Make the wrapper use the underlying type on Visual Studio builds, so
that we can pinpoint on the "right" bitwise operator to use on Visual
Studio builds, for the class enumeration(s) we may use.
Also, on the same token, define casts for Visual Studio for the wrapped
types for use on the '|=' and '&=' operators, as well as the '&' and '|'
operators that will be used in value assignments so that the compiler
does not get upset when converting between the underlying type and the
class enumeration type.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
When computing an array length to pass to a function call we bother
some JSapi code to get a value (that also may be of an incorrect type)
while we already compute this value into the arg-cache depending on the
GIArgument type.
Since this is the correct way of doing this and avoids lots of unneeded
checks, let's just expose the function and reuse it.
This is also something that will be quite useful to support BigInt as in
such case we may create objects instead of numbers, causing this to
fail.
|
| |
|
|
|
|
|
|
| |
Make GjsFunctionCallState a bit smarter by saving the bits there and
adding few constexpr methods to verify common conditions.
This will make possible also to split the function calls so that we can
avoid using some C-isms (goto!)
|
| |
|
|
|
|
|
|
|
|
| |
Oops, we accidentally made the IWYU mapping file invalid, so the IWYU job
was not functioning correctly. Move the workaround into the postprocessing
script, and fix all the include fixes that have been missed in the
meantime while this was broken.
New code has also exposed some false positives in IWYU, so add those
exceptions to the false positive list.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
If an array-length parameter precedes the corresponding array parameter,
we skip counting the latter instead of the former. As the parameter type
isn't relevant for the argument counter, that works fine - provided that
the length parameter is only used for a single array.
That is usually the case, but there are exceptions (for example Gtk4's
gtk_accessible_update_property_value()). Address this by only skipping
array arguments if the corresponding length parameter wasn't already
marked skip-all previously.
|
| |\
| |
| |
| |
| |
| |
| | |
gi: Support caller-allocates GValue arguments
Closes #74
See merge request GNOME/gjs!496
|
| | |
| |
| |
| |
| | |
There won't be any case in which a string can be unsigned, or when a
number is a filename, so we can reuse the same bit safely.
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When we were handling a GValue caller-allocates argument we actually
were changing a new temporary GArgument copy that we actually didn't end
up passing to the function that was changing it.
To ensure that this doesn't happen:
- When a caller-allocates argument is passed to a function, use the
actual GIArgument pointer and not deference it.
- Include an argument flag that can be used at conversion time to make
clear that we're working on the actual caller allocated argument and
not in a gjs copy of it.
- When converting a JSValue to GValue, act on the caller allocated
(possibly by arg-cache) memory, instead of setting a new GValue.
As per this we can enable the `vfunc_caller_allocated_out_parameter`
test now.
This doesn't seem enough to fix GNOME/gjs#74 testcase though as the gtk
introspection doesn't give us enough information about being
caller-allocated.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Make arguments more readable and easier to extend when it comes to pass
flags to such functions, instead of rely on hard-to-maintain booleans.
Using an enum class that needs some more redefinitions but gives more
type safety, as per this adding a enum-utils.h header with some smarter
templated definitions for operators that match all the enum class types.
We automatically define operators for Enum class (only if strongly typed)
types, just include this header and the magic will happen now.
Internally using a wrapper struct to wrap the return values so that we
can define the bool() operator and so no need to use the !! operator
anymore for simple true checks.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
| |
As found in https://gitlab.gnome.org/GNOME/gjs/-/issues/361, getting the
GType using g_registered_type_info_get_g_type() is very expensive and
can make up for a significant part of the overhead when calling into a C
function.
Since the argument cache seems to be fairly small in a typical
gnome-shell session (about 1300 entries), the total increased memory
usage of about 10kB seems very reasonable given the benefits of the
caching.
This reverts commit adfb7dc3ba79b17b0cefa47f5249e9da917217ad.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we create a GValue using the actual constructor we're holding
internally a boxed type, and in such case we don't have to try to
convert it to another GValue to pass around, but we should just use it
as our argument.
However, this can't be computed when parsing the argument, but depends
case-by-case, as per this when we've a GValue and this is an object
containing an actual GValue, pass it around and remember that we've not
to free the returned argument, as that will be handled by the boxed
wrapper.
This allows to finally use some Gtk functions to get GValue parameters
as "in/out" parameters, and the gvalue copy function.
Add tests checking that this is the case.
Fixes #74
|
| |
|
|
|
|
|
| |
If our argument is a GValue and it's marked as the instance parameter,
then it means we're calling a g_value_* function, and as per this we
should not pass a temporary value but we can just pass the object we've
initialized locally.
|
| | |
|
| |
|
|
|
|
| |
Now that we rely on static compiler checks for type safety and
conversions, we can use just one function to handle the conversion of
all the numerc types as the compiler will do the job for us.
|
| |
|
|
|
|
| |
Addresses regression where fundamental objects cannot be passed into functions.
Fixes #353
|
| |\
| |
| |
| |
| | |
Prevent passing null pointers when not nullable.
See merge request GNOME/gjs!503
|
| | |
| |
| |
| | |
The arg-cache marshaller for null (pointers) doesn't check if the argument is nullable. This is a regression introduced by the arg-cache that can cause segfaults in functions such as GLib.str_hash.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
The SPDX format is machine-readable, which should make it much easier to
avoid mistakes with licensing and copyright using automated tools.
This commit does not add any implicit information. It converts exactly
what was already specified into SPDX format comments.
|
| |/
|
|
|
|
|
|
|
|
|
| |
We already have a function to set a GArgument from the JS value, taking care
of number conversions and out of bound errors, that is used in arg in the
same way, so let's use this function to be consistent, ensuring that we can
save each type in a container type big enough and that, in case we go out of
bounds we can handle this correctly.
Only use a simple function override, so that we can handle the out of
bound errors the same way we used to do.
|
| |
|
|
|
|
|
| |
Make the struct a bit smarter, including auto pointers and methods to
access to the internal values.
Still manage the object creation and reference counting externally.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
In arg-cache we add a new reference to the trampoline that is going to
be removed on DestroyNotify callback or(?) when the async calls are
cleared. However this is quite unclear as it picks multiple types of
function scopes.
So put all together adding a ref and using a lambda function to unref,
so that we have a more readable and understandable code.
This fixes also a potential issue when a function is both async and has
a callback destroy, as in such case we'd just add one reference, while
we'd remove two.
|
| | |
|
| | |
|
| |\
| |
| |
| |
| | |
cleanup: Don't use GSlice anywhere
See merge request GNOME/gjs!488
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
GSlice API is going to be deprecated [1] and marked as such for some
years now, even if this change didn't happen yet, there's no point to
keep GSlice usage in gjs, considering that we can semplificate it with
calls to system allocator using g_new/g_malloc and g_free.
Once I applied this change, I've noticed that the memory sanitzer found
some leaks in the trampoline code that for some reason GSlice was hiding
and that this commit underlines.
[1] https://gitlab.gnome.org/GNOME/glib/-/issues/1079
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previous of commit aa28329b7 we were assuming that all the
caller-allocates function were using an interface tag, that was wrong
but at the same time we don't yet support any other type of
caller-allocats function yet [1], so if we detect one type that we don't
support we should just throw with some valuable error as we used to do and
not just crash.
Otherwise it would be impossible to test a solution for bug [2].
[1] https://gitlab.gnome.org/GNOME/gjs/-/issues/106
[2] https://gitlab.gnome.org/GNOME/gjs/-/issues/344
|
| |/
|
|
|
|
|
|
|
|
|
| |
When we get an out parameter with caller-allocates flag we assume that
it's an interface, but this may not be the case like for array out
parameters.
As per this, verify that the type tag matches the interface even when
we've a caller allocates argument.
Closes: https://gitlab.gnome.org/GNOME/gjs/-/issues/344
|
| |\
| |
| |
| |
| |
| |
| | |
arg-cache: Always use an unsigned int mask for flags
Closes #341
See merge request GNOME/gjs!477
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When retrieving flags from the GIArgumentInfo in
set_return_ffi_arg_from_giargument(), we use the v_int member of the
union (arguably we ought to use the v_uint member for flags, but we
don't), so to avoid undefined behaviour from type-punning we need to
store flags via that same member.
In particular, even if we somehow convince ourselves that a flags type
can have values outside the 32-bit range, it would be incorrect to
store them in the v_int64 or v_uint64 member, because when we read
them back out it'll be via the v_int member. That's formally undefined
behaviour, but in practice will access the first 32 bits of the 64-bit
integer, which is the same as (real_value & 0xffff'ffff) on
little-endian CPUs, but gives a meaningless result on big-endian.
Sure enough, after 1f3984c9 the unit tests start failing on s390x,
which is 64-bit big-endian.
GLib's GFlagsClass.mask and GFlagsValue.value are of type guint
(equivalent to unsigned int), so it can't possibly represent flags
outside the range of guint as a GType anyway. To be consistent with
that (and have reasonable unsigned arithmetic), go via an unsigned
integer.
The corresponding fix for enum types was done as a side-effect of
8b2927b0 "arg-cache: Save space in enum bounds".
Fixes: 1f3984c9 "arg-cache: extend to handle interface types too"
Resolves: https://gitlab.gnome.org/GNOME/gjs/-/issues/341
Signed-off-by: Simon McVittie <smcv@debian.org>
|
| | | |
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The size value we get may be in some cases a signed integer and in some others
an unsigned one. However in case we're using a little endian
architecture we always save it as the GIArgument unsigned value, while
we read it depending on the type tag.
This seems to work properly in all the architectures, but the assumtion
is wrong in armhf, causing a test failure:
it('marshals an array with a 64-bit length parameter', function () {
expect(() => GIMarshallingTests.array_in_guint64_len([-1, 0, 1, 2]))
.not.toThrow();
});
ERROR:/usr/share/gobject-introspection-1.0/tests/gimarshallingtests.c
gi_marshalling_tests_array_in_guint64_len:
assertion failed (length == 4): (-498216206332 == 4)
At the end the switch check is just an extra operation that will ensure
us safety, at a really minimal cost at this point (given we've got the tag
anyways).
Closes: #342
|
| |
|
|
|
|
|
| |
I mistakenly left this out during a rebase of the argument cache
patches.
Closes: #338
|
| |
|
|
|
|
|
|
| |
Unfortunately using designated initializers in C++ is a GCCism, which is not
allowed in non-GCC C++17, so go back to the traditional way to set up the
structures...
See: https://en.cppreference.com/w/cpp/language/aggregate_initialization
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Now that we depend on C++17, we can use these attributes that are part
of the language, instead of relying on macros to make them portable.
(GJS_USE still needs to be defined for public header files, which may be
compiled in C.)
Attributes which are new in C++17 are [[maybe_unused]], [[nodiscard]],
and [[fallthrough]].
[skip cpplint] because cpplint doesn't deal with C++ attributes:
https://github.com/google/styleguide/issues/165
|
| |
|
|
|
|
|
|
|
|
| |
The four marshaller function pointers cannot be just any functions.
There are a relatively small number of configurations of marshallers, so
it saves space to write them all out explicitly, and store only one
pointer in the argument cache instead of four.
It also prevents accidentally leaving a control flow path where one of
the marshaller function pointers remains unset.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The GType is fetchable from the GIRegisteredTypeInfo, and it is the only
member keeping the GjsArgumentCache struct from being smaller. However,
this is a bit of a tradeoff since g_registered_type_info_get_g_type()
involves a call to g_module_symbol().
Hopefully this doesn't have much of an effect on performance, compared
to the net benefit of introducing the argument cache in the first place,
since g_registered_type_info_get_type() is also used in the "old way" of
marshalling values in arg.cpp.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
We were storing a 64-bit int for the min and max values of an enum type.
However, the documentation of gobject-introspection notes that enum
values are always representable in 32 bits, either signed or unsigned,
and the use of a 64-bit return type is to easily allow for both.
We store the bounds in 32-bit unsigned bitfields, and use a flag that we
add to the slop space in GjsArgumentCache (which we can share with
number types as well) to indicate whether the bounds should be
interpreted as signed or unsigned.
|
