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

Setup ClangCL PlatformToolset #4

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kornman00
Copy link

This is the initial work to support building with ClangCL under VS2022 (I don't believe VS2019 proper ever had ClangCL support). There are no code changes yet, as I would prefer to supply those in a subsequent PR and not hold up this rudimentary groundwork. E.g., trying to build DebugClangCL will not work right now, as there are clang-compat issues in the code that need to be addressed first.

Aside from ClangCL support, I also consolidated the vcxproj properties (DRY principal and all), use v143 out of the box under VS2022 (although if this doesn't sit right with you, I can remove that change...I was able to run under v143 without issue, on VS2022 17.8.3), and moved the default debugger properties to the vcxproj and removed the committed .user file.

As of this comment, VS2022's clang-cl toolset is based on LLVM 16.0.5. I have not had major issues with this version on a different project which I was updating to C++20 (for constexpr and saner STL features, like string_view, span, etc). FWIW however, there's an actively maintained fork of LLVM that explicitly targets MSVC compatibility: https://github.com/backengineering/llvm-msvc. Although, I've had issues with some of their changes, e.g. NewWorldComingSoon/llvm-msvc-issues#109. That related to unused-function warnings being implicitly silenced without being able to turn them back on. On a large project with lots of legacy code (that is no longer relevant), that is less than ideal.

* Remove duplicated properties.
* Remove committed .user file, default debugger properties are defined in the vcxproj itself now.
* Use folder based patterns for gitignore'ing most files, and add .user files to the ignore list.
* Remove the default "Source Files", "Header Files", and "Resource Files" filter folders and place files in the root of the vcxproj, since the filter patterns were not being observed.
You need to have "MSBuild support for LLVM (clang-cl) toolset" installed under VS2022.

These currently do not compile, as there are issues in the code that Clang does not like that need to be addressed in future commits.
@richgel999
Copy link
Owner

Thanks! Will review soon and merge in.

@kornman00
Copy link
Author

I've done the work today to get DebugClangCL compiling (haven't yet committed it, since it would update this PR I believe).

Some things of note ahead of time:

  • I changed the <LanguageStandard> from the implicit default (stdcpp14) to stdcpp17. Namely, to leverage static_assert without an explicit message. I came across one assert() which could have been statically validated. I could switch it back to stdcpp14 if you prefer, though.
    • For the utils like string_format / vformat, C++20 introduced std::format. If that specific language addition is okay with you, I could evaluate enabling C++20 post-clang_cl and updating formatted prints to std::format. Likewise, with stdcpp17 std::string_view is viable, which could replace some areas where you're having to construct or pass std::string around. Though, I can respect being averse to some of the committee's decisions.
  • I had to resave at least two source files as UTF8 (with BOM) as Clang saw sequences like u8"ValléeMagonia" as containing UTF16 characters and called fowl on the character sequence.
  • There are currently unused functions (-Wunused-functions) in various places. E.g., ufojson.cpp's detect_bad_urls(). I've marked these up with the [[maybe_unused]] attribute to silence these warnings for now, as I expect these are probably functions written in some prototype project before being committed to github, or are being used in uncommitted changes on your end.
  • There are some signed/unsigned and implicit casts warnings across the codebase which I'm trying to address. Ideally, to get to a place where <TreatWarningAsError> can be enabled.
  • I'm noticing some generated files in bin\ after I run are appearing as modified to use CRLF while in the repo they are apparently LF...not sure if that's something goofy on my end, e.g. maybe a misconfigured git setting to deal with crlf quirks.

Please let me know if anything mentioned does not jive with your expectations of contributions. Thanks!

Change the language standard from MSVC's default stdcpp14 to stdcpp17. Haven't evaluated what issues may or may not be present when going to stdcpp20.
Enable string pooling and multiprocessor compilation.
Use C++11 `[[fallthrough]]` and `[[maybe_unused]]` attributes.
Use std::size for getting lengths of C arrays at compile time.
Resave converters.cpp and ufojson_core.cpp as UTF8 with BOM.
Address various signed/unsigned warnings.
Add nipcap_date_is_year_valid helper to deal with year values coming in as `int` but the constants being `uint32_t` (signed/unsigned mismatches).

Fix constructor member initialization order issue in pjson.h.
Explicitly handle some cJSONValueType's which have no conversions to silence unhandled enums warnings.

Fix missing comma in g_cap_exceptions list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants