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

fix: remove -Wextra-semi #1581

Merged
merged 1 commit into from Sep 4, 2023
Merged

Conversation

getchoo
Copy link
Member

@getchoo getchoo commented Aug 30, 2023

Parent PR: #1107

this flag is unavailable on gcc versions < 8. we could detect the
version of the compiler here, but i don't think we lose much in this
flags removal and this is a simpler option

Signed-off-by: seth getchoo@tuta.io

this flag is unavailable on gcc versions < 8. we could detect the
version of the compiler here, but i don't think we lose much in this
flags removal and this is a simpler option

Signed-off-by: seth <getchoo@tuta.io>
@Trial97
Copy link
Member

Trial97 commented Aug 30, 2023

Can we close this in favor of this commit:edd1b08 ?
From my understanding, this only happens with the gnu compiler.

@getchoo
Copy link
Member Author

getchoo commented Sep 3, 2023

Can we close this in favor of this commit:edd1b08 ?

From my understanding, this only happens with the gnu compiler.

i would be up for that, but i can't confirm whether or not the flag has always been in clang - or at the very least, has been in all decently modern versions. a bit of googling gave me references to the flag around clang 7/8 (which aren't that old), but i can't find anything earlier. this might just be the safest option unless we dig up some random patch note to date the flag in clang

@Trial97
Copy link
Member

Trial97 commented Sep 4, 2023

Dug out the past: llvm/llvm-project@2f7dc46
It looks like this option was added in Clang 3.2(https://github.com/llvm/llvm-project/releases/tag/llvmorg-3.2.0-rc1)
Sadly no direct mention in the release notes of the flag was found, just the random patch that adds it.

@Scrumplex Scrumplex merged commit cf59986 into PrismLauncher:develop Sep 4, 2023
32 of 33 checks passed
@Scrumplex Scrumplex added this to the 8.0 milestone Sep 4, 2023
@Scrumplex Scrumplex added the changelog:merged A PR that will be merged into a parent PR in the changelog label Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:merged A PR that will be merged into a parent PR in the changelog simple change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants