Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARC] assignment of generics with (some) pragmas leads to bad codegen #19374

Open
Vindaar opened this issue Jan 12, 2022 · 2 comments
Open

[ARC] assignment of generics with (some) pragmas leads to bad codegen #19374

Vindaar opened this issue Jan 12, 2022 · 2 comments

Comments

@Vindaar
Copy link
Contributor

Vindaar commented Jan 12, 2022

This seems to be a related issue to #16246.

I get a codegen error using ARC in another project. Realized that I'm essentially assigning a seq[cdouble] to a seq[float] field of an object. While that may not be ideal code, it should be fine given the definition of cdouble and it works fine on refc.

Fortunately, it's easy to reproduce it:

Example

The simplest example is the following:

let bar = @[1.cdouble] 
let foo: seq[float] = bar

which results in:

/home/basti/.cache/nim/bla3_d/@mbla3.nim.c: In function 'NimMainModule':
/home/basti/.cache/nim/bla3_d/@mbla3.nim.c:152:17: error: incompatible types when assigning to type 'tySequence__Zj5xKXgsXnI4n4hPS6nACA' from type 'tySequence__rdiO4Ep0L0U9b79caehGUmfA'
  152 |  foo__bla51_6 = bar__bla51_5;
      |                 ^~~~~~~~~~~~
Error: execution of an external compiler program 'gcc -c  -w -fmax-errors=3   -I/home/basti/src/nim/nim_git_repo/lib -I/tmp -o /home/basti/.cache/nim/bla3_d/@mbla3.nim.c.o /home/basti/.cache/nim/bla3_d/@mbla3.nim.c' failed with exit code: 1

Related (broken) and working cases

It seems to be related to types carrying some pragmas. For example the following also doesn't work:

type
  cdouble2 = cdouble
let bar = @[1.cdouble2]
let foo: seq[cdouble] = bar

(equivalent error)
but this works fine:

type
  float2 = float
let bar = @[1.float2]
let foo: seq[float] = bar

Assigning an exportc pragma to float2 however, breaks it again:

type
  float2 {.exportc.} = float
let bar = @[1.float2]
let foo: seq[float] = bar

Comparison to refc

In comparison to compilation with refc, the generated code for the two cases is rather different (partially due to the seq V1 vs. V2 implementations I suppose?).

Sticking to the first broken example (cdouble and float), we generate the following two types, variables and assignment using refc:

struct tySequence__rdiO4Ep0L0U9b79caehGUmfA {
  TGenericSeq Sup;
  double data[SEQ_DECL_SIZE];
};
struct tySequence__Zj5xKXgsXnI4n4hPS6nACA {
  TGenericSeq Sup;
  NF data[SEQ_DECL_SIZE];
};
// ...
N_LIB_PRIVATE tySequence__rdiO4Ep0L0U9b79caehGUmfA* bar__bla51_5;
N_LIB_PRIVATE tySequence__Zj5xKXgsXnI4n4hPS6nACA* foo__bla51_6;
// ...
	genericSeqAssign((&foo__bla51_6), bar__bla51_5, (&NTIseqLfloatT__Zj5xKXgsXnI4n4hPS6nACA_));

So the variables are pointers to the two different sequence structs and assignment is done via genericSeqAssign.

In comparison using ARC, we see the following:

struct tySequence__rdiO4Ep0L0U9b79caehGUmfA {
  NI len; tySequence__rdiO4Ep0L0U9b79caehGUmfA_Content* p;
};
struct tySequence__Zj5xKXgsXnI4n4hPS6nACA {
  NI len; tySequence__Zj5xKXgsXnI4n4hPS6nACA_Content* p;
};
// ...
N_LIB_PRIVATE tySequence__rdiO4Ep0L0U9b79caehGUmfA bar__bla51_5;
N_LIB_PRIVATE tySequence__Zj5xKXgsXnI4n4hPS6nACA foo__bla51_6;
// ...
	foo__bla51_6 = bar__bla51_5;

The two variables are pure objects of the struct and assignment is a pure C assignment. Due to both being of different types, it of course doesn't compile.

Interestingly enough (to me anyway), a pure type alias of a "simple" type (like the float2 and float example above) actually does eliminate on of the two types completely, which is why the pure C assignment is fine in that case on ARC.

Thoughts on a solution

In the spirit of "learn from it" in #16246 I did indeed try to fix this myself, but in the end I'm very confused about what the solution should even look like. At first I thought this was just an edge case of the sameTypeAux flags comparison that was added in e8dad48, but it doesn't seem to matter whether the comparison holds true or false for these types here.

Some thoughts:

  • on refc the genericSeqAssign handles all this gracefully. I suppose we don't want to go back to having such a function when using ARC?
  • fully eliminating aliases, might be valid in some of these cases (e.g. cdouble and float maybe), but which pragmas attached to a type would disallow that? When using ARC it seems like the Nim compiler should complain that the two types are incompatible in these cases? Or rather all pragmas that the compiler does not complain about, should lead to the alias being removed?
  • I thought about fixing the generated assignment in the case where the two types lead to different struct definitions. But that doesn't work, because the associated variables aren't pointers so we cannot cast them.

Hmm. If someone can give me some guidance on what a solution should look like, I can try to implement it.

Additional Information

This is on current devel:

Nim Compiler Version 1.7.1 [Linux: amd64]
Compiled at 2022-01-11
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 58656aa5bba572672f093499280b69bb0f0d4c06
active boot switches: -d:release -d:danger
@Araq
Copy link
Member

Araq commented Jan 13, 2022

The underlying problem here is that we have type aliases that are not aliases since they use importc. Type aliases cannot have pragmas.

@bung87
Copy link
Collaborator

bung87 commented Oct 22, 2022

workaround is remove the type annotation part from the var declaration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants