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] unused-function diagnostic regression? #109

Open
kornman00 opened this issue Aug 8, 2023 · 11 comments
Open

[clang] unused-function diagnostic regression? #109

kornman00 opened this issue Aug 8, 2023 · 11 comments
Labels
enhancement New feature or request

Comments

@kornman00
Copy link

I recently downloaded 2.9.2, after using 2.8.2. I was using -Werror=unused-function to remove some dead code but noticed I was not hitting any errors after performing a rebuild on a library. After switching back to 2.8.2 in my msbuild .props files I once again get errors about unused functions as expected.

For 2.8.2, I'm setting LLVMInstallDir to my local llvm-msvc 2.8.2 copy and LLVMToolsVersion to 17.
For 2.9.2, I'm setting LLVMInstallDir to my local llvm-msvc 2.9.2 copy and LLVMToolsVersion to 18.

I'm using the latest VS2022 Preview (17.8 Preview 1) in the event that it matters.

Note, I have not tried stock LLVM v18 - may be possible it was regressed there?

@gmh5225 gmh5225 added the enhancement New feature or request label Aug 9, 2023
@gmh5225
Copy link
Contributor

gmh5225 commented Aug 9, 2023

It was deleted by us because some people said that this diagnostic is trouble and there is no such diagnostic on MSVC. I'm thinking of a compatible solution

@kornman00
Copy link
Author

Isn't the compatible solution to default to or implicitly set -Wno-unused-function :)? Then people that desire the warning can say -Wunused-function?

For context, I'm building code that was previously only ever compiled with MSVC. I'm building with warnings-as-errors but have toggled off or no-error'd some warnings which I want to address later, like unused-function. Also going through the exercise of figuring out how to get clang or the linker warn about which public symbols are unused (or if its even possible) since unused-function only seems to apply to static linkage or LLVM's nature of omitting unused symbols short circuits any additional reporting.

@gmh5225
Copy link
Contributor

gmh5225 commented Aug 10, 2023

I will figure it out.

@gmh5225
Copy link
Contributor

gmh5225 commented Aug 12, 2023

Give it a try https://github.com/backengineering/llvm-msvc/releases/tag/llvm-msvc-v2.9.3

@kornman00
Copy link
Author

kornman00 commented Sep 20, 2023

Sorry, haven't had time to setup a test until today. I tested with v3.0.8 and it still is silent about unused functions and variables, even though I have them explicitly set as warnings.

I realize that in my original post that I mentioned I had these set as errors, but for iteration of catching the list of everything that appears as unused (at least in 2.8.2, before the change you mentioned on August 8th) I set these to show up as warnings still, while not changing the overall default of warnings-as-errors. Here's a subset of what I have in a .props file:

    <ClangClAdditionalOptions>
      -Wno-unused-value
      -Wno-error=unused-variable
      -Wno-error=unused-function
      -Wno-error=unused-member-function
      -Wno-error=unused-const-variable
      -Wno-unused-but-set-variable
      -Wno-unused-local-typedef
      -Wno-unused-but-set-parameter
      $(ClangClAdditionalOptions)
    </ClangClAdditionalOptions>

Here's the current output, for what it is worth (names have been changed to protect the innocent):

1>------ Rebuild All started: Project: TheProject, Configuration: TheProjectConfiguation_clang x64 ------
1>llvm-msvc(v3.0.8) spent 4.193s in TheProjectPCH.cpp
1>llvm-msvc(v3.0.8) spent 0.100s in TheProjectPCH.cpp
1>TheProject.vcxproj -> TheProject.lib
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========
========== Rebuild started at 10:53 AM and took 01:33.943 minutes ==========

But when I compile with 2.8.2 there is of course much more output (and takes ~12s less time to rebuild, whatever that anecdote may mean to you).

@gmh5225
Copy link
Contributor

gmh5225 commented Oct 6, 2023

Can you provide some code so that I can reproduce something like "unused-function"

@kornman00
Copy link
Author

#include <cstdio>

static void this_function_is_unused();

int main()
{
	puts("Hello, World");
}

static void this_function_is_unused()
{
	// This function should generate a -Wunused-function warning under ClangCL
}
1>------ Build started: Project: llvm-msvc-issue-109, Configuration: Debug x64 ------
1>llvm-msvc-issue-109.cpp(10,13): warning : unused function 'this_function_is_unused' [-Wunused-function]
1>llvm-msvc-issue-109.vcxproj -> D:\SourceCode\KS\llvm-msvc-issue-109\x64\Debug\llvm-msvc-issue-109.exe

In the attached minimal repro vcxproj file, if you change <PlatformToolsetUseLlvmMsvcFork> on line 25 to true and ensure <LLVMInstallDir> on line 31 is updated to your llvm-msvc install (I tested today with 3.2.6) you will hit the issue where -Wunused-function is not surfaced.

1>------ Rebuild All started: Project: llvm-msvc-issue-109, Configuration: Debug x64 ------
1>llvm-msvc(v3.2.6) spent 0.040s in llvm-msvc-issue-109.cpp
1>llvm-msvc-issue-109.vcxproj -> D:\SourceCode\KS\llvm-msvc-issue-109\x64\Debug\llvm-msvc-issue-109.exe

Even though I have the warning level set to 4, with warnings as error, and -Wunused-function set to be non-error warning:

  <PropertyGroup Label="LLVM" Condition="$(PlatformToolset)=='ClangCL'">
    <ClangClAdditionalOptions>
      -Wno-error=unused-variable
      -Wno-error=unused-function
      -Wno-error=unused-member-function
      -Wno-error=unused-const-variable
      -Wno-unused-but-set-variable
      -Wno-unused-local-typedef
      -Wno-unused-but-set-parameter
      $(ClangClAdditionalOptions)
    </ClangClAdditionalOptions>
  </PropertyGroup>

llvm-msvc-issue-109.min_repro.zip

@gmh5225
Copy link
Contributor

gmh5225 commented Dec 25, 2023

Thanks for the feedback, I'll take care of this problem right away.

@gmh5225
Copy link
Contributor

gmh5225 commented Dec 25, 2023

image

Did you miss parameters?(-Wunused-function -Wno-error=unused-function) It works for me.
https://github.com/backengineering/llvm-msvc/releases/tag/llvm-msvc-v3.2.6

@kornman00
Copy link
Author

Ah, okay, if I have the following:

  <PropertyGroup Label="LLVM" Condition="$(PlatformToolset)=='ClangCL'">
    <ClangClAdditionalOptions>
      -Wunused-function
      -Wno-error=unused-function
      $(ClangClAdditionalOptions)
    </ClangClAdditionalOptions>
  </PropertyGroup>

Where -Wunused-function is explicitly stated, then it reports the unused function. So, this issue can be closed as fixed.

For reference, where in llvm-msvc's code do you configure these normally-on-by-default warnings to be off by default for the sake of MSVC users that don't expect them? Is it in llvm-msvc specific code, or do you modify the llvm code where the warning is defined directly?

Thanks!

@gmh5225
Copy link
Contributor

gmh5225 commented Mar 3, 2024

The latest version of LLVM-MSVC adds a new option /WX2. You can enable /WX2 to solve your problem
https://github.com/backengineering/llvm-msvc/releases/tag/llvm-msvc-v3.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants