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

Clang 16 -> 17 migration path for -pedantic -Wno-gnu-empty-initializer #64357

Closed
porglezomp opened this issue Aug 2, 2023 · 8 comments · Fixed by llvm/llvm-project-release-prs#566
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer release:backport release:merged

Comments

@porglezomp
Copy link
Contributor

We have a user building with -pedantic who has -Wno-gnu-empty-initializer set on their codebase.
Preflighting Clang 17 adoption, their code is failing because -Wno-gnu-empty-initializer no longer exists.
That's now covered under -Wno-c2x-extensions but this doesn't suppress that warning on Clang 16.
You can't use both flags, because -Wno-gnu-empty-intializer is no longer a flag in Clang 17, which means there is no combination of flags that lets you suppress this warning across adjacent clang versions.
Can we re-add this warning flag to leave a migration path that lets them continue to build with two adjacent clang versions?

Reproducer to check against:

struct Example {
    int x;
};

int main(void) {
    struct Example example = {};
}
  • Clang 16: -Werror -pedantic -Wno-gnu-empty-intializer - OK
  • Clang 17: -Werror -pedantic -Wno-c2x-extensions - OK
  • Clang 16: -Werror -pedantic -Wno-c2x-extensions - does not suppress the diagnostic, fails with -Wgnu-empty-intializer.
  • Clang 17: -Werror -pedantic -Wno-gnu-empty-intializer - fails with -Wunknown-warning-option
@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Aug 2, 2023
@porglezomp
Copy link
Contributor Author

I'll also note: If the solution is "do -Werror -pedantic -Wno-gnu-empty-intializer -Wno-c2x-extensions -Wno-unknown-warning-option" that's probably fine, but I wanted to bring up this migration pain point either way since I expect some other people who need to compile on two consecutive compiler versions will hit this.

@ahatanaka
Copy link
Contributor

5d8aaad removed -Wgnu-empty-intializer.

@AaronBallman
Copy link
Collaborator

I think it's reasonable to leave -Wgnu-empty-intializer in place for Clang 17 to give people more time to transition. I'll try to get this taken care of soon so we can get it into the next 17.x release candidate.

@AaronBallman
Copy link
Collaborator

While working on this, I realized there's at least two ways to view this: add the warning group back in and let it control diagnostic behavior or add the warning group back in and silently accept it without changing diagnostic behavior. I think the latter makes the most sense to me -- this used to be a GNU extension, but the feature was added to C2x so now it's a C extension rather than a GNU extension.

Would you be fine if the behavior was to silently eat use of the warning group? e.g.,

// RUN: %clang_cc1 %s -std=c2x -Wall -pedantic -fsyntax-only -verify=good
// RUN: %clang_cc1 %s -std=c2x -Wpre-c2x-compat -fsyntax-only -verify=c2x
// RUN: %clang_cc1 %s -std=c2x -Wpre-c2x-compat -Wno-gnu-empty-initializer -fsyntax-only -verify=c2x
// RUN: %clang_cc1 %s -std=c2x -Wgnu-empty-initializer -fsyntax-only -verify=good
// RUN: %clang_cc1 %s -std=c17 -Wall -pedantic -fsyntax-only -verify=c2x-ext
// RUN: %clang_cc1 %s -std=c17 -Wgnu-empty-initializer -fsyntax-only -verify=good
// RUN: %clang_cc1 %s -std=c17 -Wc2x-extensions -fsyntax-only -verify=c2x-ext
// RUN: %clang_cc1 %s -std=c17 -Wpre-c2x-compat -fsyntax-only -verify=good

// good-no-diagnostics

// Empty brace initialization used to be a GNU extension, but the feature was
// added to C2x. We now treat empty initialization as a C extension rather than
// a GNU extension. Thus, -Wgnu-empty-initializer is always silently ignored.

struct S {
  int a;
};

struct S s = {};     /* c2x-warning {{use of an empty initializer is incompatible with C standards before C2x}}
                        c2x-ext-warning {{use of an empty initializer is a C2x extension}}
                      */

void func(void) {
  struct S s2 = {};  /* c2x-warning {{use of an empty initializer is incompatible with C standards before C2x}}
                        c2x-ext-warning {{use of an empty initializer is a C2x extension}}
                      */
  (void)s2;
}

This would allow you to do -Werror -pedantic -Wno-gnu-empty-intializer -Wno-c2x-extensions in both Clang 16 & 17 without tripping up on the unknown warning diagnostic or the use of an extension diagnostic.

@porglezomp
Copy link
Contributor Author

Yes, that sounds perfectly fine to me here.

@AaronBallman
Copy link
Collaborator

/cherry-pick 151214b

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 10, 2023

/branch llvm/llvm-project-release-prs/issue64357

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 10, 2023
llvm/llvm-project@5d8aaad
removed the warning group as the functionality is no longer a GNU
extension. However, users have asked for the warning group to be
supported so that code transitioning from Clang 16 to Clang 17 has an
easier migration path when compiling with -Werror. This patch restores
the warning group, but as an ignored warning group because the
functionality is now always considered to be a C extension rather than
a GNU extension. This allows users to do:

  -Werror -pedantic -Wno-gnu-empty-intializer -Wno-c2x-extensions

to silence the diagnostics in both Clang 16 and Clang 17.

Fixes llvm/llvm-project#64357
Differential Revision: https://reviews.llvm.org/D157503

(cherry picked from commit 151214b)
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 10, 2023

/pull-request llvm/llvm-project-release-prs#566

@EugeneZelenko EugeneZelenko reopened this Aug 10, 2023
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 30, 2023
llvm/llvm-project@5d8aaad
removed the warning group as the functionality is no longer a GNU
extension. However, users have asked for the warning group to be
supported so that code transitioning from Clang 16 to Clang 17 has an
easier migration path when compiling with -Werror. This patch restores
the warning group, but as an ignored warning group because the
functionality is now always considered to be a C extension rather than
a GNU extension. This allows users to do:

  -Werror -pedantic -Wno-gnu-empty-intializer -Wno-c2x-extensions

to silence the diagnostics in both Clang 16 and Clang 17.

Fixes llvm/llvm-project#64357
Differential Revision: https://reviews.llvm.org/D157503

(cherry picked from commit 151214b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer release:backport release:merged
Projects
Development

Successfully merging a pull request may close this issue.

5 participants