Skip to content

Separate Initializer semantic#7007

Merged
WalterBright merged 1 commit intodlang:masterfrom
RazvanN7:Semantic_Initializer
Jul 24, 2017
Merged

Separate Initializer semantic#7007
WalterBright merged 1 commit intodlang:masterfrom
RazvanN7:Semantic_Initializer

Conversation

@RazvanN7
Copy link
Contributor

This PR separates the semantic routines from the AST classes of the Initializer nodes. It is a follow-up to @yebblies 's work #5730

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

@dlang-bot dlang-bot added the Review:WIP Work In Progress - not ready for review or pulling label Jul 19, 2017
@RazvanN7
Copy link
Contributor Author

@UplinkCoder any ideas why compiling druntime leads to CTFE errors?

@UplinkCoder
Copy link
Member

UplinkCoder commented Jul 19, 2017

@RazvanN7 Nothing that jumps in the eye.
I suggest to leave the code as is, for now.

Are you sure the constructors are getting called correctly ?

@RazvanN7
Copy link
Contributor Author

@UplinkCoder you mean drop the PR? why?

@UplinkCoder
Copy link
Member

The code is not likely to change.
So there is little benefit it splitting it out.

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Jul 19, 2017

@UplinkCoder

Are you sure the constructors are getting called correctly ?

Yep. I just copy-pasted the code and added i. in front of (1) all the variables that would normally have been parameters and (2) calls to member functions.

So there is little benefit it splitting it out.

I disagree. Refactoring the code this way makes the code easier to read and makes packaging the compiler as a library easier

@RazvanN7 RazvanN7 force-pushed the Semantic_Initializer branch from a9fdc0f to 141c339 Compare July 19, 2017 13:30
@wilzbach
Copy link
Contributor

I disagree. Refactoring the code this way makes the code easier to read and makes packaging the compiler as a library easier

I can't agree more. Code is read a lot more than its written and every change that helps to improve the readability of the dmd codebase is imho very useful.
@UplinkCoder don't forgot that there are and will be people who want to understand the dmd codebase in the future.

@RazvanN7 Please don't stop cleaning up and refactoring the frontend. You are doing great and important work!

@RazvanN7 RazvanN7 force-pushed the Semantic_Initializer branch from 64b503e to 8c849f4 Compare July 21, 2017 10:17
@RazvanN7 RazvanN7 changed the title [WIP] Separate Initializer semantic Separate Initializer semantic Jul 21, 2017
@RazvanN7
Copy link
Contributor Author

I plan on doing Expression next.

@ibuclaw
Copy link
Member

ibuclaw commented Jul 21, 2017

And C++ headers?

@RazvanN7 RazvanN7 force-pushed the Semantic_Initializer branch from 8c849f4 to c3ee87c Compare July 24, 2017 07:57
@RazvanN7
Copy link
Contributor Author

@ibuclaw forgot about those... done.

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

Labels

Review:WIP Work In Progress - not ready for review or pulling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants