Skip to content

Update the CLanguageStandard and CXXLanguageStandard enums #3079

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

Merged
merged 1 commit into from
Dec 3, 2020
Merged

Update the CLanguageStandard and CXXLanguageStandard enums #3079

merged 1 commit into from
Dec 3, 2020

Conversation

benrimmington
Copy link
Contributor

@benrimmington benrimmington commented Nov 24, 2020

Generate the enum cases by preprocessing the LangStandards.def file.

Motivation:

Follow-up to: #2716

Modifications:

  • Add enum cases for C17/C18, C2x, and C++20.
  • Deprecate enum cases for C++1z.
  • Remove enum cases for C++2a (which aren't in release/5.3 yet).
  • Update the raw string value for C94 (bug fix).

Result:

  • New documentation comments.
  • New cases for the latest C/C++ language standards.

@benrimmington
Copy link
Contributor Author

benrimmington commented Nov 28, 2020

The enum cases were generated with:

https://gist.github.com/benrimmington/3ca4f22389186766c060d99f05784c1a

  1. In commit 642834d (using shouldGroupByPrefix = false), the two C++1z cases were manually moved, to preserve the existing order.

  2. In commit 1d57496 (using shouldGroupByPrefix = true), library evolution and the synthesized Encodable conformance should allow enum cases to be re-ordered, if readability is improved.

The new C17/C18 modes don't seem to differ from C11, but the guiding principle of the SE-0181 review was "supporting all of the non-deprecated GCC/Clang modes".

For the deprecated C++1z modes, is the introduced: 4 correct?

Cc: @ddunbar

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

First, thanks for this clean-up!

Several of the enum cases have the same comments. From the .def file it looks as if they are synonyms... should it perhaps be noted in the comments that this is intentional (that it's a synonym?).

@abertelrud
Copy link
Contributor

Looks as if the self-hosted Linux test fails because of package-collection related unit tests:

20:42:01 Test Case 'GitHubPackageMetadataProviderTests.testAPILimit' started at 2020-12-02 04:41:33.814
20:42:01 
/home/buildnode/jenkins/workspace/swift-package-manager-self-hosted-Linux-smoke-test/branch-main/swiftpm/Tests/PackageCollectionsTests/GitHubPackageMetadataProviderTests.swift:217: error: GitHubPackageMetadataProviderTests.testAPILimit : XCTAssertEqual failed: ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.invalidResponse(https://api.github.com/repos/octocat/Hello-World, "Empty body"))") is not equal to ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.apiLimitsExceeded(https://api.github.com/repos/octocat/Hello-World, 5))") - 
20:42:01 
/home/buildnode/jenkins/workspace/swift-package-manager-self-hosted-Linux-smoke-test/branch-main/swiftpm/Tests/PackageCollectionsTests/GitHubPackageMetadataProviderTests.swift:217: error: GitHubPackageMetadataProviderTests.testAPILimit : XCTAssertEqual failed: ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.invalidResponse(https://api.github.com/repos/octocat/Hello-World, "Empty body"))") is not equal to ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.apiLimitsExceeded(https://api.github.com/repos/octocat/Hello-World, 5))") - 
20:42:01 
/home/buildnode/jenkins/workspace/swift-package-manager-self-hosted-Linux-smoke-test/branch-main/swiftpm/Tests/PackageCollectionsTests/GitHubPackageMetadataProviderTests.swift:217: error: GitHubPackageMetadataProviderTests.testAPILimit : XCTAssertEqual failed: ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.invalidResponse(https://api.github.com/repos/octocat/Hello-World, "Empty body"))") is not equal to ("Optional(PackageCollections.GitHubPackageMetadataProvider.Errors.apiLimitsExceeded(https://api.github.com/repos/octocat/Hello-World, 5))") - 

@tomerd or @yim-lee Do these look familiar to you?

@tomerd
Copy link
Contributor

tomerd commented Dec 2, 2020

@tomerd or @yim-lee Do these look familiar to you?

hmm, I did run into this once before but thought I fixed it - this seems like a flaky test. re-run the tests and see if this goes away. if so, I will need to come back to fix this test

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Thanks @benrimmington! It does look as if one of the unit test failures is related:

02:06:19 Test Case '-[PackageLoadingTests.PackageDescription4LoadingTests testLanguageStandards]' started.
02:06:19 
/Users/buildnode/jenkins/workspace/swift-package-manager-PR-osx-smoke-test/branch-main/swiftpm/Tests/PackageLoadingTests/PD4LoadingTests.swift:325: error: -[PackageLoadingTests.PackageDescription4LoadingTests testLanguageStandards] : XCTAssertEqual failed: ("Optional("iso9899:199409")") is not equal to ("Optional("iso9899:1994")")

@benrimmington
Copy link
Contributor Author

@abertelrud I've updated the test assertion and documentation comments, and squashed the commits.

I didn't see the test failure locally, because it only gets as far as the guard statement.

https://github.com/apple/swift-package-manager/blob/c68fd5b22b51da157a93eaaa4aaa0e4d90bc38d9/Tests/PackageLoadingTests/PD4LoadingTests.swift#L308-L309

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Great, thanks!

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud abertelrud merged commit b17088e into swiftlang:main Dec 3, 2020
@benrimmington benrimmington deleted the language-standards branch December 4, 2020 03:10
federicobucchi pushed a commit to federicobucchi/swift-package-manager that referenced this pull request Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants