ubisoft / Sharpmake

Sharpmake is an open-source C#-based solution for generating project definition files, such as Visual Studio projects and solutions, GNU makefiles, Xcode projects, etc.
Apache License 2.0
927 stars 171 forks source link

Clang toolset support in VS2019 #88

Closed sammyfreg closed 3 years ago

sammyfreg commented 4 years ago

While trying to generate projects with 'Clang' compiler support, I noticed that the built-in LLVM toolset support in VS2019 is not working when using : conf.Options.Add(Sharpmake.Options.Vc.General.PlatformToolset.LLVM);

To fix this, I had to make 2 changes in Sharpmake code, making the result identical to what VisualStudio set on a new project with LLVM toolset :

  1. Change the toolset name. Sharpmake currently set 'llvm' in the .vcxprj file but VC2019 expects 'ClangCL' ProjectOptionsGenerator.cs : 1133 Options.Option(Options.Vc.General.PlatformToolset.LLVM, () => { context.Options["PlatformToolset"] = "ClangCL"; })
  2. Set the default Clang tool path to '$(LLVMInstallDir)' Util.cs : 1739 return GetRegistryLocalMachineSubKeyValue(registryKeyString, null, @"$(LLVMInstallDir)"); // null to get default

It could also be nice of the documentation mentioned how to activate the LLVM toolset.

belkiss commented 4 years ago

Heya!

Interesting! Sharpmake's support for clang-cl has been mostly tested with vs2017, using the extension from LLVM https://marketplace.visualstudio.com/items?itemName=LLVMExtensions.llvm-toolchain

Is the ClangCL value you mention the one to use for activating the version that you can install with the visual studio installer?

Agreed that the documentation is lacking...

Thanks for the heads-up!

sammyfreg commented 4 years ago

Yes, I don't have any version of Clang installed other than the one you can select when installing VS2019 and making these 2 changes got me past the toolchain associated errors. Now that VS ofter the option to use Clang painlesslly, a lot more people are probably going to try it.

belkiss commented 4 years ago

In that case and in order to keep backward compat we'll need to add another entry in the PlatformToolset enum.

belkiss commented 3 years ago

Support is in the dev branch https://github.com/ubisoft/Sharpmake/commit/d93823ee, will be in the next release.

sammyfreg commented 3 years ago

I have been testing it and still having issues.

Testcase

Executable Dir Before : C:\Program Files\LLVM\bin;C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\bin\HostX64\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.16299.0\x64;$(PATH) After : $(LLVMInstallDir)\bin;$(ExecutablePath)

Include Dir : C:\Program Files\LLVM\lib\clang\7.0.0\lib\windows;C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\lib\x64;C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\atlmfc\lib\x64;;C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\ucrt\x64;C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\um\x64; After : $(LLVMInstallDir)\x64\lib\clang\10.0.0\include;$(IncludePath)

Library Dir : C:\Program Files\LLVM\lib\clang\7.0.0\lib\windows;C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\lib\x64;C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\atlmfc\lib\x64;;C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\ucrt\x64;C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\um\x64; After : $(VC_LibraryPath_x64);$(WindowsSDK_LibraryPath_x64)

It feels that for VisualStudio projects, leaving these value blank would be a safer way to go (unless user wants a custom Clang installation).

Replacing C:\Program Files\LLVM with $(LLVMInstallDir) is not sufficient, I end up with compile issues related to findind basic headers 1>..\..\..\Code\ThirdParty\Imgui_17800/imgui.h(58,10): fatal error : 'string.h' file not found. I believe this last point occured with latest version of VS2019, since I also started having the same problem with my previously working modified Sharpmake, since my VS reinstall.

sammyfreg commented 3 years ago

I had to make a small change to the latest branch, for Clang support. In BaseWindowsPlatform.cs I commented out lines 112 - 117 to disable changing the default values.

                if (platformToolset.IsLLVMToolchain())
                {
                    Options.Vc.General.PlatformToolset overridenPlatformToolset = Options.Vc.General.PlatformToolset.Default;
                    if (Options.WithArgOption<Options.Vc.General.PlatformToolset>.Get<Options.Clang.Compiler.LLVMVcPlatformToolset>(conf, ref overridenPlatformToolset))
                        platformToolset = overridenPlatformToolset;

                    devEnv = platformToolset.GetDefaultDevEnvForToolset() ?? devEnv;
                #if 0
                    context.Options["ExecutablePath"] = ClangForWindows.GetWindowsClangExecutablePath() + ";" + devEnv.GetWindowsExecutablePath(conf.Platform);
                    if (Options.GetObject<Options.Vc.LLVM.UseClangCl>(conf) == Options.Vc.LLVM.UseClangCl.Enable)
                    {
                        context.Options["IncludePath"] = ClangForWindows.GetWindowsClangIncludePath() + ";" + devEnv.GetWindowsIncludePath();
                        context.Options["LibraryPath"] = ClangForWindows.GetWindowsClangLibraryPath() + ";" + devEnv.GetWindowsLibraryPath(conf.Platform, Util.IsDotNet(conf) ? conf.Target.GetFragment<DotNetFramework>() : default(DotNetFramework?));
                    }
                #endif
                }
jonathansty commented 3 years ago

I was having a similar issue as @sammyfreg. It seems it's using the wrong paths here, removing the code mentioned also fixes this for me.

belkiss commented 3 years ago

Haven't found the time to test that yet, but would be great if someone could contribute a small sample that enables clang-cl :)

sammyfreg commented 3 years ago

My previous comment has repro for this issue.

You can Download NetImgui and try generating its solution.

belkiss commented 3 years ago

Will do, thanks!

belkiss commented 3 years ago

Heya! Just to let you know that I fixed support for the ClangCL PlatformToolset in the dev branch, by actually removing the whole block of code writing the overrides (c167bc584b3acfbc3a35a187822786f9ceffd87d), since with the improved WinSDK path overrides happening in the vcxproj it's useless.

Some additional fixes were required for everything to behave correctly: e6ae5fed2374ed05a5a66e32cedd1dc466363a28

And I also added a small sample HelloClangCl that shows how to set it up with both MSBuild and FastBuild support in one solution: 5b55a56030119be4b81a776ab6423ca6fb1a5ba8

Hope with all of that we can manage to have it working properly :) Cheers!

sammyfreg commented 3 years ago

I got the latest version and can confirm the Clang support work as is.

Thank you!

belkiss commented 3 years ago

Good to hear! Thanks for the confirmation :)