NewWorldComingSoon / llvm-msvc-issues

Issues about llvm-msvc can be sent here
https://llvm-msvc.win
GNU General Public License v3.0
5 stars 0 forks source link

[clang] unused-function diagnostic regression? #109

Open kornman00 opened 1 year ago

kornman00 commented 1 year ago

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?

kornman00 commented 1 year ago

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.

kornman00 commented 11 months ago

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).

kornman00 commented 8 months ago
#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

kornman00 commented 8 months ago

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!