Refactor checking flow to allow for ordering based on import/package. (#3379)

As I was working on this, I noticed `import` and `library` syntax needs
to be fixed for how it imports the current package, and for `Main`
libraries. This mostly reflects the current state in its testing.

Otherwise, this should handle most of the errors I could think of:
dependency cycles, redundant imports, etc.

It does not actually deal with the nuances of cross-IR references.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

toolchain/parse/BUILD

@@ -54,6 +54,7 @@ cc_library(
         "//common:ostream",
         "//common:vlog",
         "//toolchain/base:pretty_stack_trace_function",
+        "//toolchain/base:value_store",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/lex:token_kind",
         "//toolchain/lex:tokenized_buffer",

toolchain/parse/context.h

@@ -300,6 +300,19 @@ class Context {
   auto RecoverFromDeclError(StateStackEntry state, NodeKind parse_node_kind,
                             bool skip_past_likely_end) -> void;
 
+  // Sets the package directive information. Called at most once.
+  auto set_packaging_directive(Tree::PackagingNames packaging_names,
+                               Tree::ApiOrImpl api_or_impl) -> void {
+    CARBON_CHECK(!tree_->packaging_directive_);
+    tree_->packaging_directive_ = {.names = packaging_names,
+                                   .api_or_impl = api_or_impl};
+  }
+
+  // Adds an import.
+  auto AddImport(Tree::PackagingNames package) -> void {
+    tree_->imports_.push_back(package);
+  }
+
   // Prints information for a stack dump.
   auto PrintForStackDump(llvm::raw_ostream& output) const -> void;
 

toolchain/parse/handle_import_and_package.cpp

@@ -2,6 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+#include "toolchain/base/value_store.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/context.h"
 
@@ -20,10 +21,13 @@ static auto ExitOnParseError(Context& context, Context::StateStackEntry state,
 // introducer is already added.
 static auto HandleImportAndPackage(Context& context,
                                    Context::StateStackEntry state,
-                                   NodeKind directive, bool expect_api_or_impl)
+                                   NodeKind directive, bool is_package)
     -> void {
-  if (!context.ConsumeAndAddLeafNodeIf(Lex::TokenKind::Identifier,
-                                       NodeKind::Name)) {
+  Tree::PackagingNames names{.node = Node(state.subtree_start)};
+  if (auto package_name_token = context.ConsumeIf(Lex::TokenKind::Identifier)) {
+    names.package_id = context.tokens().GetIdentifier(*package_name_token);
+    context.AddLeafNode(NodeKind::Name, *package_name_token);
+  } else {
     CARBON_DIAGNOSTIC(ExpectedIdentifierAfterKeyword, Error,
                       "Expected identifier after `{0}`.", Lex::TokenKind);
     context.emitter().Emit(*context.position(), ExpectedIdentifierAfterKeyword,
@@ -32,12 +36,14 @@ static auto HandleImportAndPackage(Context& context,
     return;
   }
 
-  bool library_parsed = false;
   if (auto library_token = context.ConsumeIf(Lex::TokenKind::Library)) {
     auto library_start = context.tree().size();
 
-    if (!context.ConsumeAndAddLeafNodeIf(Lex::TokenKind::StringLiteral,
-                                         NodeKind::Literal)) {
+    if (auto library_name_token =
+            context.ConsumeIf(Lex::TokenKind::StringLiteral)) {
+      names.library_id = context.tokens().GetStringLiteral(*library_name_token);
+      context.AddLeafNode(NodeKind::Literal, *library_name_token);
+    } else {
       CARBON_DIAGNOSTIC(
           ExpectedLibraryName, Error,
           "Expected a string literal to specify the library name.");
@@ -48,11 +54,11 @@ static auto HandleImportAndPackage(Context& context,
 
     context.AddNode(NodeKind::Library, *library_token, library_start,
                     /*has_error=*/false);
-    library_parsed = true;
   }
 
-  auto next_kind = context.tokens().GetKind(*(context.position()));
-  if (!library_parsed && next_kind == Lex::TokenKind::StringLiteral) {
+  auto next_kind = context.PositionKind();
+  if (!names.library_id.is_valid() &&
+      next_kind == Lex::TokenKind::StringLiteral) {
     // If we come acroess a string literal and we didn't parse `library
     // "..."` yet, then most probably the user forgot to add `library`
     // before the library name.
@@ -63,14 +69,17 @@ static auto HandleImportAndPackage(Context& context,
     return;
   }
 
-  if (expect_api_or_impl) {
+  Tree::ApiOrImpl api_or_impl;
+  if (is_package) {
     switch (next_kind) {
       case Lex::TokenKind::Api: {
         context.AddLeafNode(NodeKind::PackageApi, context.Consume());
+        api_or_impl = Tree::ApiOrImpl::Api;
         break;
       }
       case Lex::TokenKind::Impl: {
         context.AddLeafNode(NodeKind::PackageImpl, context.Consume());
+        api_or_impl = Tree::ApiOrImpl::Impl;
         break;
       }
       default: {
@@ -89,6 +98,12 @@ static auto HandleImportAndPackage(Context& context,
     return;
   }
 
+  if (is_package) {
+    context.set_packaging_directive(names, api_or_impl);
+  } else {
+    context.AddImport(names);
+  }
+
   context.AddNode(directive, context.Consume(), state.subtree_start,
                   state.has_error);
 }
@@ -107,7 +122,7 @@ auto HandleImport(Context& context) -> void {
 
     case Context::PackagingState::InImports:
       HandleImportAndPackage(context, state, NodeKind::ImportDirective,
-                             /*expect_api_or_impl=*/false);
+                             /*is_package=*/false);
       break;
 
     case Context::PackagingState::AfterNonPackagingDecl: {
@@ -157,7 +172,7 @@ auto HandlePackage(Context& context) -> void {
   // `package` is no longer allowed, but `import` may repeat.
   context.set_packaging_state(Context::PackagingState::InImports);
   HandleImportAndPackage(context, state, NodeKind::PackageDirective,
-                         /*expect_api_or_impl=*/true);
+                         /*is_package=*/true);
 }
 
 }  // namespace Carbon::Parse

toolchain/parse/tree.h

@@ -10,7 +10,6 @@
 #include "common/error.h"
 #include "common/ostream.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
@@ -62,6 +61,26 @@ class Tree : public Printable<Tree> {
   class PostorderIterator;
   class SiblingIterator;
 
+  // For PackagingDirective.
+  enum class ApiOrImpl : uint8_t {
+    Api,
+    Impl,
+  };
+
+  // Names in packaging, whether the file's packaging or an import. Links back
+  // to the node for diagnostics.
+  struct PackagingNames {
+    Node node;
+    IdentifierId package_id = IdentifierId::Invalid;
+    StringLiteralId library_id = StringLiteralId::Invalid;
+  };
+
+  // The file's packaging.
+  struct PackagingDirective {
+    PackagingNames names;
+    ApiOrImpl api_or_impl;
+  };
+
   // Parses the token buffer into a `Tree`.
   //
   // This is the factory function which is used to build parse trees.
@@ -107,6 +126,11 @@ class Tree : public Printable<Tree> {
 
   [[nodiscard]] auto node_subtree_size(Node n) const -> int32_t;
 
+  auto packaging_directive() const -> const std::optional<PackagingDirective>& {
+    return packaging_directive_;
+  }
+  auto imports() const -> llvm::ArrayRef<PackagingNames> { return imports_; }
+
   // See the other Print comments.
   auto Print(llvm::raw_ostream& output) const -> void;
 
@@ -241,6 +265,9 @@ class Tree : public Printable<Tree> {
   // is true we do *not* have the expected 1:1 mapping between tokens and parsed
   // nodes as some tokens may have been skipped.
   bool has_errors_ = false;
+
+  std::optional<PackagingDirective> packaging_directive_;
+  llvm::SmallVector<PackagingNames> imports_;
 };
 
 // A random-access iterator to the depth-first postorder sequence of parse nodes