Quellcode durchsuchen

Enable debug info by default (#4232)

Discussed in the toolchain meeting today - we'd like to try having this
on by default and see if the cost isn't too high.

The nodebug test is a bit verbose, because it doesn't have the
`--exclude-dump-file-prefix` that test_file would usually add. Is there
a nicer way I could write this test to verify that --no-debug-info does
what it's meant to?
David Blaikie vor 1 Jahr
Ursprung
Commit
1e1034ee81

+ 5 - 0
testing/file_test/README.md

@@ -123,6 +123,11 @@ Supported comment markers are:
 
         Replaced with `${TEST_TMPDIR}/temp_file`.
 
+    -   `%{identifier}`
+
+        Replaces some implementation-specific identifier with a value. (Mappings
+        provided by way of an optional `MyFileTest::GetArgReplacements`)
+
     ARGS can be specified at most once. If not provided, the FileTestBase child
     is responsible for providing default arguments.
 

+ 16 - 0
testing/file_test/file_test_base.cpp

@@ -332,6 +332,7 @@ auto FileTestBase::ProcessTestFileAndRun(TestContext& context)
 auto FileTestBase::DoArgReplacements(
     llvm::SmallVector<std::string>& test_args,
     const llvm::SmallVector<TestFile>& test_files) -> ErrorOr<Success> {
+  auto replacements = GetArgReplacements();
   for (auto* it = test_args.begin(); it != test_args.end(); ++it) {
     auto percent = it->find("%");
     if (percent == std::string::npos) {
@@ -362,6 +363,21 @@ auto FileTestBase::DoArgReplacements(
         it->replace(percent, 2, llvm::formatv("{0}/temp_file", tmpdir));
         break;
       }
+      case '{': {
+        auto end_brace = it->find('}', percent);
+        if (end_brace == std::string::npos) {
+          return ErrorBuilder() << "%{ without closing }: " << *it;
+        }
+        llvm::StringRef substr(&*(it->begin() + percent + 2),
+                               end_brace - percent - 2);
+        auto replacement = replacements.find(substr);
+        if (replacement == replacements.end()) {
+          return ErrorBuilder()
+                 << "unknown substitution: %{" << substr << "}: " << *it;
+        }
+        it->replace(percent, end_brace - percent + 1, replacement->second);
+        break;
+      }
       default:
         return ErrorBuilder() << "%" << c << " is not supported: " << *it;
     }

+ 7 - 0
testing/file_test/file_test_base.h

@@ -14,6 +14,7 @@
 #include "common/ostream.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "testing/file_test/autoupdate.h"
@@ -86,6 +87,12 @@ class FileTestBase : public testing::Test {
   // Returns default arguments. Only called when a file doesn't set ARGS.
   virtual auto GetDefaultArgs() -> llvm::SmallVector<std::string> = 0;
 
+  // Returns a map of string replacements to implement `%{key}` -> `value` in
+  // arguments.
+  virtual auto GetArgReplacements() -> llvm::StringMap<std::string> {
+    return {};
+  }
+
   // Returns a regex to match the default file when a line may not be present.
   // May return nullptr if unused. If GetLineNumberReplacements returns an entry
   // with has_file=false, this is required.

+ 3 - 0
testing/file_test/file_test_base_test.cpp

@@ -136,6 +136,9 @@ class FileTestBaseTest : public FileTestBase {
       return ErrorBuilder() << "Unexpected file: " << filename;
     }
   }
+  auto GetArgReplacements() -> llvm::StringMap<std::string> override {
+    return {{"replacement", "replaced"}};
+  }
 
   auto GetDefaultArgs() -> llvm::SmallVector<std::string> override {
     return {"default_args", "%s"};

+ 2 - 2
testing/file_test/testdata/args.carbon

@@ -2,11 +2,11 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-// ARGS: abc file=%t %s
+// ARGS: abc file=%t %s prefix_%{replacement}_suffix
 // AUTOUPDATE
 // TIP: To test this file alone, run:
 // TIP:   bazel test //testing/file_test:file_test_base_test --test_arg=--file_tests=testing/file_test/testdata/args.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //testing/file_test:file_test_base_test -- --dump_output --file_tests=testing/file_test/testdata/args.carbon
 
-// CHECK:STDOUT: 3 args: `abc`, `file={{.+}}/temp_file`, `args.carbon`
+// CHECK:STDOUT: 4 args: `abc`, `file={{.+}}/temp_file`, `args.carbon`, `prefix_replaced_suffix`

+ 5 - 8
toolchain/driver/driver.cpp

@@ -344,7 +344,10 @@ Excludes files with the given prefix from dumps.
 Emit DWARF debug information.
 )""",
         },
-        [&](auto& arg_b) { arg_b.Set(&include_debug_info); });
+        [&](auto& arg_b) {
+          arg_b.Default(true);
+          arg_b.Set(&include_debug_info);
+        });
   }
 
   Phase phase;
@@ -366,7 +369,7 @@ Emit DWARF debug information.
   bool preorder_parse_tree = false;
   bool builtin_sem_ir = false;
   bool prelude_import = false;
-  bool include_debug_info = false;
+  bool include_debug_info = true;
 
   llvm::StringRef exclude_dump_file_prefix;
 };
@@ -532,12 +535,6 @@ auto Driver::ValidateCompileOptions(const CompileOptions& options) const
                       << options.phase << "'.\n";
         return false;
       }
-      if (options.include_debug_info) {
-        error_stream_
-            << "ERROR: Requested debug info but compile phase is limited to '"
-            << options.phase << "'.\n";
-        return false;
-      }
       [[fallthrough]];
     case Phase::Lower:
     case Phase::CodeGen:

+ 6 - 5
toolchain/driver/testdata/fail_flags.carbon → toolchain/lower/testdata/debug/nodebug.carbon

@@ -2,12 +2,13 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-// ARGS: compile --debug-info --phase=check %s
+// ARGS: compile --no-debug-info --phase=lower --dump-llvm-ir --output=- --exclude-dump-file-prefix=%{core_package_dir} %s
 //
 // AUTOUPDATE
 // TIP: To test this file alone, run:
-// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_flags.carbon
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/debug/nodebug.carbon
 // TIP: To dump output, run:
-// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_flags.carbon
-// CHECK:STDERR: ERROR: Requested debug info but compile phase is limited to 'check'.
-//
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/debug/nodebug.carbon
+
+// CHECK:STDOUT: ; ModuleID = 'nodebug.carbon'
+// CHECK:STDOUT: source_filename = "nodebug.carbon"

+ 4 - 1
toolchain/testing/file_test.cpp

@@ -29,6 +29,10 @@ class ToolchainFileTest : public FileTestBase {
         component_(GetComponent(test_name)),
         installation_(InstallPaths::MakeForBazelRunfiles(exe_path)) {}
 
+  auto GetArgReplacements() -> llvm::StringMap<std::string> override {
+    return {{"core_package_dir", installation_.core_package()}};
+  }
+
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
            llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
            llvm::raw_pwrite_stream& stderr) -> ErrorOr<RunResult> override {
@@ -70,7 +74,6 @@ class ToolchainFileTest : public FileTestBase {
       args.push_back("--dump-sem-ir");
     } else if (component_ == "lower") {
       args.push_back("--dump-llvm-ir");
-      args.push_back("--debug-info");
     } else {
       CARBON_FATAL() << "Unexpected test component " << component_ << ": "
                      << test_name();