Skip to content

Fix include guards#25070

Merged
compnerd merged 1 commit into
swiftlang:masterfrom
moatom:fix-include-guard
Jun 21, 2019
Merged

Fix include guards#25070
compnerd merged 1 commit into
swiftlang:masterfrom
moatom:fix-include-guard

Conversation

@moatom

@moatom moatom commented May 26, 2019 •
edited
Loading

Copy link
Copy Markdown

I fixed include guards using the identifiers reserved by C++.

gottesmm
gottesmm approved these changes May 26, 2019
View reviewed changes

@gottesmm gottesmm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! Thanks! = ).

@gottesmm

gottesmm commented May 26, 2019

Copy link
Copy Markdown
Contributor

@swift-ci test and merge

@compnerd

compnerd commented May 27, 2019

Copy link
Copy Markdown
Member

@swift-ci please test and merge

@moatom

moatom commented May 28, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Sorry.
I accidentally send this PR without testing in my environment.
But I didn't think it would fail.

Anyway, it seems to work fine if undoing the changes of HeapObject.h, KeyPath.h and System.h's include guards and then I found that these just have include guards of the same names as others.

@moatom

moatom commented May 28, 2019

Copy link
Copy Markdown
Author

Now, It would work.
Please check.

@CodaFi

CodaFi commented May 28, 2019

Copy link
Copy Markdown
Contributor

@swift-ci please test

@CodaFi CodaFi closed this May 28, 2019
@CodaFi CodaFi reopened this May 28, 2019
@moatom

moatom commented May 31, 2019

Copy link
Copy Markdown
Author

@gottesmm
Could you merge it, or is there something wrong with it?

gottesmm
gottesmm reviewed May 31, 2019
View reviewed changes
Comment thread stdlib/public/SwiftShims/Target.h Outdated

@gottesmm gottesmm May 31, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you change /* to //?

@moatom

moatom commented May 31, 2019

Copy link
Copy Markdown
Author

ok

@compnerd

compnerd commented Jun 1, 2019

Copy link
Copy Markdown
Member

@moatom could you please squash the commits into a single commit?

@moatom

moatom commented Jun 2, 2019

Copy link
Copy Markdown
Author

@compnerd
Thank you for pointing it out.
It is rebase -i, isn't it?

I have another question.
When I searched Swift's source code by #endif /* , I found further ununified comments at the namespace ends in 24 files.
Should I change their /* to // this time?
We cannot avoid recompilation of all the translation units depending on them :(.

@compnerd

compnerd commented Jun 2, 2019

Copy link
Copy Markdown
Member

Yes, use git rebase -i to squash the commits.

Sure, changing the ending on those in a follow up is fine.

@moatom

moatom commented Jun 2, 2019

Copy link
Copy Markdown
Author

Done!

@compnerd

compnerd commented Jun 2, 2019

Copy link
Copy Markdown
Member

@swift-ci please test

@compnerd
compnerd merged commit 38995f5 into swiftlang:master Jun 21, 2019
@moatom
moatom deleted the fix-include-guard branch July 21, 2019 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gottesmm gottesmm gottesmm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@moatom @gottesmm @compnerd @CodaFi