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

cgo: Fix naming of nested anonymous unions #3869

Closed
wants to merge 2 commits into from

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Aug 15, 2023

No description provided.

@QuLogic QuLogic mentioned this pull request Aug 15, 2023
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
tinygo-org#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
@deadprogram
Copy link
Member

@QuLogic can you please rebase this PR against the latest llvm16 branch?

@aykevl can you verify that this PR addresses the current build issues on the latest llvm16 branch so we can look at getting it merged?

Thank you!

@deadprogram
Copy link
Member

@QuLogic
Copy link
Contributor Author

QuLogic commented Sep 12, 2023

That's something new that was added to the llvm16 branch on its last rebase: https://github.com/tinygo-org/tinygo/actions/runs/5931803266/job/16084408715?pr=3741#step:17:21 The CGo error is fixed though.

@deadprogram deadprogram added this to the v0.30.0 milestone Sep 17, 2023
aykevl added a commit that referenced this pull request Sep 18, 2023
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
@deadprogram
Copy link
Member

@QuLogic this PR is now included in #3741 so it can be closed. Thank you very much!!!

@QuLogic QuLogic deleted the llvm16-anonymous-union branch September 18, 2023 18:22
@aykevl
Copy link
Member

aykevl commented Sep 18, 2023

@QuLogic thanks a lot for this fix! It saved me quite a bit of digging.

deadprogram pushed a commit that referenced this pull request Sep 18, 2023
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants