-
Notifications
You must be signed in to change notification settings - Fork 22
refactor: cl arguments always map to regular arguments #1004
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
base: develop
Are you sure you want to change the base?
Changes from 15 commits
1e6db93
9427cf1
bb07fd2
ad395e4
1b05a94
22aae97
0474788
1ce477c
dc42cff
c133111
4d13984
57011a5
2bceef0
b4501f2
d4ba27f
c181b19
0b7c9aa
7aa51ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -219,10 +219,15 @@ isValidMrDocsOption( | |
driver::options::OPT_clang_ignored_gcc_optimization_f_Group, | ||
driver::options::OPT_clang_ignored_legacy_options_Group, | ||
driver::options::OPT_clang_ignored_m_Group, | ||
driver::options::OPT_flang_ignored_w_Group | ||
#if 0 | ||
driver::options::OPT_flang_ignored_w_Group, | ||
driver::options::OPT__SLASH_O, | ||
driver::options::OPT__SLASH_EH, | ||
driver::options::OPT__SLASH_GR, | ||
driver::options::OPT__SLASH_GR_, | ||
driver::options::OPT__SLASH_M_Group, | ||
|
||
// input file options | ||
driver::options::OPT_INPUT, | ||
//driver::options::OPT_INPUT, | ||
|
||
// output file options | ||
driver::options::OPT_o, | ||
|
@@ -237,11 +242,10 @@ isValidMrDocsOption( | |
driver::options::OPT__SLASH_Fr, | ||
driver::options::OPT__SLASH_Fm, | ||
driver::options::OPT__SLASH_Fx, | ||
#endif | ||
// driver::options::OPT__SLASH_TP | ||
// driver::options::OPT__SLASH_Tp | ||
// driver::options::OPT__SLASH_TC | ||
// driver::options::OPT__SLASH_Tc | ||
driver::options::OPT__SLASH_TP, | ||
driver::options::OPT__SLASH_Tp, | ||
driver::options::OPT__SLASH_TC, | ||
driver::options::OPT__SLASH_Tc | ||
)) | ||
{ | ||
return false; | ||
|
@@ -288,7 +292,7 @@ adjustCommandLine( | |
// Copy the compiler path | ||
// ------------------------------------------------------ | ||
std::string const& progName = cmdline.front(); | ||
std::vector new_cmdline = {progName}; | ||
std::vector new_cmdline = {std::string{"clang"}}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. :) That information was so annoying. The only thing it affected was the driver::getDriverMode function. |
||
|
||
// ------------------------------------------------------ | ||
// Convert to InputArgList | ||
|
@@ -300,24 +304,12 @@ adjustCommandLine( | |
cmdLineCStrs.data(), | ||
cmdLineCStrs.data() + cmdLineCStrs.size()); | ||
|
||
// ------------------------------------------------------ | ||
// Get driver mode | ||
// ------------------------------------------------------ | ||
// The driver mode distinguishes between clang/gcc and msvc | ||
// command line option formats. The value is deduced from | ||
// the `-drive-mode` option or from `progName`. | ||
// Common values are "gcc", "g++", "cpp", "cl" and "flang". | ||
StringRef const driver_mode = driver::getDriverMode(progName, cmdLineCStrs); | ||
// Identify if we should use "msvc/clang-cl" or "clang/gcc" format | ||
// for options. | ||
bool const is_clang_cl = driver::IsClangCL(driver_mode); | ||
|
||
// ------------------------------------------------------ | ||
// Supress all warnings | ||
// ------------------------------------------------------ | ||
// Add flags to ignore all warnings. Any options that | ||
// affect warnings will be discarded later. | ||
new_cmdline.emplace_back(is_clang_cl ? "/w" : "-w"); | ||
new_cmdline.emplace_back("-w"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's also great. This |
||
new_cmdline.emplace_back("-fsyntax-only"); | ||
|
||
// ------------------------------------------------------ | ||
|
@@ -411,9 +403,10 @@ adjustCommandLine( | |
isExplicitCCompileCommand || (!isExplicitCppCompileCommand && isImplicitCSourceFile); | ||
|
||
constexpr auto is_std_option = [](std::string_view const opt) { | ||
return opt.starts_with("-std=") || opt.starts_with("--std=") || opt.starts_with("/std:"); | ||
return opt.starts_with("-std=") || opt.starts_with("--std=") || | ||
opt.starts_with("/std:") || opt.starts_with("-std:"); | ||
K-ballo marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
}; | ||
if (std::ranges::find_if(cmdline, is_std_option) == cmdline.end()) | ||
if (std::ranges::none_of(cmdline, is_std_option)) | ||
K-ballo marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) |
||
{ | ||
if (!isCCompileCommand) | ||
{ | ||
|
@@ -449,7 +442,7 @@ adjustCommandLine( | |
for (auto const& inc : it->second) | ||
{ | ||
new_cmdline.emplace_back(std::format("-isystem{}", inc)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
@@ -492,6 +485,18 @@ adjustCommandLine( | |
new_cmdline.emplace_back(std::format("-I{}", inc)); | ||
} | ||
|
||
// ------------------------------------------------------ | ||
// Get driver mode | ||
// ------------------------------------------------------ | ||
// The driver mode distinguishes between clang/gcc and msvc | ||
// command line option formats. The value is deduced from | ||
// the `-drive-mode` option or from `progName`. | ||
// Common values are "gcc", "g++", "cpp", "cl" and "flang". | ||
StringRef const driver_mode = driver::getDriverMode(progName, cmdLineCStrs); | ||
// Identify if we should use "msvc/clang-cl" or "clang/gcc" format | ||
// for options. | ||
bool const is_clang_cl = driver::IsClangCL(driver_mode); | ||
|
||
Comment on lines
+490
to
+501
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic being moved down here is also great. It limits errors that could go up there. |
||
// ------------------------------------------------------ | ||
// Adjust each argument in the command line | ||
// ------------------------------------------------------ | ||
|
@@ -507,18 +512,40 @@ adjustCommandLine( | |
while (idx < cmdline.size()) | ||
{ | ||
// Parse one argument as a Clang option | ||
// ParseOneArg updates Index to the next argument to be parsed. | ||
unsigned const idx0 = idx; | ||
std::unique_ptr<llvm::opt::Arg> arg = | ||
opts_table.ParseOneArg(args, idx, visibility); | ||
if (!isValidMrDocsOption(workingDir, arg)) | ||
{ | ||
continue; | ||
} | ||
new_cmdline.insert( | ||
new_cmdline.end(), | ||
cmdline.begin() + idx0, | ||
cmdline.begin() + idx); | ||
|
||
// Translate clang-cl /std: into clang -std= | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So |
||
const llvm::opt::Option opt = | ||
arg->getOption().getUnaliasedOption(); | ||
if (opt.matches(clang::driver::options::OPT__SLASH_std)) | ||
{ | ||
MRDOCS_ASSERT(arg->getNumValues() == 1); | ||
|
||
std::string_view v = arg->getValue(); | ||
if (v == "c++latest" || v == "c++23preview") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
or
or
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. |
||
{ | ||
new_cmdline.emplace_back("-std=c++23"); | ||
} | ||
else // c++14,c++17,c++20,c11,c17 | ||
{ | ||
new_cmdline.emplace_back(std::format("-std={}", v)); | ||
} | ||
continue; | ||
} | ||
|
||
// Append the translated arguments to the new command line | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really nice. Clang just translates the whole thing. :) |
||
llvm::opt::ArgStringList output; | ||
arg->render(args, output); | ||
|
||
for (auto const& v : output) | ||
{ | ||
new_cmdline.emplace_back(v); | ||
} | ||
} | ||
|
||
return new_cmdline; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
// | ||
// Licensed under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
// Copyright (c) 2023 Vinnie Falco ([email protected]) | ||
// | ||
// Official repository: https://github.com/cppalliance/mrdocs | ||
// | ||
|
||
#include "lib/ConfigImpl.hpp" | ||
#include "lib/MrDocsCompilationDatabase.hpp" | ||
#include "lib/SingleFileDB.hpp" | ||
#include <mrdocs/Support/Path.hpp> | ||
#include <mrdocs/Support/ThreadPool.hpp> | ||
#include <test_suite/test_suite.hpp> | ||
|
||
namespace clang { | ||
namespace mrdocs { | ||
|
||
namespace { | ||
/** Compilation database for a single .cpp file. | ||
*/ | ||
class SingleFileTestDB | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't you can just use the existing one? |
||
: public tooling::CompilationDatabase | ||
{ | ||
tooling::CompileCommand cc_; | ||
|
||
public: | ||
explicit | ||
SingleFileTestDB( | ||
tooling::CompileCommand cc) | ||
: cc_(std::move(cc)) | ||
{} | ||
|
||
std::vector<tooling::CompileCommand> | ||
getCompileCommands( | ||
llvm::StringRef FilePath) const override | ||
{ | ||
if (FilePath != cc_.Filename) | ||
return {}; | ||
return { cc_ }; | ||
} | ||
|
||
std::vector<std::string> | ||
getAllFiles() const override | ||
{ | ||
return { cc_.Filename }; | ||
} | ||
|
||
std::vector<tooling::CompileCommand> | ||
getAllCompileCommands() const override | ||
{ | ||
return { cc_ }; | ||
} | ||
}; | ||
} | ||
|
||
struct MrDocsCompilationDatabase_test | ||
{ | ||
Config::Settings fileSettings_; | ||
ThreadPool threadPool_; | ||
ReferenceDirectories dirs_; | ||
|
||
std::shared_ptr<ConfigImpl const> config_ = | ||
ConfigImpl::load(fileSettings_, dirs_, threadPool_).value(); | ||
|
||
auto adjustCompileCommand(std::vector<std::string> CommandLine) const | ||
{ | ||
tooling::CompileCommand cc; | ||
cc.Directory = "."; | ||
cc.Filename = "test.cpp"; // file does not exist | ||
cc.CommandLine = std::move(CommandLine); | ||
cc.CommandLine.push_back(cc.Filename); | ||
cc.Heuristic = "unit test"; | ||
|
||
// Create an adjusted MrDocsDatabase | ||
std::unordered_map<std::string, std::vector<std::string>> defaultIncludePaths; | ||
MrDocsCompilationDatabase compilations( | ||
llvm::StringRef(cc.Directory), SingleFileTestDB(cc), config_, defaultIncludePaths); | ||
return compilations.getCompileCommands(cc.Filename).front().CommandLine; | ||
} | ||
|
||
static bool has(std::vector<std::string> const& commandLine, std::string_view flag) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have a |
||
{ | ||
return std::find(commandLine.begin(), commandLine.end(), flag) != commandLine.end(); | ||
} | ||
static bool has(std::vector<std::string> const& commandLine, std::initializer_list<std::string_view> flags) | ||
{ | ||
MRDOCS_ASSERT(flags.size() > 0); | ||
return std::search(commandLine.begin(), commandLine.end(), flags.begin(), flags.end()) != commandLine.end(); | ||
} | ||
|
||
void testClangCLStdFlag() | ||
{ | ||
// /std:c++14 -> -std=c++14 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c++14"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c++14")); | ||
} | ||
|
||
// /std:c++17 -> -std=c++17 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c++17"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c++17")); | ||
} | ||
|
||
// /std:c++20 -> -std=c++20 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c++20"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c++20")); | ||
} | ||
|
||
// /std:c++23preview -> -std=c++23 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c++23preview"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c++23")); | ||
} | ||
|
||
// /std:c++latest -> -std=c++23 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c++latest"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c++23")); | ||
} | ||
|
||
// /std:c11 -> -std=c11 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c11"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c11")); | ||
} | ||
|
||
// /std:c17 -> -std=c17 | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/std:c17"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, "-std=c17")); | ||
} | ||
} | ||
|
||
void testClangCLIncludeFlag() | ||
{ | ||
// /I<path> -> -I<path> | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/I<path>"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, {"-I", "<path>"})); | ||
} | ||
|
||
// /external:I<path> -> -isystem<path> | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl", "/external:I<path>"}); | ||
BOOST_TEST_GE(adjusted.size(), 2); | ||
BOOST_TEST(has(adjusted, {"-isystem", "<path>"})); | ||
} | ||
} | ||
|
||
void testClangCL() | ||
{ | ||
auto adjusted = adjustCompileCommand({ "clang-cl"}); | ||
BOOST_TEST_GE(adjusted.size(), 1); | ||
BOOST_TEST_EQ(adjusted[0], "clang"); | ||
|
||
testClangCLStdFlag(); | ||
testClangCLIncludeFlag(); | ||
} | ||
|
||
void run() | ||
{ | ||
testClangCL(); | ||
} | ||
}; | ||
|
||
TEST_SUITE( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your test suite is great. You had nothing to worry about. |
||
MrDocsCompilationDatabase_test, | ||
"clang.mrdocs.MrDocsCompilationDatabase"); | ||
|
||
} // mrdocs | ||
} // clang |
Uh oh!
There was an error while loading. Please reload this page.