vscode-cpptools icon indicating copy to clipboard operation
vscode-cpptools copied to clipboard

clang-format in the latest pre-release version has broken main include sorting

Open benvanik opened this issue 1 year ago • 6 comments

Environment

  • OS and Version: Windows 10 x64
  • VS Code Version: 1.86.0 (2024-01-31)
  • C/C++ Extension Version: v1.19.2 (pre-release) ---- works with release version v1.18.5

Bug Summary and Steps to Reproduce

Bug Summary: The clang-format included with the extension incorrectly recognizes the main file for SortIncludes and moves it to the incorrect category. The current release version works correctly.

Steps to reproduce:

  1. Create a new .clang-format file with the given contents:
BasedOnStyle: Google
  1. Configure the C/C++ formatter settings to use clang-format and the file style:
    "C_Cpp.formatting": "clangFormat",
    "C_Cpp.clang_format_style": "file",
  1. Create a new file bugtest.c with the given contents:
#include "bugtest.h"

#include <stdio.h>

#include "something_else.h"
  1. Format the document with the "C/C++" formatter

Expected behavior (with current release version v1.18.5 and all prior):

#include "bugtest.h"

#include <stdio.h>

#include "something_else.h"

Actual behavior (with the pre-release version v1.19.2):

#include <stdio.h>

#include "bugtest.h"
#include "something_else.h"

Switching back to the release version and formatting again will resort back to the expected order with the bugtest.h file at the top.

Configuration and Logs

Workspace configuration:

    "C_Cpp.formatting": "clangFormat",
    "C_Cpp.clang_format_style": "file",

This reproduces with setting "C_Cpp.clang_format_style": "Google", and not using the .clang-format file either.

Other Extensions

No response

Additional context

No response

benvanik avatar Feb 01 '24 23:02 benvanik

Hi @benvanik . I'm not able to repro this issue. I get the same results with 1.18.5 and 1.19.2. I'm seeing the later behavior from both versions.

#include <stdio.h>

#include "bugtest.h"
#include "something_else.h"

We haven't updated the binary we're shipping for clang-format between these versions.

C:\Users\coleng\.vscode\extensions\ms-vscode.cpptools-1.19.2-win32-x64\LLVM\bin>clang-format.exe --version
clang-format version 17.0.4 (https://github.com/llvm/llvm-project 309d55140c46384b6de7a7573206cbeba3f7077f)
C:\Users\coleng\.vscode\extensions\ms-vscode.cpptools-1.18.5-win32-x64\LLVM\bin>clang-format.exe --version
clang-format version 17.0.4 (https://github.com/llvm/llvm-project 309d55140c46384b6de7a7573206cbeba3f7077f)

Do you have other settings set, such as C_Cpp.clang_format_path?

If the difference were due to a change in the behavior of clang-format between versions, that's something that would need to be reported to LLVM folks, as we don't control clang-format itself.

If you're still seeing a difference, can you provide more information, such as the output of the C/C++ output channel, for both repros, having set "C_Cpp.loggingLevel": "Debug" ? It seems that, perhaps prior to 1.19.2, there may have been something preventing the format operation from being applied correctly?

Colengms avatar Feb 01 '24 23:02 Colengms

Interesting! I confirmed what you say about the clang-format version being the same in both 1.18.5/1.19.2 on my machine, and invoking from the command line produces the correct results:

C:\Users\Ben>type C:\Users\Ben\bugtest.c
#include "bugtest.h"
#include <stdio.h>
#include "something_else.h"
C:\Users\Ben>C:\Users\Ben\.vscode\extensions\ms-vscode.cpptools-1.19.2-win32-x64\LLVM\bin\clang-format --style=Google C:\Users\Ben\bugtest.c
#include "bugtest.h"

#include <stdio.h>

#include "something_else.h"

I don't have C_Cpp.clang_format_path set as far as I can tell in user settings or workspace settings. I enabled debug output but it doesn't seem to tell me what clang-format it's using, just that it is formatting (identical output in both versions):

LSP: (received) cpptools/formatDocument: file:///c%3A/Users/Ben/bugtest.c (id: 140)
LSP: (invoked) cpptools/formatDocument: file:///c%3A/Users/Ben/bugtest.c (id: 140)
Formatting document: file:///c%3A/Users/Ben/bugtest.c
Formatting Engine: clangFormat
LSP: (received) textDocument/didChange: file:///c%3A/Users/Ben/bugtest.c
LSP: (invoked) textDocument/didChange: file:///c%3A/Users/Ben/bugtest.c

If I switch to the release version, reload the UI, and format the document it does the right thing. If I switch to the pre-release version, reload the UI, and format it does the incorrect thing. No other settings are changed between them, and it reproduces 100% of the time (I'm sitting and switching versions back and forth), and since the clang-format binary is the same that seems to indicate something else.

I don't think I'm taking crazy pills, so maybe this will at least show the difference being the version of the cpptools:

https://github.com/microsoft/vscode-cpptools/assets/75337/63e407f3-42f0-4368-9fd5-4e567a495609

Is there a way to see the command line being passed to clang-format? Maybe it differs?

benvanik avatar Feb 01 '24 23:02 benvanik

Ahh I was wrong, there is a tiny difference:

Release version: note the capitalized drive specifier in the Formatting document line (D):

LSP: (received) cpptools/formatDocument: file:///d%3A/Dev/iree/bugtest.c (id: 61)
LSP: (invoked) cpptools/formatDocument: file:///d%3A/Dev/iree/bugtest.c (id: 61)
Formatting document: file:///D%3A/Dev/iree/bugtest.c
Formatting Engine: clangFormat
LSP: (received) textDocument/didChange: file:///d%3A/Dev/iree/bugtest.c
LSP: (invoked) textDocument/didChange: file:///d%3A/Dev/iree/bugtest.c

Pre-release version: note that the driver specifier is now lower-case (d):

LSP: (received) cpptools/formatDocument: file:///d%3A/Dev/iree/bugtest.c (id: 22)
LSP: (invoked) cpptools/formatDocument: file:///d%3A/Dev/iree/bugtest.c (id: 22)
Formatting document: file:///d%3A/Dev/iree/bugtest.c
Formatting Engine: clangFormat
LSP: Sending response (id: 22)
LSP: (received) textDocument/didChange: file:///d%3A/Dev/iree/bugtest.c
LSP: (invoked) textDocument/didChange: file:///d%3A/Dev/iree/bugtest.c

Perhaps a clue?

benvanik avatar Feb 02 '24 00:02 benvanik

Aha. I failed to repro because I wasn't using a source name that matches the header's name. Now I can repro the difference. It does appear to be a difference in casing in the command line, in an -assume-filename= argument. I have a fix, which should be available in 1.19.3, once it's ready. I'll also add logging of the command line used to invoke clang-format, to make this type of issue easier to investigate in the future.

Colengms avatar Feb 02 '24 01:02 Colengms

Thank you for the speedy response!

benvanik avatar Feb 02 '24 01:02 benvanik

@benvanik Fixed with https://github.com/microsoft/vscode-cpptools/releases/tag/v1.19.3

sean-mcmanus avatar Feb 13 '24 22:02 sean-mcmanus