Generated itype incompatibility in test hash.c

[kyleheadley]

Running 3c -alltypes hash.c in our test directory returns a file with this declaration near line 25

vsf_sysutil_memclr(_Array_ptr<_Ptr<T>> p_dest : itype(_Array_ptr<T>) byte_count(size), unsigned int size)

Clang will not compile it (even with -f3c-tool) because the type and itype of p_dest conflict.

[john-h-kastner]

The same incorrect conversion happens under my new itype implementation.

Seems to be related to #216 and #236. vsf_sysutil_memclr already has _Itype_for_any, so it shouldn't be rewritten. What's odd is that it adds that the extra _Ptr in there.

[kyleheadley]

I reduced the above code into a minimum example:

_Itype_for_any(T)
void foo(void* bar : itype(_Array_ptr<T>))
{ return; }

void do_foo(int count)
{
  int **arr = malloc(sizeof(int*) * count);
  foo(arr);
}

rewrites to:

_Itype_for_any(T)
void foo(_Array_ptr<_Ptr<T>> bar : itype(_Array_ptr<T>))
{
  return;
}


void do_foo(int count)
{
  _Array_ptr<_Ptr<int>> arr : count(count) = malloc(sizeof(int*) * count);
  foo<_Ptr<int>>(arr);
}

The generic is correctly inferred inside of do_foo, but the formal parameter of foo is rewritten either without this inference or with some bug.

[kyleheadley]

@mwhicks1 and @jackastner asked for addition details about this issue:

The type_params.c test file misses two things: it does not check that function declarations are rewritten; it does not call functions with variables that need rewritten declarations. It does check that type parameters are correctly inserted at function calls. Looking at the simplified example in the post above, these are the three things that are rewritten, in order.

The error seen above is with the first of these, void* bar is rewritten as _Array_ptr<_Ptr<T>> bar. This can be solved by reusing the typedef code (setting typelevelinfo based on the adjusted typed, here the itype, rather than the initial type, here void*), which prevents rewriting from using any atoms past the location of the typedef/type parameter. This solution does not currently extend to the original example in the thread, so something else must be going on as well.

However, if I add a call to foo using a type with two arrays, the currently correct rewriting of int **arr to _Array_ptr<_Ptr<int>> arr becomes _Array_ptr<_Array_ptr<int>> arr, presumably because of a constraint set linking to the second pointer level. We need to avoid creating this constraint.

The full example with the initial fix mentioned above is:

_Itype_for_any(T)
void foo(void* bar : itype(_Array_ptr<T>))
{ return; }

void do_foo(int count)
{
  int **arr = malloc(sizeof(int*) * count);
  foo(arr);
}

void do_foo_float(void)
{
  float fl _Checked[5][5] = {};
  foo(fl);
}

rewrites to:

_Itype_for_any(T)
void foo(_Array_ptr<T> bar : itype(_Array_ptr<T>) count(5))
{ return; }

void do_foo(int count)
{
  _Array_ptr<_Array_ptr<int>> arr : count(count) = malloc(sizeof(int*) * count);
  foo<_Array_ptr<int>>(arr);
}

void do_foo_float(void)
{
  float fl _Checked[5][5] = {};
  foo<float _Checked[5]>(fl);
}

On the other hand, this can only affect the 2nd+ pointer levels now, so I could deal with it later. I suspect it will come up when inferring the first pointer level, which is the work I'll be doing next.

[mwhicks1]

I'm a little unsure where we are on all of this. It seems to me that:

• We should add a new test, or extend hash.c, to get the full depth of the issue, and the assertion that it's fixed.
• We should write a separate issue about any limitations on generic type instantiations, e.g., involving pointers like void **.

Does that seem right @kyleheadley ?

[kyleheadley]

I'll add the simplified version above as a test and an xfail test for the over-constrained version. The other issue exists as #358, which describes possible improvements (ie current limitations) to generic type inference, ordered roughly by complexity and value to our goals. It stands as a reminder of what not to worry about yet as much as what to do next. "void**" is number 4, this bugfix is part of number 1.

[kyleheadley]

This issue was solved by #361, but an aspect still remains as issue #367.

The correctness problem was that rewriting didn't stop at the generic variable, instead continuing on to add atoms from the full analysis. This was solved by using typedef info (generics are represented internally by typedefs), which itself had a bug, being defined by the wrong data. Solving the typedef issue solved the generic rewriting as well.

The remaining aspect is that the type and itype are identical and therefor only one is necessary.