refactor: cl arguments always map to regular arguments #1004

K-ballo · 2025-09-10T09:49:14Z

as discussed in #997

cppalliance-bot · 2025-09-10T09:52:25Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

K-ballo · 2025-09-10T09:57:22Z

@alandefreitas please advice on how to test this (so that we are informed when the tools stops working on Windows)

cppalliance-bot · 2025-09-10T10:38:47Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-10T11:07:32Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-10T12:17:30Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-11T13:12:58Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-12T07:12:28Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-16T13:12:42Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-16T13:57:36Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

vinniefalco · 2025-09-16T14:04:32Z

This will be squashed I hope?

Copilot

Pull Request Overview

This PR removes MSVC emulation mode from the MrDocs compilation database handling, simplifying command line processing by always using Clang's native format instead of supporting both Clang and MSVC/clang-cl command line styles.

Removes driver mode detection and conditional command line formatting
Adds explicit translation of clang-cl /std: flags to Clang -std= format
Updates option filtering to exclude more MSVC-specific flags

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/lib/MrDocsCompilationDatabase.cpp	Core changes removing MSVC emulation and adding clang-cl flag translation
src/test/lib/MrDocsCompilationDatabase.cpp	New test file validating clang-cl flag translation functionality

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/lib/MrDocsCompilationDatabase.cpp

cppalliance-bot · 2025-09-16T14:07:36Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-16T14:17:29Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-18T08:17:33Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

cppalliance-bot · 2025-09-21T19:47:34Z

An automated preview of the documentation is available at https://1004.mrdocs.prtest2.cppalliance.org/index.html

alandefreitas · 2025-09-21T19:51:54Z

This is great. These ternary operators with is_clang_cl were error-prone.

feat: drop msvc emulation mode #1004

This title is misleading to someone who doesn't understand the problem being fixed here because it sounds like MrDocs won't recognise compilation commands in the MSVC format. What's happening is just that all commands will be converted to the same format so that we can ensure the same behaviour and so that options included by MrDocs are less prone to errors. But that's internal and transparent to the user.

Also, this it's is more of a "refactor" than a "feat" because nothing is changing in terms of MrDocs expected behavior. It could also be a "fix" in the sense that (I think) it's fixing an issue with nostdinc in passing.

@alandefreitas please advice on how to test this (so that we are informed when the tools stops working on Windows)

What I would do is upsert a test suite for MrDocsCompilationDatabase (I don't know if it already exists) and then compare the input and output. Things should remain the same when it's not MSVC mode, and things should change when it's in MSVC mode. I wouldn't touch the golden tests at all.

src/lib/MrDocsCompilationDatabase.cpp

alandefreitas · 2025-09-21T19:54:58Z

src/lib/MrDocsCompilationDatabase.cpp

+            opt.starts_with("/std:") || opt.starts_with("-std:"); // clang-cl options
    };
-    if (std::ranges::find_if(cmdline, is_std_option) == cmdline.end())
+    if (std::ranges::none_of(cmdline, is_std_option))


src/lib/MrDocsCompilationDatabase.cpp

alandefreitas · 2025-09-21T20:01:59Z

src/lib/MrDocsCompilationDatabase.cpp

-            cmdline.begin() + idx0,
-            cmdline.begin() + idx);
+
+        // Translate clang-cl /std: into clang -std=


So arg->render() works for all kinds of arguments except -std (not that bad, but it would be good to include a small comment)? Or does that mean arg->render() requires custom logic for lots of kinds of arguments, but the only one for which we got custom logic is -std (much worse)?

The later, not all arguments can be automatically translated.

Oh.... OK.

requires custom logic for lots of kinds of arguments

So what are we going to do about this? Do we know the kinds of arguments that can't be translated? Do we know what happens when there's one of them? Do we know how to handle them?

Whenever an argument can't be translated clang will emit an error. The argument then has to either be added to the ignore list defined inside isValidMrDocsOption, or custom handling logic has to be added here.

alandefreitas · 2025-09-21T20:02:37Z

src/lib/MrDocsCompilationDatabase.cpp

+            continue;
+        }
+
+        // Append the translated arguments to the new command line


This is really nice. Clang just translates the whole thing. :)

alandefreitas · 2025-09-21T20:08:47Z

src/lib/MrDocsCompilationDatabase.cpp

+            MRDOCS_ASSERT(arg->getNumValues() == 1);
+
+            std::string_view v = arg->getValue();
+            if (v == "c++latest" || v == "c++23preview")


This is cumbersome. Since we have access to the whole Clang library, I'm sure there should be some way to know what the highest std is. Even if it's some enum of standards somewhere, we can evaluate an inline constexpr requires to check if STD_NUMBER exists in that enum.

A few ways to get the highest stardard could be:

#include "clang/Frontend/LangStandards.def"

or

clang::getLangStandardForName("c++23")

or

bool ok = clang::CompilerInvocation::CreateFromArgs( CI, {"-std=c++23"}, *Diags );

there must be any other ways.

These functions only give you if one specific standard is there, but you can just iterate 23 + X * 3 until you find the highest valid number, potentially at compile time.

src/test/lib/MrDocsCompilationDatabase.cpp

alandefreitas · 2025-09-21T20:10:10Z

src/test/lib/MrDocsCompilationDatabase.cpp

+    }
+};
+
+TEST_SUITE(


Your test suite is great. You had nothing to worry about.

alandefreitas · 2025-09-21T20:11:11Z

src/test/lib/MrDocsCompilationDatabase.cpp

+        return compilations.getCompileCommands(cc.Filename).front().CommandLine;
+    }
+
+    static bool has(std::vector<std::string> const& commandLine, std::string_view flag)


We already have a contains function in Algorithm.hpp IIRC.

alandefreitas · 2025-09-22T17:31:03Z

Oh... There's also some logic in the GenerateAction that infers the implicit include paths depending on the compiler. We have to check if this is still working correctly after we changed the first arg to "clang".

K-ballo · 2025-09-29T15:27:24Z

I believe the current PR title is not appropriate. This is not a simple refactor, this is a massive functionality change on windows: we are explicitly dropping clang-cl behavior, by only ever invoking normal clang behavior.

alandefreitas · 2025-09-29T18:09:42Z

I believe the current PR title is not appropriate. This is not a simple refactor, this is a massive functionality change on windows: we are explicitly dropping clang-cl behavior, by only ever invoking normal clang behavior.

Yes. From your last comment it was clear it's not worth pursuing this path at all:

Whenever an argument can't be translated clang will emit an error. The argument then has to be either added to the ignore list defined inside isValidMrDocsOption, or custom handling logic has to be added here.

We can't have code that doesn't work with a subset of flags, especially when we don't know what the subset is, and we know the consequences are catastrophic. Users shouldn't have to open an issue because of random flags that failed.

K-ballo requested a review from alandefreitas September 10, 2025 09:49

K-ballo force-pushed the drop-msvc-emulation branch from 6d43aed to 92ae4ff Compare September 12, 2025 07:05

K-ballo added 12 commits September 16, 2025 10:31

avoid msvc emulation

1e6db93

skip some msvc args

9427cf1

translate args

bb07fd2

tweak

ad395e4

ignore more msvc options

1b05a94

more...

22aae97

fix unused

0474788

Translate clang-cl /std: into clang -std=

1ce477c

typo

dc42cff

tweaks

c133111

ignore some more

4d13984

draft up test suite for adjustCommandLine

57011a5

K-ballo force-pushed the drop-msvc-emulation branch from 92ae4ff to 57011a5 Compare September 16, 2025 13:08

K-ballo added 2 commits September 16, 2025 15:20

test /std

2bceef0

test /I

b4501f2

internal linkage

d4ba27f

K-ballo marked this pull request as ready for review September 16, 2025 14:03

vinniefalco requested a review from Copilot September 16, 2025 14:04

Copilot AI reviewed Sep 16, 2025

View reviewed changes

src/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

src/lib/MrDocsCompilationDatabase.cpp Outdated Show resolved Hide resolved

src/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

comment

c181b19

ignore more cl flags

0b7c9aa

Merge branch 'develop' into drop-msvc-emulation

7aa51ec

alandefreitas reviewed Sep 21, 2025

View reviewed changes

src/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

alandefreitas reviewed Sep 21, 2025

View reviewed changes

src/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

alandefreitas reviewed Sep 21, 2025

View reviewed changes

src/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

alandefreitas reviewed Sep 21, 2025

View reviewed changes

src/test/lib/MrDocsCompilationDatabase.cpp Show resolved Hide resolved

alandefreitas reviewed Sep 21, 2025

View reviewed changes

src/test/lib/MrDocsCompilationDatabase.cpp

}

};

TEST_SUITE(

Copy link

Collaborator

alandefreitas Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your test suite is great. You had nothing to worry about.

alandefreitas reviewed Sep 21, 2025

View reviewed changes

alandefreitas changed the title ~~feat: drop msvc emulation mode~~ refactor: cl arguments always map to regular arguments Sep 22, 2025

alandefreitas closed this Sep 29, 2025

alandefreitas mentioned this pull request Oct 1, 2025

fix(ci): setup C++ when building releases #1041

Draft

refactor: cl arguments always map to regular arguments #1004

refactor: cl arguments always map to regular arguments #1004

Uh oh!

Conversation

K-ballo commented Sep 10, 2025

Uh oh!

cppalliance-bot commented Sep 10, 2025

Uh oh!

K-ballo commented Sep 10, 2025

Uh oh!

cppalliance-bot commented Sep 10, 2025

Uh oh!

cppalliance-bot commented Sep 10, 2025

Uh oh!

cppalliance-bot commented Sep 10, 2025

Uh oh!

cppalliance-bot commented Sep 11, 2025

Uh oh!

cppalliance-bot commented Sep 12, 2025

Uh oh!

cppalliance-bot commented Sep 16, 2025

Uh oh!

cppalliance-bot commented Sep 16, 2025

Uh oh!

vinniefalco commented Sep 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cppalliance-bot commented Sep 16, 2025

Uh oh!

cppalliance-bot commented Sep 16, 2025

Uh oh!

cppalliance-bot commented Sep 18, 2025

Uh oh!

cppalliance-bot commented Sep 21, 2025

Uh oh!

alandefreitas commented Sep 21, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alandefreitas Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alandefreitas commented Sep 22, 2025

Uh oh!

K-ballo commented Sep 29, 2025

Uh oh!

alandefreitas commented Sep 29, 2025

Uh oh!

Uh oh!

alandefreitas Sep 21, 2025 •

edited

Loading