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-format] Fix breaking change in 20.1.0 by automatically falling back to Cpp formatting if C formatting fails #132832

Closed
sean-mcmanus opened this issue Mar 24, 2025 · 14 comments · Fixed by #133033
Assignees

Comments

@sean-mcmanus
Copy link

sean-mcmanus commented Mar 24, 2025

@owenca The PR at #128287 broke the case where a .c file is formatted in a folder with a .clang-format with Language: Cpp (and no "Language: C").

Can you make it not be a breaking change by automatically falling back to formatting with Cpp if no C language block is specified in .clang-format instead of giving an error?

Or do you recommend a higher level tooling layer detect the failure and change use --assume-filename=file.cpp instead of file.c as a workaround or what?

i.e. command line clang-format test.c gives "Configuration file(s) do(es) not support C".

@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025

@llvm/issue-subscribers-clang-format

Author: Sean McManus (sean-mcmanus)

@owenca The PR at https://github.com//pull/128287 broke the case where a .c file is formatted in a folder with a .clang-format with Language: Cpp.

Can you make it not be a breaking change by automatically falling back to formatting with Cpp if no C language block is specified in .clang-format instead of giving an error?

Or do you recommend a higher level tooling layer detect the failure and change use --assume-filename=file.cpp instead of file.c as a workaround or what?

@bobbrow
Copy link

bobbrow commented Mar 24, 2025

Additional context: We are maintainers of the Microsoft C/C++ extension for VS Code and this issue was brought to our attention by one of our users.

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

Can you provide a reproducer? It seems to work for me.

$ cat .clang-format
Language: Cpp
$ cat foo.c
int i;
$ clang-format -version
clang-format version 20.1.0
$ clang-format foo.c
int i;
$ 

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

Using the config file in microsoft/vscode-cpptools#13406 also works for me.

@MaJerle
Copy link

MaJerle commented Mar 25, 2025

@owenca the code in the cpptools#13406 is a reproducible on my side.

With the latest release, if you pass the .c file and you don't have the C language in the clang-config, then you get an error for wrong language.

This breaks practically everything as now clang-format has to be updated and eventually 2 configs have to be put together for C and Cpp, respectively. For versions prior to 20, adding the C language will throw an error.

vscode cpp tools does this call:

clang-format --style=file --fallback-style=LLVM --Wno-error=unknown --assume-filename=C:\Users\file\main.c C:\USERS\FILE\MAIN.C

When you run it with clang-format 20.1, with format file

---
Language: Cpp
---

You get:

Configuration file(s) do(es) not support C:

But when you run it with clang-format as:

---
Language: Cpp
---
Language: C
---

Then no error and it produces the expected result.

Ultimately, I think what we shall have is:

  • Cpp parameter remains valid for C and C++, since this is how users are used to do it up to now
  • C shall be for C only while maybe CppOnly or similar parameter would be only for C++ files but no C.

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

@owenca the code in the cpptools#13406 is a reproducible on my side.

With the latest release, if you pass the .c file and you don't have the C language in the clang-config, then you get an error for wrong language.

This breaks practically everything as now clang-format has to be updated and eventually 2 configs have to be put together for C and Cpp, respectively. For versions prior to 20, adding the C language will throw an error.

vscode cpp tools does this call:

clang-format --style=file --fallback-style=LLVM --Wno-error=unknown --assume-filename=C:\Users\file\main.c C:\USERS\FILE\MAIN.C

When you run it with clang-format 20.1, with format file

---
Language: Cpp
---

You get:

Configuration file(s) do(es) not support C:

But when you run it with clang-format as:

---
Language: Cpp
---
Language: C
---

Then no error and it produces the expected result.

Ultimately, I think what we shall have is:

  • Cpp parameter remains valid for C and C++, since this is how users are used to do it up to now
  • C shall be for C only while maybe CppOnly or similar parameter would be only for C++ files but no C.

I still couldn't reproduce it:

$ cat .clang-format
---
Language: Cpp
---
$ cat main.c
int main() {
  return 0;
}
$ clang-format -version
clang-format version 20.1.1
$ clang-format --style=file --fallback-style=LLVM --Wno-error=unknown --assume-filename=main.c main.c
int main() { return 0; }
$ echo $?
0
$ 

@sean-mcmanus
Copy link
Author

sean-mcmanus commented Mar 25, 2025

@owenca I may know why you're not reprong it -- adding a .clang-format with

BasedOnStyle:  LLVM

in the parent folder causes it to not repro for me -- can you check your parent folders for any .clang-format files? When I remove/that file the bug start to repro. I just have a .clang-format in my current working directory where the source file is.

repro:
Image

My repro is on Windows, but I can check mac/Linux later today.

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

@sean-mcmanus you are right!

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

@MaJerle if you delete the Language: Cpp line in your config, it should work for both .c and .cpp files.

@MaJerle
Copy link

MaJerle commented Mar 25, 2025

@MaJerle if you delete the Language: Cpp line in your config, it should work for both .c and .cpp files.

Yup, I confirm this is the case.

But this will still impact other users in general. Because C and Cpp will have to be updated in their format file.

@owenca
Copy link
Contributor

owenca commented Mar 25, 2025

@sean-mcmanus One workaround you may want to consider is to change the command to:

clang-format --style=file --fallback-style=LLVM --Wno-error=unknown < main.c

@sean-mcmanus
Copy link
Author

sean-mcmanus commented Mar 25, 2025

@owenca What is changed by that? Removing the --assume-filename? That flag also enables the include sorting format feature to properly sort the file's header include at the top. Also, we already pass the file text to clang-format via stdin, so I'm not sure how that would be different from using <.

An example C language repo with a Cpp language in .clang-format is at https://github.com/leo-arch/clifm/blob/91e1e7f03adcc757882a310a4bac55220eb9e7d1/src/.clang-format#L4 . The comment next to it is Language: Cpp # Cpp is also used for C.

@owenca owenca self-assigned this Mar 26, 2025
owenca added a commit to owenca/llvm-project that referenced this issue Mar 26, 2025
@owenca
Copy link
Contributor

owenca commented Mar 26, 2025

@owenca What is changed by that? Removing the --assume-filename? That flag also enables the include sorting format feature to properly sort the file's header include at the top. Also, we already pass the file text to clang-format via stdin, so I'm not sure how that would be different from using <.

If you need to use --assume-filename, then pass a .cpp argument, e.g. --assume-filename=test.cpp. Either way, you will need to use the < for the workaround:

$ clang-format --style=file --fallback-style=LLVM --Wno-error=unknown < test.c
int i;
$ clang-format --style=file --fallback-style=LLVM --Wno-error=unknown --assume-filename=.cpp < test.c
int i;
$ 

@bobbrow
Copy link

bobbrow commented Mar 27, 2025

Thank you for the quick response @owenca!

swift-ci pushed a commit to swiftlang/llvm-project that referenced this issue Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants