Chapel: extern type for one C type in two Chapel modules

[npadmana]

Summary of Problem

Using an extern block in a module appears to cause an error in resolving a function. If I poke around a bit, it appears that the problem is due to common.h being included twice in Chapel, which is somehow resulting in a collision.

Steps to Reproduce

Consider the following set of .h and .chpl files

Source Code:
common.h

#ifndef _COMMON_H
#define _COMMON_H

typedef struct {
  double matrix;
} X;

#endif

test.h

#include "common.h"

X val;

void f(double y, double* x);

void f(double y, double* x) {}

common.chpl

prototype module common {
  use SysCTypes;

  extern {
    #include "common.h"

    static double* _getPtr(X* x) {
      return &(x->matrix);
    }
  }

  proc getPtr(ref val : X) {
    return _getPtr(c_ptrTo(val));
  }
}

test.chpl

use common;

extern {
  #include "test.h"
}

var x : X;

f(1.0, getPtr(x));

Compile command:
chpl test.chpl

fails with

test.chpl:9: error: unresolved call 'getPtr(X)'
./common.chpl:12: note: candidates are: getPtr(ref val: X)

Note that the error message is also a little confusing, since the candidate seems to correctly match
the input.

Additional Information

Removing the common.chpl module and putting everything into the same file works --

test2.chpl

extern {
  #include "test.h"

    static double* _getPtr(X* x) {
      return &(x->matrix);
    }
}

proc getPtr(ref val : X) {
  return _getPtr(c_ptrTo(val));
}
var x : X;

f(1.0, getPtr(x));

Configuration Information

chpl version 1.19.0 pre-release (6509986)

[bradcray]

@npadmana : I thought you'd know that champs use c2chapel!! :D (authored by an unsung hero...)

[mppf]

I'm pretty sure extern blocks are better 😉

[npadmana]

@bradcray : A slightly more serious response -- I find extern block a somewhat lower impedance approach to just calling out to C, when I don't want to write a wrapper module (and worry about making this Chapel-rrific). The biggest downside is building LLVM, but I don't end up rebuilding the Chapel compiler very often these days!

[npadmana]

Ok, this took longer for me to figure out, but I think the issue is that when the same file gets included in two extern blocks in two separate files, the symbols are included in separate modules. And then one module definitions shadows the other.

For example:
common2.h:

#ifndef _COMMON_H
#define _COMMON_H

typedef struct {
  double matrix;
} X;

void f(X x);

void f(X x) {};

#endif

a1.chpl:

extern {
  #include "common2.h"
}

var x1 : X;

a2.chpl:

use a1;

extern {
  #include "common2.h"
}

var x2 : X;

f(x2);
f(x1);

The line f(x1) fails, but the line above works.

[npadmana]

I'd be curious if you have other solutions, but one approach would be to have common header files included in the same module. If one could split a logical module across multiple files as in #10909, then this would be very easy to do.

P.S. I'm fine with closing this issue in favor of eg. #10909 if you agree.

P.P.S. One argument in favor of the c2chapel version where you have more control. :-)

[mppf]

@npadmana - in the current compiler, the extern type declaration creates a new type each time it is seen, rather than (for example) a single type shared by all extern types with the same name. This is true even without using extern blocks.

Consider, for example, the following example (which does not use extern blocks at all):

// demo.h
#include <stdio.h>

typedef int X;
void f(X x);

void f(X x) {
  printf("in f %i\n", x);
}

// demo1.chpl
extern type X;
extern proc f(arg:X);

var demo1var: X;
f(demo1var);

// demo2.chpl
use demo1;

extern type X;
extern proc f(arg:X);

// using the type/f in demo2
var demo2var: X;
f(demo2var);
demo2.f(demo2var);

// using the type/f in demo1
f(demo1var);
demo1.f(demo1var);

// using the type in demo1 and f in demo2
demo2.f(demo1var);

// using the type in demo2 and f in demo1
demo1.f(demo2var);

$ chpl demo.h demo2.chpl
demo2.chpl:18: error: unresolved call 'demo2.f(X)'
demo2.chpl:6: note: candidates are: f(arg: X)

I think it might be reasonable to adjust the compiler's consideration of extern type to consider two different extern types referring to the same C type to be the same type. I'm curious what @bradcray thinks about this. One immediate issue would be that two extern record declarations could include different fields. That might be handle-able by merging the fields, but then weird things might happen with Reflection / param for loops over the fields for that type... Another way of viewing it might be for the two extern types to be different types from the Chapel compiler point of view, but to allow them to coerce between each other (without actually emitting any code to do so, since they are the same type).

@npadmana - in your use case, is it possible to create a Chapel module to contain the extern block including the C resource you are trying to work with?

[daviditen]

My intuition for @mppf's example from the previous comment is that demo1.X and demo2.X should be treated as two distinct types by the Chapel compiler. I'd be somewhat surprised if two different type declarations were magically treated as one. I'd also be surprised if there was an automatic coercion between the two types, but maybe less than if they were completely the same type.

[npadmana]

@mppf -- short answer, yes, I have a workaround for this by just collecting all the relevant pieces in the same module. So this is not blocking in any way.

And I agree with @daviditen -- these probably should be the different types from Chapel's point of view, although I would have loved the compiler to give me a hint here that types were being shadowed. The error message was just very confusing.

I'm fine with automatic coercion if Chapel can determine that the underlying C type is the same. I'd also have been fine with having a module defined piecemeal across multiple files, in which case I could again collect definitions. Or a third option.

@mppf -- on extern blocks vs simple extern type declarations -- good point. However, I think extern blocks are more susceptible to these because of cases like the following (which is closer to what caught me). The issue is that they define types for all header files that get included. If I were writing the external declarations by hand + c2chapel, I get to control this a little more.

common.h

typedef struct {
  int i;
} X;

test1.h

#include "common.h"

X makeX();

test2.h

#include "common.h"

useX(X x);

test1.chpl

extern {
  #include "test1.h"
}

test2.chpl

use test1;

extern {
  #include "test2.h"
}

proc main() {
  var x = makeX();
  useX(x);
}

I'll note that if X were an int instead of a struct, this would compile (ignoring a trivial link error), so for basic types, Chapel seems to know that the types are the same.

[bradcray]

Here are a few quick thoughts:

• Given that Chapel supports renaming of externs, I don't think it makes sense to merge extern declarations that happen to share the same Chapel name on that basis alone (and realize that Michael isn't actually suggesting this, but just to make sure we're all on the same page...). For example, consider:

  {
  extern "long" type my_c_int;
  extern "foo1" proc foo();
  ...
  }
  {
  extern "short" type my_c_int;
  extern "foo2" proc foo();
  ...
  }
  

  It seems clear that the two definitions of my_c_int`foo()` should not get merged in any way since they refer to different C symbols (and Michael was only proposing doing this when they refer to the same C symbol).

• However, it seems like it _might_ _possibly_ be OK to merge extern declarations that share (a) the same external C symbol name and (b) the same Chapel definition (?). This second condition would avoid going down the "merge field lists" path, which definitely sounds terrifying and like a disaster piece of code to maintain to me.

• That said, for Chapel-level code, it seems weird to auto-merge things declared at different scopes for extern declarations when we wouldn't do it for other symbols (@daviditen's point, essentially), so maybe a shadowing warning when the conditions of the previous bullet fire would be more useful/prudent.

• "the same C type" seems like a potentially worrisome condition to have to check

• That said, once extern blocks are in the mix, I have no intuition as to what should happen (they're not a feature I understand very well or feel very invested in... I really do prefer the c2chapel approach).

• I wonder whether suggesting (or requiring?) that any given #include of a header file in an extern block only appear once (or be factored into its own helper module?) might keep things more sane

• I guess I'm leaning more towards "warning" approaches than what seem to me like fiddlier ones that seem to add exceptions.

[mppf]

@npadmana - in terms of ways to adjust extern blocks specifically to help with this issue, I can think of two things:

1. Store all of the extern blocks into a single "C" module. For this to be practical, you'd want to be able to add an extern block to this "C" module even if that block is in a different module.
2. Enable one to filter the C declarations imported by the extern block by name and/or header file. This could possibly control which are public (visible outside of the module) and/or which are available to use from Chapel at all. I wonder extern use is a reasonable way to think about writing this, along with the normal filtering use has. But that doesn't allow controlling declarations by which header file they are in...
3. extern blocks just generate bindings for declarations immediately contained and #included (but not things included by an include).

[npadmana]

@mppf -- I think the issue in this case (and it's a problem for both extern blocks and c2chapel) is that, by default, it generates bindings for everything #included inside. For extern blocks, a simple solution might be to only generate bindings for the code explicitly in the block (or in the top-level included file). This is what eg. SWIG does, and I think is what you want most of the time.

Of course, you might be missing declarations -- so the user would need to include in additional headers explicitly, but then one would have control over what got included and where.

[mppf]

@npadmana - that is an interesting option and I'll add it to the menu in my comment above. My concern with only handling data types that are included directly is that C libraries frequently have something like mylibrary.h contains include mylibrary-impl.h which contains data types that the compiler needs but that aren't really supposed to be part of the API. My concern in particular is that it'd be harder to get such libraries to work with the proposed rule - in particular because you'd have to reason about and use "implementation details" that the library authors didn't want you to rely upon.

[npadmana]

@mppf - I agree with your concerns. And mulling over this some more, I think there isn't a really clean solution once you throw #include's into the mix. At the end of the day, I think extern blocks make sense if the code is contained to a single module, but gets messy when that module gets imported somewhere else.

[mppf]

@npadmana - I think having a way to opt-in to filtering an extern block to specific include files or specific types would handle the problem pretty nicely (option 2 in my list of ideas). Was there a reason you didn't think that was workable?

[npadmana]

@mppf -- I think option 2 would nicely solve the problem of controlling which C types are generated in the extern block. I don't think it solves the other problem though, which is that makeX and useX have different X types. I'm guessing, even in option 2, makeX will generate something like

extern proc makeX() : test1.X;

while useX will look like

extern proc useX(test2.X);

and we still have the issue of letting the compiler know that test1.X and test2.X are the same type.

I tried the simple test of removing the #include in test2.h to see if it just proceeded with an unresolved type that would get fixed at the end in Chapel, but that fails in Clang.

I will say that all of these solutions do start to detract from the simplicity of extern blocks somewhat. It's not that c2chapel doesn't have these problems, but I can just fiddle the created code until it works.

[mppf]

@npadmana - I think we might be getting closer, but I think we need the complete example with the "filter each extern block" strategy:

common.h

typedef struct {
  int i;
} X;

test1.h

#include "common.h"

X makeX();

test2.h

#include "common.h"

useX(X x);

test1.chpl

extern only X, makeX {
  #include "test1.h"
}

test2.chpl

use test1;

extern only useX {
  #include "test2.h"
}

proc main() {
  var x = makeX(); // makeX, X come from test1
  useX(x); // useX comes from test2
}

Now here the key is that the extern block in test2 needs an additional rule. The additional rule is to not add extern declarations to the AST for types or functions that are already handled by other extern blocks in visible modules. In particular, it would generate useX as taking in a test1.X as an argument, where we can imagine that test1.X is defined by something like extern record X { ... }.

In fact, as far as I know, this rule would address the issue even without even requiring the filtering.

[npadmana]

@mppf - yes, I think you've very nicely captured the issue and the solution here. And I agree, with a rule like that, we don't need to filter. And this would let you compose with extern blocks, which is effectively what we were doing in the above case.

Some thoughts:

• I'm guessing we don't add in extern declarations for types/functions with existing extern declarations, whether on not these got produced in an external block or not. Would we have a problem if the previous extern declaration does not match what the compiler might have produced (especially in the case that the previous declaration was handwritten).
• Do we need such a rule for functions, or only for types?
• A nice side-effect of this strategy is that the user can then override a extern block-wrapped declaration with one of their own.
• An orthogonal comment -- it would be nice to have some way of debugging extern block issues by looking at the what the compiler produced for the types/functions.

[mppf]

hi @npadmana - I just wanted to check if you've had a chance to try out extern blocks after PR #12235 and if you find the improvements satisfying. Thanks!

[npadmana]

hi @mppf -- not yet, unfortunately -- but I'm hoping to take a look at this over the weekend. Will keep you updated....