Re: [C++ PATCH] Partially fix designated-initializer-list handling in overload resolution (PR c++/71446)
On Fri, Mar 01, 2019 at 04:14:33PM -0500, Jason Merrill wrote:
> > Note, this is just a partial fix for the PR, for the second part (making
> > struct S { void *a; int b; };
> > void foo (S);
> > ... foo ({.b = 1}) work), I'm afraid I don't really understand what the C++
> > standard wants to say in http://eel.is/c++draft/over.ics.list#2 and what
> > build_aggr_conv should do, such that the cpp2a/desig{4,6}.C testcases taken
> > from the standard as well as the desig11.C in the earlier version of the
> > patch (would need to be renamed to desig12.C) can pass.
>
> It seems to me that for designated initializers, we should make sure that
> all the designators match fields, and otherwise check convertibility like
> build_aggr_conv already does.
For the simple standard conforming case when if any initializer clause has
designator, then all of them have to I can imagine e.g. one loop
where we e.g.
if (CONSTRUCTOR_IS_DESIGNATED_INIT (ctor))
{
tree idx, val;
FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, idx, val)
{
if (idx && TREE_CODE (idx) == FIELD_DECL)
{
tree ftype = TREE_TYPE (idx);
if (TREE_CODE (ftype) == ARRAY_TYPE
&& TREE_CODE (val) == CONSTRUCTOR)
ok = can_convert_array (ftype, val, flags, complain);
else
ok = can_convert_arg (ftype, TREE_TYPE (val), val, flags,
complain);
if (!ok)
return NULL;
// Put idx into some hash_set
}
}
}
and then in the current
for (; field; field = next_initializable_field (DECL_CHAIN (field)))
loop for the if (CONSTRUCTOR_IS_DESIGNATED_INIT (ctor)) case assume all has
been done if field is in the hash_set (with the slightly more complicated
case of ANON_AGGR_TYPE_P (ftype) where we likely need to search recursively
for any initialized field) and just go the missing initializer way
otherwise. But it isn't clear to me what to do in the case where
the initializer has some designators and doesn't have others, because
then the order is significant, the initializers without designator are
supposed to follow those that do have them.
struct S { int a, b, c, d, e; };
void foo (S);
void bar ()
{
foo ({.d = 5, 6, .b = 2, 3});
}
or
struct T { int a, b; };
void baz (T);
void qux ()
{
baz ({.b = 1, .a = 2});
}
clang++ currently accepts both, but I believe it doesn't claim to supported
the C++2a designated initializer, just an extension which likely allowed
reordering like C99 and maybe that is why the C++2a standard says the
ordering doesn't matter?
I'm not really sure what to do for foo. Perhaps if we find that case just
require that the order is ok already during build_aggr_conv and fail the
conversion otherwise? We are outside of the standard in that case anyway.
For baz we currently error with:
error: designator order for field âT::aâ does not match declaration order in âTâ
and I guess we should continue to do so, but only after build_aggr_conv
succeeds, right?
Jakub
Re: [patch, fortran] Fix pointers not escaping via C_PTR
Hi Steve, On Thu, Feb 28, 2019 at 09:14:48PM +0100, Thomas Koenig wrote: the attached patch fixes a wrong-code bug for gfortran where pointers were not marked as escaping. A C_PTR can be stashed away and reused later (at least as long as the variable it points to remains active). This is not a regression, but IMHO a bad wrong-code bug. The chances of this patch introducing regressions are really, really low. Regression-tested. OK for trunk? Is this a wrong code bug or a clever user wandering off into undefined behavior? subroutine init() use, intrinsic :: iso_c_binding implicit none integer(c_int), pointer :: a allocate(a) call save_cptr(c_loc(a)) a = 100 end subroutine init Of particular note, 'a' is an unsaved local variable. When init() returns, 'a' goes out-of-scope. Fortran normally deallocates an unsaved allocatable entity, but 'a' is an unsaved local variable with the pointer attribute. For lack of a better term, 'allocate(a)' allocates an anonymous target and associates the anonymous target with the pointer 'a'. save_ptr() caches the address of the anonymous target. When init() returns does the anonymous target get deallocated or does the program leak memory? Neither. If there were no C_PTR pointing to the memory, it would leak memory. In addition, when init() returns, what happens to the pointer association of 'a'? 'a' becomes undefined, hence it is no longer associated with the memory. I think it becomes undefined, which means the anonymous memory is inaccessible at least by Fortran means as there are no pointers associated with the anonymous target. That is not correct. Looking at F2018, 18.2.3.3, by using C_F_POINTER, you can " Associate a data pointer with the target of a C pointer and specify its shape. " First, this talks about a C pointer having a target. Second, you can re-estabilsh the association to a different pointer to the Fortran program. Regards Thomas
Re: [PR fortran/89516, patch] - ICE in gfc_calculate_transfer_sizes at gcc/fortran/check.c:5506
Hi Harald, Adding -Wsurprising as option to gfortran exercised a code path that I hadn't seen when working on simplifications for the TRANSFER intrinsic. While regtesting, I realized that one of the checks was too strict when the MOLD argument was scalar and of size 0 and should only apply to array arguments. I adjusted the corresponding testcase. Regtested on x86_64-pc-linux-gnu. OK for trunk? OK. Thanks for the patch! Regards Thomas
Re: [PR fortran/89516, patch] - ICE in gfc_calculate_transfer_sizes at gcc/fortran/check.c:5506
Committed as rev. 269341 on trunk. On 03/02/19 16:17, Thomas Koenig wrote: > Hi Harald, > >> Adding -Wsurprising as option to gfortran exercised a code path >> that I hadn't seen when working on simplifications for the TRANSFER >> intrinsic. While regtesting, I realized that one of the checks was >> too strict when the MOLD argument was scalar and of size 0 and should >> only apply to array arguments. I adjusted the corresponding testcase. >> >> Regtested on x86_64-pc-linux-gnu. >> >> OK for trunk? > > OK. > > Thanks for the patch! > > Regards > > Thomas >
Re: [PATCH] PR d/89177 - Fix unaligned access in std.digest.murmurhash
Am 01.03.19 um 10:43 schrieb Iain Buclaw: On Sun, 24 Feb 2019 at 16:30, Johannes Pfau wrote: Backport latest murmurhash version from upstream (2.084.1). Ran gdc testsuite on X86_64 linux and got feedback on the bugzilla this really fixes the issue. Raise a pull request with upstream (dmd-cxx is the branch), then this is OK to commit. PR at https://github.com/dlang/phobos/pull/6886 , committed as r269343.
