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

refactor: merge redundant super() and this() code #8122

Merged
merged 1 commit into from Apr 5, 2018

Conversation

WalterBright
Copy link
Member

No description provided.

@WalterBright WalterBright added the WIP Work In Progress - not ready for review or pulling label Apr 4, 2018
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

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

dub fetch digger
dub run digger -- build "master + dmd#8122"

@WalterBright WalterBright force-pushed the merge-super branch 2 times, most recently from bdc0f9c to 8febede Compare April 4, 2018 02:14
@WalterBright WalterBright changed the title refactor: merge redundate super() and this() code refactor: merge redundant super() and this() code Apr 4, 2018
@WalterBright WalterBright added Easy Review Refactoring No semantic changes to code and removed WIP Work In Progress - not ready for review or pulling labels Apr 4, 2018
Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

+25 −48 👍


if (!exp.f || exp.f.errors)
return setError();

checkFunctionAttributes(exp, sc, exp.f);
//checkAccess(loc, sc, NULL, f); // necessary?
checkAccess(exp.loc, sc, null, exp.f);
Copy link
Member

@wilzbach wilzbach Apr 4, 2018

Choose a reason for hiding this comment

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

Unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

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

The check should have been in both sections of code, not just one.

}

if (!sc.intypeof)
if (!sc.intypeof /*&& !(sc.ctorflow.callSuper & CSX.halt)*/)
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing that test was a bug introduced by #8100

I plan on doing a proper fix after this is pulled (because then I don't have to do rebases).

@JinShil
Copy link
Contributor

JinShil commented Apr 5, 2018

PR looks good. Why is Jenkins failing?

[libdparse] Running shell script
+ cd test
+ ./run_tests.sh
find: ‘../stdx-allocator/source’: No such file or directory
Compiling unit tests...../src/dparse/parser.d(9): Error: module `mallocator` is in file 'stdx/allocator/mallocator.d' which cannot be read
import path[0] = ../src/
import path[1] = ../stdx-allocator/source
import path[2] = /var/lib/jenkins/dlang_projects/distribution/bin/../imports
+ find test/coverage -type f -exec mv {} . ;
find: ‘test/coverage’: No such file or directory
script returned exit code 1

@wilzbach
Copy link
Member

wilzbach commented Apr 5, 2018

Unrelated -> dlang/ci#193

@wilzbach wilzbach merged commit 408c468 into dlang:master Apr 5, 2018
@WalterBright WalterBright deleted the merge-super branch April 5, 2018 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy Review Refactoring No semantic changes to code
Projects
None yet
4 participants