shivanshu3 created this revision. shivanshu3 added reviewers: aeubanks, kadircet. Herald added a project: clang. Herald added a subscriber: cfe-commits. shivanshu3 requested review of this revision. Herald added a subscriber: ormris.
MSVC's cl.exe has a few command line arguments which start with -M such as "-MD", "-MDd", "-MT", "-MTd", "-MP". These arguments are not dependency file generation related, and these arguments were being removed by getClangStripDependencyFileAdjuster() which was wrong. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D86999 Files: clang/lib/Tooling/ArgumentsAdjusters.cpp clang/unittests/Tooling/ToolingTest.cpp Index: clang/unittests/Tooling/ToolingTest.cpp =================================================================== --- clang/unittests/Tooling/ToolingTest.cpp +++ clang/unittests/Tooling/ToolingTest.cpp @@ -563,6 +563,39 @@ EXPECT_TRUE(HasFlag("-c")); } +// Check getClangStripDependencyFileAdjuster doesn't strip args when using the +// MSVC cl.exe driver +TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) { + FixedCompilationDatabase Compilations( + "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-MTd", "-MP"}); + + ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc")); + Tool.mapVirtualFile("/a.cc", "void a() {}"); + + std::unique_ptr<FrontendActionFactory> Action( + newFrontendActionFactory<SyntaxOnlyAction>()); + + CommandLineArguments FinalArgs; + ArgumentsAdjuster CheckFlagsAdjuster = + [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) { + FinalArgs = Args; + return Args; + }; + Tool.clearArgumentsAdjusters(); + Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster()); + Tool.appendArgumentsAdjuster(CheckFlagsAdjuster); + Tool.run(Action.get()); + + auto HasFlag = [&FinalArgs](const std::string &Flag) { + return llvm::find(FinalArgs, Flag) != FinalArgs.end(); + }; + EXPECT_TRUE(HasFlag("-MD")); + EXPECT_TRUE(HasFlag("-MDd")); + EXPECT_TRUE(HasFlag("-MT")); + EXPECT_TRUE(HasFlag("-MTd")); + EXPECT_TRUE(HasFlag("-MP")); +} + // Check getClangStripPluginsAdjuster strips plugin related args. TEST(ClangToolTest, StripPluginsAdjuster) { FixedCompilationDatabase Compilations( Index: clang/lib/Tooling/ArgumentsAdjusters.cpp =================================================================== --- clang/lib/Tooling/ArgumentsAdjusters.cpp +++ clang/lib/Tooling/ArgumentsAdjusters.cpp @@ -21,6 +21,17 @@ namespace clang { namespace tooling { +static Optional<StringRef> getDriverMode(const CommandLineArguments &Args) { + for (const auto &Arg : Args) { + StringRef ArgRef = Arg; + if (ArgRef.startswith("--driver-mode=")) { + auto EqualSignIndex = ArgRef.find('='); + return StringRef(ArgRef.data() + EqualSignIndex + 1); + } + } + return StringRef(); +} + /// Add -fsyntax-only option and drop options that triggers output generation. ArgumentsAdjuster getClangSyntaxOnlyAdjuster() { return [](const CommandLineArguments &Args, StringRef /*unused*/) { @@ -93,13 +104,27 @@ ArgumentsAdjuster getClangStripDependencyFileAdjuster() { return [](const CommandLineArguments &Args, StringRef /*unused*/) { + auto DriverMode = getDriverMode(Args); + auto UsingClDriver = (DriverMode.hasValue() && *DriverMode == "cl"); + CommandLineArguments AdjustedArgs; for (size_t i = 0, e = Args.size(); i < e; ++i) { StringRef Arg = Args[i]; // All dependency-file options begin with -M. These include -MM, // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD. - if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") && - !Arg.startswith("-showIncludes")) { + // Dependency-file options in MSVC do not begin with -M, so we + // do not remove those when using the cl driver. + bool IsDependencyFileArg; + if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes")) + IsDependencyFileArg = true; + else if (UsingClDriver) + IsDependencyFileArg = false; + else if (Arg.startswith("-M")) + IsDependencyFileArg = true; + else + IsDependencyFileArg = false; + + if (!IsDependencyFileArg) { AdjustedArgs.push_back(Args[i]); continue; }
Index: clang/unittests/Tooling/ToolingTest.cpp =================================================================== --- clang/unittests/Tooling/ToolingTest.cpp +++ clang/unittests/Tooling/ToolingTest.cpp @@ -563,6 +563,39 @@ EXPECT_TRUE(HasFlag("-c")); } +// Check getClangStripDependencyFileAdjuster doesn't strip args when using the +// MSVC cl.exe driver +TEST(ClangToolTest, StripDependencyFileAdjusterMsvc) { + FixedCompilationDatabase Compilations( + "/", {"--driver-mode=cl", "-MD", "-MDd", "-MT", "-MTd", "-MP"}); + + ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc")); + Tool.mapVirtualFile("/a.cc", "void a() {}"); + + std::unique_ptr<FrontendActionFactory> Action( + newFrontendActionFactory<SyntaxOnlyAction>()); + + CommandLineArguments FinalArgs; + ArgumentsAdjuster CheckFlagsAdjuster = + [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) { + FinalArgs = Args; + return Args; + }; + Tool.clearArgumentsAdjusters(); + Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster()); + Tool.appendArgumentsAdjuster(CheckFlagsAdjuster); + Tool.run(Action.get()); + + auto HasFlag = [&FinalArgs](const std::string &Flag) { + return llvm::find(FinalArgs, Flag) != FinalArgs.end(); + }; + EXPECT_TRUE(HasFlag("-MD")); + EXPECT_TRUE(HasFlag("-MDd")); + EXPECT_TRUE(HasFlag("-MT")); + EXPECT_TRUE(HasFlag("-MTd")); + EXPECT_TRUE(HasFlag("-MP")); +} + // Check getClangStripPluginsAdjuster strips plugin related args. TEST(ClangToolTest, StripPluginsAdjuster) { FixedCompilationDatabase Compilations( Index: clang/lib/Tooling/ArgumentsAdjusters.cpp =================================================================== --- clang/lib/Tooling/ArgumentsAdjusters.cpp +++ clang/lib/Tooling/ArgumentsAdjusters.cpp @@ -21,6 +21,17 @@ namespace clang { namespace tooling { +static Optional<StringRef> getDriverMode(const CommandLineArguments &Args) { + for (const auto &Arg : Args) { + StringRef ArgRef = Arg; + if (ArgRef.startswith("--driver-mode=")) { + auto EqualSignIndex = ArgRef.find('='); + return StringRef(ArgRef.data() + EqualSignIndex + 1); + } + } + return StringRef(); +} + /// Add -fsyntax-only option and drop options that triggers output generation. ArgumentsAdjuster getClangSyntaxOnlyAdjuster() { return [](const CommandLineArguments &Args, StringRef /*unused*/) { @@ -93,13 +104,27 @@ ArgumentsAdjuster getClangStripDependencyFileAdjuster() { return [](const CommandLineArguments &Args, StringRef /*unused*/) { + auto DriverMode = getDriverMode(Args); + auto UsingClDriver = (DriverMode.hasValue() && *DriverMode == "cl"); + CommandLineArguments AdjustedArgs; for (size_t i = 0, e = Args.size(); i < e; ++i) { StringRef Arg = Args[i]; // All dependency-file options begin with -M. These include -MM, // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD. - if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") && - !Arg.startswith("-showIncludes")) { + // Dependency-file options in MSVC do not begin with -M, so we + // do not remove those when using the cl driver. + bool IsDependencyFileArg; + if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes")) + IsDependencyFileArg = true; + else if (UsingClDriver) + IsDependencyFileArg = false; + else if (Arg.startswith("-M")) + IsDependencyFileArg = true; + else + IsDependencyFileArg = false; + + if (!IsDependencyFileArg) { AdjustedArgs.push_back(Args[i]); continue; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits