From ea60e4f491e68dae8c7e1490376f35984cc2a492 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 19 Jul 2023 16:07:53 -0700 Subject: [PATCH] Track the file kind on SourceLocation rather than computing it from the file name. (#2999) This will be more correct if a user file is named prelude.carbon, and reduces the explorer runtime by about 10% by removing a string comparison from the check for whether trace output is enabled. --------- Co-authored-by: Jon Ross-Perkins --- explorer/ast/ast_test_matchers_test.cpp | 2 +- explorer/ast/element_test.cpp | 2 +- explorer/ast/expression_test.cpp | 2 +- explorer/ast/pattern_test.cpp | 2 +- explorer/common/BUILD | 1 + explorer/common/error_builders_test.cpp | 2 +- .../common/set_file_context_raii_test.cpp | 42 +++++-------- explorer/common/source_location.h | 21 +++++-- explorer/common/trace_stream.h | 63 +++++-------------- explorer/file_test.cpp | 2 +- explorer/fuzzing/ast_to_proto_main.cpp | 2 +- explorer/fuzzing/ast_to_proto_test.cpp | 13 ++-- explorer/interpreter/action_stack.cpp | 8 ++- explorer/interpreter/exec_program.cpp | 2 +- explorer/interpreter/interpreter.cpp | 19 ++++-- explorer/main.cpp | 45 ++++++++++--- .../parse_and_execute/parse_and_execute.cpp | 5 +- .../parse_and_execute_test.cpp | 6 +- explorer/syntax/BUILD | 1 + explorer/syntax/parse.cpp | 18 +++--- explorer/syntax/parse.h | 7 ++- explorer/syntax/parse_and_lex_context.h | 13 +++- explorer/syntax/parse_test.cpp | 5 +- explorer/syntax/prelude.cpp | 6 +- .../syntax/unimplemented_example_test.cpp | 17 ++--- explorer/testdata/limits/fail_allocate.carbon | 2 +- .../limits/fail_function_recursion.carbon | 2 +- explorer/testdata/limits/fail_loop.carbon | 2 +- .../limits/fail_type_check_loop.carbon | 2 +- 29 files changed, 168 insertions(+), 146 deletions(-) diff --git a/explorer/ast/ast_test_matchers_test.cpp b/explorer/ast/ast_test_matchers_test.cpp index b59872454b79..64a1c8bf4742 100644 --- a/explorer/ast/ast_test_matchers_test.cpp +++ b/explorer/ast/ast_test_matchers_test.cpp @@ -21,7 +21,7 @@ using ::testing::ElementsAre; using ::testing::IsEmpty; using ::testing::Not; -static constexpr SourceLocation DummyLoc("dummy", 0); +static constexpr SourceLocation DummyLoc("dummy", 0, FileKind::Main); TEST(BlockContentsAreTest, BasicUsage) { Block empty_block(DummyLoc, {}); diff --git a/explorer/ast/element_test.cpp b/explorer/ast/element_test.cpp index 7d7491ae2439..da1c8663a104 100644 --- a/explorer/ast/element_test.cpp +++ b/explorer/ast/element_test.cpp @@ -20,7 +20,7 @@ namespace Carbon::Testing { namespace { static auto FakeSourceLoc(int line_num) -> SourceLocation { - return SourceLocation("", line_num); + return SourceLocation("", line_num, FileKind::Main); } class ElementTest : public ::testing::Test { diff --git a/explorer/ast/expression_test.cpp b/explorer/ast/expression_test.cpp index 883d30799e58..d3e39786ff4e 100644 --- a/explorer/ast/expression_test.cpp +++ b/explorer/ast/expression_test.cpp @@ -24,7 +24,7 @@ using testing::IsEmpty; MATCHER(IntField, "") { return arg->kind() == ExpressionKind::IntLiteral; } static auto FakeSourceLoc(int line_num) -> SourceLocation { - return SourceLocation("", line_num); + return SourceLocation("", line_num, FileKind::Main); } class ExpressionTest : public ::testing::Test { diff --git a/explorer/ast/pattern_test.cpp b/explorer/ast/pattern_test.cpp index 419cda0c02cf..f3c1c00005eb 100644 --- a/explorer/ast/pattern_test.cpp +++ b/explorer/ast/pattern_test.cpp @@ -24,7 +24,7 @@ using testing::IsEmpty; MATCHER(AutoField, "") { return isa(*arg); } static auto FakeSourceLoc(int line_num) -> SourceLocation { - return SourceLocation("", line_num); + return SourceLocation("", line_num, FileKind::Main); } class PatternTest : public ::testing::Test { diff --git a/explorer/common/BUILD b/explorer/common/BUILD index 7bb67ba78182..b48202372b31 100644 --- a/explorer/common/BUILD +++ b/explorer/common/BUILD @@ -58,6 +58,7 @@ cc_library( "//common:check", "//common:ostream", "//explorer/common:nonnull", + "@llvm-project//llvm:Support", ], ) diff --git a/explorer/common/error_builders_test.cpp b/explorer/common/error_builders_test.cpp index e52574d4531b..2425cdde4f79 100644 --- a/explorer/common/error_builders_test.cpp +++ b/explorer/common/error_builders_test.cpp @@ -13,7 +13,7 @@ namespace Carbon::Testing { namespace { TEST(ErrorBuildersTest, ProgramError) { - Error err = ProgramError(SourceLocation("x", 1)) << "test"; + Error err = ProgramError(SourceLocation("x", 1, FileKind::Main)) << "test"; EXPECT_EQ(err.location(), "x:1"); EXPECT_EQ(err.message(), "test"); diff --git a/explorer/common/set_file_context_raii_test.cpp b/explorer/common/set_file_context_raii_test.cpp index 50c3d3d9bf6e..61a1b8c86c26 100644 --- a/explorer/common/set_file_context_raii_test.cpp +++ b/explorer/common/set_file_context_raii_test.cpp @@ -10,37 +10,27 @@ namespace Carbon::Testing { namespace { -// TODO: write test cases to distinguish between file context of main file and -// an import once imports are supported. - -TEST(SetFileContextRaiiTest, Simple) { - TraceStream trace_stream; - { - SetFileContext set_file_ctx(trace_stream, - SourceLocation("example/main.carbon", 9)); - EXPECT_TRUE(trace_stream.file_context() == FileContext::Main); - } - - // Considering the file context for a trace stream is FileContext::Unknown by - // default, as the default value of source location in a trace stream is - // std::nullopt. - EXPECT_TRUE(trace_stream.file_context() == FileContext::Unknown); -} - TEST(SetFileContextRaiiTest, UpdateFileContext) { TraceStream trace_stream; + trace_stream.set_stream(&llvm::nulls()); + trace_stream.set_allowed_phases({ProgramPhase::All}); + trace_stream.set_allowed_file_kinds({FileKind::Main}); + { - SetFileContext set_file_ctx(trace_stream, - SourceLocation("example/prelude.carbon", 9)); - EXPECT_TRUE(trace_stream.file_context() == FileContext::Prelude); - set_file_ctx.update_source_loc(SourceLocation("example/main.carbon", 9)); - EXPECT_TRUE(trace_stream.file_context() == FileContext::Main); + SetFileContext set_file_ctx( + trace_stream, + SourceLocation("example/prelude.carbon", 9, FileKind::Prelude)); + EXPECT_FALSE(trace_stream.is_enabled()); + set_file_ctx.update_source_loc( + SourceLocation("example/main.carbon", 9, FileKind::Main)); + EXPECT_TRUE(trace_stream.is_enabled()); + set_file_ctx.update_source_loc( + SourceLocation("example/import.carbon", 9, FileKind::Import)); + EXPECT_FALSE(trace_stream.is_enabled()); } - // Considering the file context for a trace stream is FileContext::Unknown by - // default, as the default value of source location in a trace stream is - // std::nullopt. - EXPECT_TRUE(trace_stream.file_context() == FileContext::Unknown); + // The trace stream should be enabled when we're not in any particular file. + EXPECT_TRUE(trace_stream.is_enabled()); } } // namespace diff --git a/explorer/common/source_location.h b/explorer/common/source_location.h index cc60434476a0..aa3232472c76 100644 --- a/explorer/common/source_location.h +++ b/explorer/common/source_location.h @@ -13,20 +13,25 @@ namespace Carbon { +// Describes the kind of file that the source location is within. +enum class FileKind { Main, Prelude, Import, Unknown, Last = Unknown }; + class SourceLocation { public: // Produce a source location that is known to not be used, because it is fed // into an operation that creates no AST nodes and whose diagnostics are // discarded. static auto DiagnosticsIgnored() -> SourceLocation { - return SourceLocation("", 0); + return SourceLocation("", 0, FileKind::Unknown); } // The filename should be eternal or arena-allocated to eliminate copies. - constexpr SourceLocation(const char* filename, int line_num) - : filename_(filename), line_num_(line_num) {} - SourceLocation(Nonnull filename, int line_num) - : filename_(*filename), line_num_(line_num) {} + explicit constexpr SourceLocation(const char* filename, int line_num, + FileKind file_kind) + : filename_(filename), line_num_(line_num), file_kind_(file_kind) {} + explicit SourceLocation(Nonnull filename, int line_num, + FileKind file_kind) + : filename_(*filename), line_num_(line_num), file_kind_(file_kind) {} SourceLocation(const SourceLocation&) = default; SourceLocation(SourceLocation&&) = default; @@ -34,11 +39,14 @@ class SourceLocation { auto operator=(SourceLocation&&) -> SourceLocation& = default; auto operator==(SourceLocation other) const -> bool { - return filename_ == other.filename_ && line_num_ == other.line_num_; + return filename_ == other.filename_ && line_num_ == other.line_num_ && + file_kind_ == other.file_kind_; } auto filename() const -> std::string_view { return filename_; } + auto file_kind() const -> FileKind { return file_kind_; } + void Print(llvm::raw_ostream& out) const { out << filename_ << ":" << line_num_; } @@ -53,6 +61,7 @@ class SourceLocation { private: std::string_view filename_; int line_num_; + FileKind file_kind_; }; } // namespace Carbon diff --git a/explorer/common/trace_stream.h b/explorer/common/trace_stream.h index 14e859081e5f..fc47f802e6bf 100644 --- a/explorer/common/trace_stream.h +++ b/explorer/common/trace_stream.h @@ -14,6 +14,7 @@ #include "common/ostream.h" #include "explorer/common/nonnull.h" #include "explorer/common/source_location.h" +#include "llvm/ADT/ArrayRef.h" namespace Carbon { @@ -35,47 +36,19 @@ enum class ProgramPhase { Last = All // Last program phase indicator. }; -// Enumerates the contexts for different types of files, used for tracing and -// controlling for which file contexts tracing should be enabled. -enum class FileContext { Unknown, Main, Prelude, Import, All, Last = All }; - // Encapsulates the trace stream so that we can cleanly disable tracing while // the prelude is being processed. The prelude is expected to take a // disproprotionate amount of time to log, so we try to avoid it. -// -// TODO: While the prelude is combined with the provided program as a single -// AST, the AST knows which declarations came from the prelude. When the prelude -// is fully treated as a separate file, we should be able to take a different -// approach where the caller explicitly toggles tracing when switching file -// contexts. class TraceStream { public: - explicit TraceStream() { set_allowed_file_contexts({FileContext::Unknown}); } - - // This method gets the file context by using filename from source location. - // TODO: implement a way to differentiate between the main file and imports - // based upon source location / filename. - auto file_context() const -> FileContext { - if (source_loc_.has_value()) { - auto filename = - llvm::StringRef(source_loc_->filename()).rsplit("/").second; - if (filename == "prelude.carbon") { - return FileContext::Prelude; - } else { - return FileContext::Main; - } - } else { - return FileContext::Unknown; - } - }; + explicit TraceStream() {} // Returns true if tracing is currently enabled. - // TODO: use current source location for file context based filtering instead - // of just checking if current code context is Prelude. auto is_enabled() const -> bool { return stream_.has_value() && !in_prelude_ && allowed_phases_[static_cast(current_phase_)] && - allowed_file_contexts_[static_cast(file_context())]; + (!source_loc_ || + allowed_file_kinds_[static_cast(source_loc_->file_kind())]); } // Sets whether the prelude is being skipped. @@ -90,7 +63,8 @@ class TraceStream { current_phase_ = current_phase; } - auto set_allowed_phases(std::vector allowed_phases_list) { + auto set_allowed_phases(llvm::ArrayRef allowed_phases_list) + -> void { if (allowed_phases_list.empty()) { allowed_phases_.set(static_cast(ProgramPhase::Execution)); } else { @@ -104,28 +78,19 @@ class TraceStream { } } - auto set_allowed_file_contexts(std::vector contexts_list) - -> void { - if (contexts_list.empty()) { - allowed_file_contexts_.set(static_cast(FileContext::Main)); - } else { - for (auto context : contexts_list) { - if (context == FileContext::All) { - allowed_file_contexts_.set(); - } else { - allowed_file_contexts_.set(static_cast(context)); - } - } + auto set_allowed_file_kinds(llvm::ArrayRef kind_list) -> void { + for (auto kind : kind_list) { + allowed_file_kinds_.set(static_cast(kind)); } } - auto set_source_loc(std::optional source_loc) { + auto set_source_loc(std::optional source_loc) -> void { source_loc_ = source_loc; } - auto source_loc() -> std::optional { return source_loc_; } - - auto allowed_phases() { return allowed_phases_; } + auto source_loc() const -> std::optional { + return source_loc_; + } // Returns the internal stream. Requires is_enabled. auto stream() const -> llvm::raw_ostream& { @@ -149,7 +114,7 @@ class TraceStream { std::optional source_loc_ = std::nullopt; std::optional> stream_; std::bitset(ProgramPhase::Last) + 1> allowed_phases_; - std::bitset(FileContext::Last) + 1> allowed_file_contexts_; + std::bitset(FileKind::Last) + 1> allowed_file_kinds_; }; // This is a RAII class to set the current program phase, destructor invocation diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 1edf59020a21..4704cdd46a76 100644 --- a/explorer/file_test.cpp +++ b/explorer/file_test.cpp @@ -57,7 +57,7 @@ class ParseAndExecuteTestFile : public FileTestBase { if (trace_) { trace_stream.set_stream(is_trace_test ? &stdout : &trace_stream_ostream); trace_stream.set_allowed_phases({ProgramPhase::All}); - trace_stream.set_allowed_file_contexts({FileContext::Main}); + trace_stream.set_allowed_file_kinds({FileKind::Main}); } // Set the location of the prelude. diff --git a/explorer/fuzzing/ast_to_proto_main.cpp b/explorer/fuzzing/ast_to_proto_main.cpp index ed3d096729d5..ebdf1ef48b6d 100644 --- a/explorer/fuzzing/ast_to_proto_main.cpp +++ b/explorer/fuzzing/ast_to_proto_main.cpp @@ -38,7 +38,7 @@ auto Main(int argc, char** argv) -> ErrorOr { proto_file.close(); Arena arena; - const ErrorOr ast = Parse(&arena, argv[1], + const ErrorOr ast = Parse(&arena, argv[1], FileKind::Main, /*parser_debug=*/false); if (!ast.ok()) { return ErrorBuilder() << "Parsing failed: " << ast.error().message(); diff --git a/explorer/fuzzing/ast_to_proto_test.cpp b/explorer/fuzzing/ast_to_proto_test.cpp index d06dd9f6b6b0..8563cc2de537 100644 --- a/explorer/fuzzing/ast_to_proto_test.cpp +++ b/explorer/fuzzing/ast_to_proto_test.cpp @@ -99,7 +99,8 @@ TEST(AstToProtoTest, SetsAllProtoFields) { Fuzzing::Carbon merged_proto; for (const llvm::StringRef f : *carbon_files) { Arena arena; - const ErrorOr ast = Parse(&arena, f, /*parser_debug=*/false); + const ErrorOr ast = + Parse(&arena, f, FileKind::Main, /*parser_debug=*/false); if (ast.ok()) { merged_proto.MergeFrom(AstToProto(*ast)); } @@ -136,7 +137,8 @@ TEST(AstToProtoTest, Roundtrip) { int parsed_ok_count = 0; for (const llvm::StringRef f : *carbon_files) { Arena arena; - const ErrorOr ast = Parse(&arena, f, /*parser_debug=*/false); + const ErrorOr ast = + Parse(&arena, f, FileKind::Main, /*parser_debug=*/false); if (ast.ok()) { ++parsed_ok_count; const std::string source_from_proto = @@ -144,8 +146,8 @@ TEST(AstToProtoTest, Roundtrip) { SCOPED_TRACE(testing::Message() << "Carbon file: " << f << ", source from proto:\n" << source_from_proto); - const ErrorOr ast_from_proto = - ParseFromString(&arena, f, source_from_proto, /*parser_debug=*/false); + const ErrorOr ast_from_proto = ParseFromString( + &arena, f, FileKind::Main, source_from_proto, /*parser_debug=*/false); if (ast_from_proto.ok()) { EXPECT_EQ(AstToString(*ast), AstToString(*ast_from_proto)); @@ -175,7 +177,8 @@ TEST(AstToProtoTest, SameProtoAfterClone) { int parsed_ok_count = 0; for (const llvm::StringRef f : *carbon_files) { Arena arena; - const ErrorOr ast = Parse(&arena, f, /*parser_debug=*/false); + const ErrorOr ast = + Parse(&arena, f, FileKind::Main, /*parser_debug=*/false); if (ast.ok()) { ++parsed_ok_count; const AST clone = CloneAST(arena, *ast); diff --git a/explorer/interpreter/action_stack.cpp b/explorer/interpreter/action_stack.cpp index b40fcd693b0d..f58b77cc8b6f 100644 --- a/explorer/interpreter/action_stack.cpp +++ b/explorer/interpreter/action_stack.cpp @@ -275,8 +275,10 @@ void ActionStack::PushCleanUpActions( while (!actions.empty()) { auto& act = actions.top(); if (act->scope()) { + // TODO: Provide a real source location. std::unique_ptr cleanup_action = std::make_unique( - std::move(*act->scope()), SourceLocation("stack cleanup", 1)); + std::move(*act->scope()), + SourceLocation("stack cleanup", 1, FileKind::Unknown)); todo_.Push(std::move(cleanup_action)); } actions.pop(); @@ -286,8 +288,10 @@ void ActionStack::PushCleanUpActions( void ActionStack::PushCleanUpAction(std::unique_ptr act) { auto& scope = act->scope(); if (scope && act->kind() != Action::Kind::CleanUpAction) { + // TODO: Provide a real source location. std::unique_ptr cleanup_action = std::make_unique( - std::move(*scope), SourceLocation("stack cleanup", 1)); + std::move(*scope), + SourceLocation("stack cleanup", 1, FileKind::Unknown)); todo_.Push(std::move(cleanup_action)); } } diff --git a/explorer/interpreter/exec_program.cpp b/explorer/interpreter/exec_program.cpp index 5c9f298724b5..debecefc725a 100644 --- a/explorer/interpreter/exec_program.cpp +++ b/explorer/interpreter/exec_program.cpp @@ -34,7 +34,7 @@ auto AnalyzeProgram(Nonnull arena, AST ast, } } - SourceLocation source_loc("", 0); + SourceLocation source_loc("", 0, FileKind::Main); ast.main_call = arena->New( source_loc, arena->New(source_loc, "Main"), arena->New(source_loc)); diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index 9aa3a4f6d15d..f3c9dd500914 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -2620,7 +2620,7 @@ auto Interpreter::StepDestroy() -> ErrorOr { const Address object = destroy_act.location()->address(); const Address var_addr = object.ElementAddress(arena_->New(var)); - const auto v = heap_.Read(var_addr, SourceLocation("destructor", 1)); + const auto v = heap_.Read(var_addr, var->source_loc()); CARBON_CHECK(v.ok()) << "Failed to read member `" << var->binding().name() << "` from class `" << class_decl.name() << "`"; @@ -2706,21 +2706,28 @@ auto Interpreter::StepCleanUp() -> ErrorOr { // State transition. auto Interpreter::Step() -> ErrorOr { + Action& act = todo_.CurrentAction(); + + auto error_builder = [&] { + if (auto loc = act.source_loc()) { + return ProgramError(*loc); + } + return ErrorBuilder(); + }; + // Check for various overflow conditions before stepping. if (todo_.size() > MaxTodoSize) { - return ProgramError(SourceLocation("overflow", 1)) + return error_builder() << "stack overflow: too many interpreter actions on stack"; } if (++steps_taken_ > MaxStepsTaken) { - return ProgramError(SourceLocation("overflow", 1)) + return error_builder() << "possible infinite loop: too many interpreter steps executed"; } if (arena_->allocated() > MaxArenaAllocated) { - return ProgramError(SourceLocation("overflow", 1)) - << "out of memory: exceeded arena allocation limit"; + return error_builder() << "out of memory: exceeded arena allocation limit"; } - Action& act = todo_.CurrentAction(); switch (act.kind()) { case Action::Kind::LocationAction: CARBON_RETURN_IF_ERROR(StepLocation()); diff --git a/explorer/main.cpp b/explorer/main.cpp index 4f26fccb0dc1..476ee5fc8cbf 100644 --- a/explorer/main.cpp +++ b/explorer/main.cpp @@ -50,7 +50,7 @@ auto ExplorerMain(int argc, char** argv, void* static_for_main_addr, "trace_file", cl::desc("Output file for tracing; set to `-` to output to stdout.")); - cl::list allowed_program_phases( + cl::list trace_phases( "trace_phase", cl::desc("Select the program phases to include in the output. By " "default, only the execution trace will be added to the trace " @@ -81,19 +81,20 @@ auto ExplorerMain(int argc, char** argv, void* static_for_main_addr, "Include trace output for all phases.")), cl::CommaSeparated); - cl::list allowed_file_contexts( + enum class TraceFileContext { Main, Prelude, Import, All }; + cl::list trace_file_contexts( "trace_file_context", cl::desc("Select file contexts for which you want to include the trace " "output"), cl::values( clEnumValN( - FileContext::Main, "main", + TraceFileContext::Main, "main", "Include trace output for file containing the main function"), - clEnumValN(FileContext::Prelude, "prelude", + clEnumValN(TraceFileContext::Prelude, "prelude", "Include trace output for prelude"), - clEnumValN(FileContext::Import, "import", + clEnumValN(TraceFileContext::Import, "import", "Include trace output for imports"), - clEnumValN(FileContext::All, "all", + clEnumValN(TraceFileContext::All, "all", "Include trace output for all files")), cl::CommaSeparated); @@ -116,9 +117,35 @@ auto ExplorerMain(int argc, char** argv, void* static_for_main_addr, TraceStream trace_stream; if (!trace_file_name.empty()) { - // Adding allowed phases in the trace_stream - trace_stream.set_allowed_phases(allowed_program_phases); - trace_stream.set_allowed_file_contexts(allowed_file_contexts); + // Adding allowed phases in the trace_stream. + trace_stream.set_allowed_phases(trace_phases); + + // Translate --trace_file_context setting into a list of FileKinds. + llvm::SmallVector trace_file_kinds = {FileKind::Unknown}; + if (!trace_file_contexts.getNumOccurrences()) { + trace_file_kinds.push_back(FileKind::Main); + } else { + for (auto context : trace_file_contexts) { + switch (context) { + case TraceFileContext::Main: + trace_file_kinds.push_back(FileKind::Main); + break; + case TraceFileContext::Prelude: + trace_file_kinds.push_back(FileKind::Prelude); + break; + case TraceFileContext::Import: + trace_file_kinds.push_back(FileKind::Import); + break; + case TraceFileContext::All: + trace_file_kinds.push_back(FileKind::Main); + trace_file_kinds.push_back(FileKind::Prelude); + trace_file_kinds.push_back(FileKind::Import); + break; + } + } + } + trace_stream.set_allowed_file_kinds(trace_file_kinds); + if (trace_file_name == "-") { trace_stream.set_stream(&llvm::outs()); } else { diff --git a/explorer/parse_and_execute/parse_and_execute.cpp b/explorer/parse_and_execute/parse_and_execute.cpp index 24a61a5474e2..a5a9eeb026ed 100644 --- a/explorer/parse_and_execute/parse_and_execute.cpp +++ b/explorer/parse_and_execute/parse_and_execute.cpp @@ -93,7 +93,7 @@ auto ParseAndExecuteFile(std::string_view prelude_path, Nonnull print_stream) -> ErrorOr { auto parse = [&](Arena* arena) { - return Parse(arena, input_file_name, parser_debug); + return Parse(arena, input_file_name, FileKind::Main, parser_debug); }; return ParseAndExecuteHelper(parse, prelude_path, trace_stream, print_stream); } @@ -104,7 +104,8 @@ auto ParseAndExecute(std::string_view prelude_path, Nonnull trace_stream, Nonnull print_stream) -> ErrorOr { auto parse = [&](Arena* arena) { - return ParseFromString(arena, input_file_name, file_contents, parser_debug); + return ParseFromString(arena, input_file_name, FileKind::Main, + file_contents, parser_debug); }; return ParseAndExecuteHelper(parse, prelude_path, trace_stream, print_stream); } diff --git a/explorer/parse_and_execute/parse_and_execute_test.cpp b/explorer/parse_and_execute/parse_and_execute_test.cpp index 1cb241c78601..923dbf93c474 100644 --- a/explorer/parse_and_execute/parse_and_execute_test.cpp +++ b/explorer/parse_and_execute/parse_and_execute_test.cpp @@ -11,6 +11,7 @@ namespace Carbon::Testing { namespace { using ::testing::Eq; +using ::testing::MatchesRegex; TEST(ParseAndExecuteTest, Recursion) { std::string source = R"( @@ -36,9 +37,10 @@ TEST(ParseAndExecuteTest, Recursion) { ParseAndExecute("explorer/data/prelude.carbon", "test.carbon", source, /*parser_debug=*/false, &trace_stream, &llvm::nulls()); ASSERT_FALSE(err.ok()); + // Don't expect any particular source location for the error. EXPECT_THAT(err.error().message(), - Eq("RUNTIME ERROR: overflow:1: stack overflow: too many " - "interpreter actions on stack")); + MatchesRegex("RUNTIME ERROR:.* stack overflow: too many " + "interpreter actions on stack")); } } // namespace diff --git a/explorer/syntax/BUILD b/explorer/syntax/BUILD index e2a402ee69d7..d5d5ade15a08 100644 --- a/explorer/syntax/BUILD +++ b/explorer/syntax/BUILD @@ -88,6 +88,7 @@ cc_library( "//explorer/common:arena", "//explorer/common:error_builders", "//explorer/common:nonnull", + "//explorer/common:source_location", "@llvm-project//llvm:Support", ], ) diff --git a/explorer/syntax/parse.cpp b/explorer/syntax/parse.cpp index ebe98ddeffc8..69ef8825e913 100644 --- a/explorer/syntax/parse.cpp +++ b/explorer/syntax/parse.cpp @@ -15,12 +15,12 @@ namespace Carbon { static auto ParseImpl(yyscan_t scanner, Nonnull arena, - std::string_view input_file_name, bool parser_debug) - -> ErrorOr { + std::string_view input_file_name, FileKind file_kind, + bool parser_debug) -> ErrorOr { // Prepare other parser arguments. std::optional ast = std::nullopt; ParseAndLexContext context(arena->New(input_file_name), - parser_debug); + file_kind, parser_debug); // Do the parse. auto parser = Parser(arena, scanner, context, &ast); @@ -43,11 +43,11 @@ static auto ParseImpl(yyscan_t scanner, Nonnull arena, } auto Parse(Nonnull arena, std::string_view input_file_name, - bool parser_debug) -> ErrorOr { + FileKind file_kind, bool parser_debug) -> ErrorOr { std::string name_str(input_file_name); FILE* input_file = fopen(name_str.c_str(), "r"); if (input_file == nullptr) { - return ProgramError(SourceLocation(name_str.c_str(), 0)) + return ProgramError(SourceLocation(name_str.c_str(), 0, file_kind)) << "Error opening file: " << std::strerror(errno); } @@ -58,7 +58,7 @@ auto Parse(Nonnull arena, std::string_view input_file_name, yy_switch_to_buffer(buffer, scanner); ErrorOr result = - ParseImpl(scanner, arena, input_file_name, parser_debug); + ParseImpl(scanner, arena, input_file_name, file_kind, parser_debug); // Clean up the lexer. yy_delete_buffer(buffer, scanner); @@ -69,8 +69,8 @@ auto Parse(Nonnull arena, std::string_view input_file_name, } auto ParseFromString(Nonnull arena, std::string_view input_file_name, - std::string_view file_contents, bool parser_debug) - -> ErrorOr { + FileKind file_kind, std::string_view file_contents, + bool parser_debug) -> ErrorOr { // Prepare the lexer. yyscan_t scanner; yylex_init(&scanner); @@ -79,7 +79,7 @@ auto ParseFromString(Nonnull arena, std::string_view input_file_name, yy_switch_to_buffer(buffer, scanner); ErrorOr result = - ParseImpl(scanner, arena, input_file_name, parser_debug); + ParseImpl(scanner, arena, input_file_name, file_kind, parser_debug); // Clean up the lexer. yy_delete_buffer(buffer, scanner); diff --git a/explorer/syntax/parse.h b/explorer/syntax/parse.h index 880dd0fc9fbe..bce0fffc6a11 100644 --- a/explorer/syntax/parse.h +++ b/explorer/syntax/parse.h @@ -10,20 +10,21 @@ #include "explorer/ast/ast.h" #include "explorer/common/arena.h" +#include "explorer/common/source_location.h" namespace Carbon { // Returns the AST representing the contents of the named file, or an error code // if parsing fails. Allocations go into the provided arena. auto Parse(Nonnull arena, std::string_view input_file_name, - bool parser_debug) -> ErrorOr; + FileKind file_kind, bool parser_debug) -> ErrorOr; // Equivalent to `Parse`, but parses the contents of `file_contents`. // `input_file_name` is used only for reporting source locations, and does // not need to name a real file. auto ParseFromString(Nonnull arena, std::string_view input_file_name, - std::string_view file_contents, bool parser_debug) - -> ErrorOr; + FileKind file_kind, std::string_view file_contents, + bool parser_debug) -> ErrorOr; } // namespace Carbon diff --git a/explorer/syntax/parse_and_lex_context.h b/explorer/syntax/parse_and_lex_context.h index fd63e53e839e..6c5551bb2863 100644 --- a/explorer/syntax/parse_and_lex_context.h +++ b/explorer/syntax/parse_and_lex_context.h @@ -8,6 +8,7 @@ #include #include "explorer/ast/ast.h" +#include "explorer/common/source_location.h" #include "explorer/syntax/parser.h" // from parser.ypp namespace Carbon { @@ -18,8 +19,10 @@ class ParseAndLexContext { public: // Creates an instance analyzing the given input file. ParseAndLexContext(Nonnull input_file_name, - bool parser_debug) - : input_file_name_(input_file_name), parser_debug_(parser_debug) {} + FileKind file_kind, bool parser_debug) + : input_file_name_(input_file_name), + file_kind_(file_kind), + parser_debug_(parser_debug) {} // Formats ands records a lexing oor parsing error. Returns an error token as // a convenience. @@ -28,7 +31,8 @@ class ParseAndLexContext { auto source_loc() const -> SourceLocation { return SourceLocation(input_file_name_, - static_cast(current_token_position.begin.line)); + static_cast(current_token_position.begin.line), + file_kind_); } auto parser_debug() const -> bool { return parser_debug_; } @@ -47,6 +51,9 @@ class ParseAndLexContext { // when *this is called. Nonnull input_file_name_; + // The kind of file being processed. + FileKind file_kind_; + bool parser_debug_; std::vector errors_; diff --git a/explorer/syntax/parse_test.cpp b/explorer/syntax/parse_test.cpp index fd7eaab5b389..ca8f0db40830 100644 --- a/explorer/syntax/parse_test.cpp +++ b/explorer/syntax/parse_test.cpp @@ -23,8 +23,9 @@ fn Foo() {} TEST(ParseTest, ParseFromString) { Arena arena; - ErrorOr parse_result = ParseFromString( - &arena, "file.carbon", FileContents, /*parser_debug=*/false); + ErrorOr parse_result = + ParseFromString(&arena, "file.carbon", FileKind::Main, FileContents, + /*parser_debug=*/false); ASSERT_TRUE(parse_result.ok()); EXPECT_EQ(parse_result->declarations.size(), 1); } diff --git a/explorer/syntax/prelude.cpp b/explorer/syntax/prelude.cpp index 3a17792263ef..90116026997f 100644 --- a/explorer/syntax/prelude.cpp +++ b/explorer/syntax/prelude.cpp @@ -12,10 +12,12 @@ namespace Carbon { void AddPrelude(std::string_view prelude_file_name, Nonnull arena, std::vector>* declarations, int* num_prelude_declarations) { - ErrorOr parse_result = Parse(arena, prelude_file_name, false); + ErrorOr parse_result = + Parse(arena, prelude_file_name, FileKind::Prelude, false); if (!parse_result.ok()) { // Try again with tracing, to help diagnose the problem. - ErrorOr trace_parse_result = Parse(arena, prelude_file_name, true); + ErrorOr trace_parse_result = + Parse(arena, prelude_file_name, FileKind::Prelude, true); CARBON_FATAL() << "Failed to parse prelude:\n" << trace_parse_result.error(); } diff --git a/explorer/syntax/unimplemented_example_test.cpp b/explorer/syntax/unimplemented_example_test.cpp index 57167f4b7102..a91361bf292a 100644 --- a/explorer/syntax/unimplemented_example_test.cpp +++ b/explorer/syntax/unimplemented_example_test.cpp @@ -22,14 +22,15 @@ TEST(UnimplementedExampleTest, VerifyPrecedence) { } )"; Arena arena; - EXPECT_THAT(ParseFromString(&arena, "dummy.carbon", Program, false), - ParsedAs(ASTDeclarations( - ElementsAre(MatchesFunctionDeclaration().WithBody( - BlockContentsAre(ElementsAre(MatchesReturn(MatchesEq( - MatchesUnimplementedExpression( - "ExampleInfix", ElementsAre(MatchesLiteral(1), - MatchesLiteral(2))), - MatchesLiteral(3)))))))))); + EXPECT_THAT( + ParseFromString(&arena, "dummy.carbon", FileKind::Main, Program, false), + ParsedAs( + ASTDeclarations(ElementsAre(MatchesFunctionDeclaration().WithBody( + BlockContentsAre(ElementsAre(MatchesReturn(MatchesEq( + MatchesUnimplementedExpression( + "ExampleInfix", + ElementsAre(MatchesLiteral(1), MatchesLiteral(2))), + MatchesLiteral(3)))))))))); } } // namespace diff --git a/explorer/testdata/limits/fail_allocate.carbon b/explorer/testdata/limits/fail_allocate.carbon index cb66560286b4..f596f4832a1f 100644 --- a/explorer/testdata/limits/fail_allocate.carbon +++ b/explorer/testdata/limits/fail_allocate.carbon @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE -// CHECK:STDERR: RUNTIME ERROR: overflow:1: possible infinite loop: too many interpreter steps executed package EmptyIdentifier impl; @@ -13,6 +12,7 @@ fn Main() -> i32 { // allocations before hitting max steps. Maybe with string operations we // could trigger actual excessive memory allocations by just doubling the // size of the string each time. + // CHECK:STDERR: RUNTIME ERROR: fail_allocate.carbon:[[@LINE+1]]: possible infinite loop: too many interpreter steps executed heap.New((0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0)); } return 0; diff --git a/explorer/testdata/limits/fail_function_recursion.carbon b/explorer/testdata/limits/fail_function_recursion.carbon index 9f9cea711b0c..2d6a1df0bd49 100644 --- a/explorer/testdata/limits/fail_function_recursion.carbon +++ b/explorer/testdata/limits/fail_function_recursion.carbon @@ -3,11 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE -// CHECK:STDERR: RUNTIME ERROR: overflow:1: stack overflow: too many interpreter actions on stack package EmptyIdentifier impl; fn A() { + // CHECK:STDERR: RUNTIME ERROR: fail_function_recursion.carbon:[[@LINE+1]]: stack overflow: too many interpreter actions on stack A(); } diff --git a/explorer/testdata/limits/fail_loop.carbon b/explorer/testdata/limits/fail_loop.carbon index 9e40a83c2f1d..57a5bd9b4593 100644 --- a/explorer/testdata/limits/fail_loop.carbon +++ b/explorer/testdata/limits/fail_loop.carbon @@ -3,11 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE -// CHECK:STDERR: RUNTIME ERROR: overflow:1: possible infinite loop: too many interpreter steps executed package ExplorerTest impl; fn Main() -> i32 { + // CHECK:STDERR: RUNTIME ERROR: fail_loop.carbon:[[@LINE+1]]: possible infinite loop: too many interpreter steps executed while (true) { } return 0; } diff --git a/explorer/testdata/limits/fail_type_check_loop.carbon b/explorer/testdata/limits/fail_type_check_loop.carbon index 29098a5a9782..88bfb83a5eb0 100644 --- a/explorer/testdata/limits/fail_type_check_loop.carbon +++ b/explorer/testdata/limits/fail_type_check_loop.carbon @@ -3,11 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE -// CHECK:STDERR: COMPILATION ERROR: overflow:1: possible infinite loop: too many interpreter steps executed package ExplorerTest impl; fn Loop() -> type { + // CHECK:STDERR: COMPILATION ERROR: fail_type_check_loop.carbon:[[@LINE+1]]: possible infinite loop: too many interpreter steps executed while (true) {} return i32; }