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

fix Issue 21282 mixin of AliasSeq cannot alias an expression #11810

Closed
wants to merge 1 commit into from

Conversation

WalterBright
Copy link
Member

No description provided.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
21282 regression mixin of AliasSeq "cannot alias an expression"

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

To target stable perform these two steps:

  1. Rebase your branch to upstream/stable:
git rebase --onto upstream/stable upstream/master
  1. Change the base branch of your PR to stable

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11810"

src/dmd/dsymbolsem.d Outdated Show resolved Hide resolved
@thewilsonator
Copy link
Contributor

This is a regression. It should target stable.

@BorisCarvajal
Copy link
Member

BorisCarvajal commented Sep 29, 2020

@WalterBright I was fixing this issue just now and your fix seems very overkill.
The mine is just a little change to aliasSemantic:

if (e)  // Try to convert Expression to Dsymbol
{
    if (auto te = e.isTupleExp())
         s = new TupleDeclaration(te.loc, ds.ident, cast(Objects*)te.exps);
    else
...

Currently you can alias a tuple but only if it's encapsulated as a symbol (TupleDeclaration).
The previous change to mixin exposed a new code path where the tuple is presented as an expression to the alias code.
My fix is simple, just create a TupleDeclaration from the TupleExpression.

@WalterBright
Copy link
Member Author

@BorisCarvajal where is the mixin be expanded? Anyhow, create a PR for your version, and we'll compare.

@WalterBright
Copy link
Member Author

Buildkite has ANOTHER Heisenbug:

[boostrap] Downloading compiler http://downloads.dlang.org/releases/2.x/2.088.0/dmd.2.088.0.linux.tar.xz
--
  | curl: (56) Recv failure: Connection reset by peer
  | xz: (stdin): Unexpected end of input
  | tar: Unexpected EOF in archive
  | tar: Unexpected EOF in archive
  | tar: Error is not recoverable: exiting now
  | ERROR: bootstrapping failed.
  | 🚨 Error: The command exited with status 1

https://issues.dlang.org/show_bug.cgi?id=21284

Is it possible we can get some Test Suite action on these problems? It's really not good enough to have to keep restarting the whole shebang.

https://issues.dlang.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&keywords=TestSuite%2C%20&keywords_type=allwords&list_id=233345&query_format=advanced

@BorisCarvajal
Copy link
Member

where is the mixin be expanded?

I think in typesem.d->resolve()->visitMixin()

Gonna push it soon.

@thewilsonator
Copy link
Contributor

That looks like a network error, good luck eliminating them entirely.

@WalterBright
Copy link
Member Author

That looks like a network error, good luck eliminating them entirely.

A number of them have been successfully fixed with the method: for network errors, sleep for a few seconds, then try it again.

The problem with not fixing these is the number of tests we have that are dependent on the network is getting so large it becomes a virtual certainty that it will fail somewhere and we can't get a clean test run.


template Bug(T...) {
alias Bug = I!(T[0]);
//alias Bug = mixin("I!(T[0])");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the bug is not tested.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ak, you're right

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found it when I was stealing the test case.

@BorisCarvajal
Copy link
Member

my pull #11814

@WalterBright
Copy link
Member Author

Closing in favor of #11814

@WalterBright WalterBright deleted the fix21282 branch September 29, 2020 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants