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

Don't include <iostream> in C++ #201

Merged
merged 1 commit into from
Jan 1, 2018
Merged

Conversation

Teemperor
Copy link
Contributor

@Teemperor Teemperor commented Jan 11, 2017

The imported objects aren't used and there seems to be no
dependency on the includes of in the code.

Also, including < iostream > in libraries is bad practice and some
projects completely forbid doing it (such as LLVM) because it
does some nasty static initializations.

The imported objects aren't used and there seems to be no
dependency on the includes of <iostream> in the code.

Also, including <iostream> in libraries is bad practice and some
projects completely forbid doing it (such as LLVM) because it
does some nasty static initializations.
@andreasabel
Copy link
Member

I am sorry you did not get a reply for your PR for almost a year. There was simply no maintainer to look into this.

Would you be willing to rebase your PR onto the latest master? I suppose the travis checks would then succeed.
Another thing is to run the test suite (currently not automatic). This is done as follows:

cd source
cabal install 
cd ../testing
cabal install
bnfc-system-tests

(assuming .cabal/bin is in your PATH).

@andreasabel andreasabel self-assigned this Dec 19, 2017
@andreasabel andreasabel merged commit 70fa14e into BNFC:master Jan 1, 2018
@andreasabel
Copy link
Member

Thanks @Teemperor, I merged it now.

@andreasabel andreasabel added this to the 2.8.2 milestone Jan 1, 2018
@andreasabel andreasabel added the C++ label Nov 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants