benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, Bigcheese.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When reseting modular options, propagate the values from certain options
that have ImpliedBy relations instead of setting to the default. Also,
verify in clang-scan-deps that the command line produced round trips
exactly.
Ideally we would automatically derive the set of options that need this
kind of propagation, but for now there aren't very many impacted.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143446
Files:
clang/include/clang/Frontend/CompilerInvocation.h
clang/lib/Basic/LangOptions.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/ClangScanDeps/modules-implied-args.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -8,6 +8,7 @@
#include "clang/Driver/Driver.h"
#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
@@ -202,6 +203,12 @@
llvm::cl::init(RDRK_ModifyCompilerPath),
llvm::cl::cat(DependencyScannerCategory));
+llvm::cl::opt<bool>
+ RoundTripArgs("round-trip-args", llvm::cl::Optional,
+ llvm::cl::desc("verify that command-line arguments are "
+ "canonical by parsing and re-serializing"),
+ llvm::cl::cat(DependencyScannerCategory));
+
llvm::cl::opt<bool> Verbose("v", llvm::cl::Optional,
llvm::cl::desc("Use verbose output."),
llvm::cl::init(false),
@@ -280,6 +287,37 @@
Inputs.push_back(std::move(ID));
}
+ bool roundTripCommand(ArrayRef<std::string> ArgStrs,
+ DiagnosticsEngine &Diags) {
+ if (ArgStrs.empty() || ArgStrs[0] != "-cc1")
+ return false;
+ SmallVector<const char *> Args;
+ for (const std::string &Arg : ArgStrs)
+ Args.push_back(Arg.c_str());
+ return !CompilerInvocation::checkCC1RoundTrip(Args, Diags);
+ }
+
+ // Returns \c true if any command lines fail to round-trip. We expect
+ // commands already be canonical when output by the scanner.
+ bool roundTripCommands(raw_ostream &ErrOS) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions{};
+ TextDiagnosticPrinter DiagConsumer(ErrOS, &*DiagOpts);
+ IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+ CompilerInstance::createDiagnostics(&*DiagOpts, &DiagConsumer,
+ /*ShouldOwnClient=*/false);
+
+ for (auto &&M : Modules)
+ if (roundTripCommand(M.second.BuildArguments, *Diags))
+ return true;
+
+ for (auto &&I : Inputs)
+ for (const auto &Cmd : I.Commands)
+ if (roundTripCommand(Cmd.Arguments, *Diags))
+ return true;
+
+ return false;
+ }
+
void printFullOutput(raw_ostream &OS) {
// Sort the modules by name to get a deterministic order.
std::vector<IndexedModuleID> ModuleIDs;
@@ -602,6 +640,15 @@
}
Pool.wait();
+#ifndef NDEBUG
+ bool DoRoundTripDefault = true;
+#else
+ bool DoRoundTripDefault = false;
+#endif
+ if (RoundTripArgs.getNumOccurrences() ? RoundTripArgs : DoRoundTripDefault)
+ if (FD.roundTripCommands(llvm::errs()))
+ HadErrors = true;
+
if (Format == ScanningOutputFormat::Full)
FD.printFullOutput(llvm::outs());
Index: clang/test/ClangScanDeps/modules-implied-args.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-implied-args.c
@@ -0,0 +1,46 @@
+// Check that we get canonical round-trippable command-lines, in particular
+// for the options modified for modules.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full -round-trip-args > %t/result.json
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefixes=CHECK,NEGATIVE
+
+// -ffast-math implies -menable-no-infs, -menable-no-nans, and -mreassociate;
+// those options are modified by resetNonModularOptions.
+
+// NEGATIVE-NOT: "-menable-no-infs"
+// NEGATIVE-NOT: "-menable-no-nans"
+// NEGATIVE-NOT: "-mreassociate"
+
+// CHECK: "modules": [
+// CHECK-NEXT: {
+// CHECK: "clang-module-deps": []
+// CHECK: "command-line": [
+// CHECK: "-ffast-math"
+// CHECK: ]
+// CHECK: "name": "Mod"
+// CHECK: }
+// CHECK-NEXT: ]
+// CHECK: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "commands": [
+// CHECK: {
+// CHECK: "command-line": [
+// CHECK: "-ffast-math"
+// CHECK: ]
+
+//--- cdb.json.template
+[{
+ "file": "DIR/tu.c",
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -ffast-math"
+}]
+
+//--- module.modulemap
+module Mod { header "mod.h" }
+//--- mod.h
+//--- tu.c
+#include "mod.h"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -652,7 +652,9 @@
CompilerInvocation &RealInvocation,
CompilerInvocation &DummyInvocation,
ArrayRef<const char *> CommandLineArgs,
- DiagnosticsEngine &Diags, const char *Argv0) {
+ DiagnosticsEngine &Diags, const char *Argv0,
+ bool CheckAgainstOriginalInvocation = false,
+ bool ForceRoundTrip = false) {
#ifndef NDEBUG
bool DoRoundTripDefault = true;
#else
@@ -660,11 +662,15 @@
#endif
bool DoRoundTrip = DoRoundTripDefault;
- for (const auto *Arg : CommandLineArgs) {
- if (Arg == StringRef("-round-trip-args"))
- DoRoundTrip = true;
- if (Arg == StringRef("-no-round-trip-args"))
- DoRoundTrip = false;
+ if (ForceRoundTrip) {
+ DoRoundTrip = true;
+ } else {
+ for (const auto *Arg : CommandLineArgs) {
+ if (Arg == StringRef("-round-trip-args"))
+ DoRoundTrip = true;
+ if (Arg == StringRef("-no-round-trip-args"))
+ DoRoundTrip = false;
+ }
}
// If round-trip was not requested, simply run the parser with the real
@@ -737,10 +743,13 @@
return false;
}
- // Generate arguments again, this time from the options we will end up using
- // for the rest of the compilation.
SmallVector<const char *> GeneratedArgs2;
- Generate(RealInvocation, GeneratedArgs2, SA);
+ if (CheckAgainstOriginalInvocation)
+ GeneratedArgs2.assign(CommandLineArgs.begin(), CommandLineArgs.end());
+ else
+ // Generate arguments again, this time from the options we will end up using
+ // for the rest of the compilation.
+ Generate(RealInvocation, GeneratedArgs2, SA);
// Compares two lists of generated arguments.
auto Equal = [](const ArrayRef<const char *> A,
@@ -771,6 +780,24 @@
return Success2;
}
+bool CompilerInvocation::checkCC1RoundTrip(ArrayRef<const char *> Args,
+ DiagnosticsEngine &Diags,
+ const char *Argv0) {
+ CompilerInvocation DummyInvocation1, DummyInvocation2;
+ return RoundTrip(
+ [](CompilerInvocation &Invocation, ArrayRef<const char *> CommandLineArgs,
+ DiagnosticsEngine &Diags, const char *Argv0) {
+ return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
+ },
+ [](CompilerInvocation &Invocation, SmallVectorImpl<const char *> &Args,
+ StringAllocator SA) {
+ Args.push_back("-cc1");
+ Invocation.generateCC1CommandLine(Args, SA);
+ },
+ DummyInvocation1, DummyInvocation2, Args, Diags, Argv0,
+ /*CheckAgainstOriginalInvocation=*/true, /*ForceRoundTrip=*/true);
+}
+
static void addDiagnosticArgs(ArgList &Args, OptSpecifier Group,
OptSpecifier GroupWithValue,
std::vector<std::string> &Diagnostics) {
Index: clang/lib/Basic/LangOptions.cpp
===================================================================
--- clang/lib/Basic/LangOptions.cpp
+++ clang/lib/Basic/LangOptions.cpp
@@ -29,6 +29,14 @@
Name = static_cast<unsigned>(Default);
#include "clang/Basic/LangOptions.def"
+ // Reset "benign" options with implied values (Options.td ImpliedBy relations)
+ // rather than their defaults. This avoids unexpected combinations and
+ // invocations that cannot be round-tripped to arguments.
+ // FIXME: we should derive this automatically from ImpliedBy in tablegen.
+ AllowFPReassoc = UnsafeFPMath;
+ NoHonorNaNs = FiniteMathOnly;
+ NoHonorInfs = FiniteMathOnly;
+
// These options do not affect AST generation.
NoSanitizeFiles.clear();
XRayAlwaysInstrumentFiles.clear();
Index: clang/include/clang/Frontend/CompilerInvocation.h
===================================================================
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -241,6 +241,17 @@
/// This is a (less-efficient) wrapper over generateCC1CommandLine().
std::vector<std::string> getCC1CommandLine() const;
+ /// Check that \p Args can be parsed and re-serialized without change,
+ /// emiting diagnostics for any differences.
+ ///
+ /// This check is only suitable for command-lines that are expected to already
+ /// be canonical.
+ ///
+ /// \return false if there are any errors.
+ static bool checkCC1RoundTrip(ArrayRef<const char *> Args,
+ DiagnosticsEngine &Diags,
+ const char *Argv0 = nullptr);
+
/// Reset all of the options that are not considered when building a
/// module.
void resetNonModularOptions();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
