Support declaration modifier keywords (#3412)

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

toolchain/check/BUILD

@@ -36,6 +36,7 @@ cc_library(
         "convert.cpp",
         "decl_name_stack.cpp",
         "inst_block_stack.cpp",
+        "modifiers.cpp",
         "return.cpp",
         "return.h",
     ] +
@@ -48,7 +49,9 @@ cc_library(
         "context.h",
         "convert.h",
         "decl_name_stack.h",
+        "decl_state.h",
         "inst_block_stack.h",
+        "modifiers.h",
         "pending_block.h",
     ],
     deps = [

toolchain/check/context.h

@@ -10,6 +10,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/check/decl_name_stack.h"
+#include "toolchain/check/decl_state.h"
 #include "toolchain/check/inst_block_stack.h"
 #include "toolchain/check/node_stack.h"
 #include "toolchain/parse/tree.h"
@@ -263,6 +264,11 @@ class Context {
   // Prints information for a stack dump.
   auto PrintForStackDump(llvm::raw_ostream& output) const -> void;
 
+  // Get the Lex::TokenKind of a node for diagnostics.
+  auto token_kind(Parse::NodeId parse_node) -> Lex::TokenKind {
+    return tokens().GetKind(parse_tree().node_token(parse_node));
+  }
+
   auto tokens() -> const Lex::TokenizedBuffer& { return *tokens_; }
 
   auto emitter() -> DiagnosticEmitter& { return *emitter_; }
@@ -293,6 +299,8 @@ class Context {
 
   auto decl_name_stack() -> DeclNameStack& { return decl_name_stack_; }
 
+  auto decl_state_stack() -> DeclStateStack& { return decl_state_stack_; }
+
   // Directly expose SemIR::File data accessors for brevity in calls.
   auto identifiers() -> StringStoreWrapper<IdentifierId>& {
     return sem_ir().identifiers();
@@ -451,6 +459,9 @@ class Context {
   // The stack used for qualified declaration name construction.
   DeclNameStack decl_name_stack_;
 
+  // The stack of declarations that could have modifiers.
+  DeclStateStack decl_state_stack_;
+
   // Maps identifiers to name lookup results. Values are a stack of name lookup
   // results in the ancestor scopes. This offers constant-time lookup of names,
   // regardless of how many scopes exist between the name declaration and

toolchain/check/decl_state.h

@@ -0,0 +1,107 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TOOLCHAIN_CHECK_DECL_STATE_H_
+#define CARBON_TOOLCHAIN_CHECK_DECL_STATE_H_
+
+#include "llvm/ADT/BitmaskEnum.h"
+
+namespace Carbon::Check {
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+// Represents a set of keyword modifiers, using a separate bit per modifier.
+enum class KeywordModifierSet {
+  // At most one of these access modifiers allowed for a given declaration,
+  // and if present it must be first:
+  Private = 1 << 0,
+  Protected = 1 << 1,
+
+  // At most one of these declaration modifiers allowed for a given
+  // declaration:
+  Abstract = 1 << 2,
+  Base = 1 << 3,
+  Default = 1 << 4,
+  Final = 1 << 5,
+  Impl = 1 << 6,
+  Virtual = 1 << 7,
+
+  // Sets of modifiers:
+  Access = Private | Protected,
+  Class = Abstract | Base,
+  Method = Abstract | Impl | Virtual,
+  Interface = Default | Final,
+  None = 0,
+
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ Virtual)
+};
+
+inline auto operator!(KeywordModifierSet k) -> bool {
+  return !static_cast<unsigned>(k);
+}
+
+// State stored for each declaration we are currently in: the kind of
+// declaration and the keyword modifiers that apply to that declaration.
+struct DeclState {
+  // What kind of declaration
+  enum DeclKind { FileScope, Class, Constraint, Fn, Interface, Let, Var };
+
+  explicit DeclState(DeclKind decl_kind, Parse::NodeId parse_node)
+      : kind(decl_kind), first_node(parse_node) {}
+
+  DeclKind kind;
+
+  // Nodes of modifiers on this declaration. `Invalid` if no modifier of that
+  // kind is present.
+  Parse::NodeId saw_access_modifier = Parse::NodeId::Invalid;
+  Parse::NodeId saw_decl_modifier = Parse::NodeId::Invalid;
+
+  // Invariant: contains just the modifiers represented by `saw_access_modifier`
+  // and `saw_other_modifier`.
+  KeywordModifierSet modifier_set = KeywordModifierSet::None;
+
+  // Node corresponding to the first token of the declaration.
+  Parse::NodeId first_node;
+};
+
+// Stack of `DeclState` values, representing all the declarations we are
+// currently nested within.
+// Invariant: Bottom of the stack always has a "DeclState::FileScope" entry.
+class DeclStateStack {
+ public:
+  DeclStateStack() {
+    stack_.emplace_back(DeclState::FileScope, Parse::NodeId::Invalid);
+  }
+
+  // Enters a declaration of kind `k`, with `parse_node` for the introducer
+  // token.
+  auto Push(DeclState::DeclKind k, Parse::NodeId parse_node) -> void {
+    stack_.push_back(DeclState(k, parse_node));
+  }
+
+  // Gets the state of declaration at the top of the stack -- the innermost
+  // declaration currently being processed.
+  auto innermost() -> DeclState& { return stack_.back(); }
+
+  // Gets the state for the declaration containing the innermost declaration.
+  // Requires that the innermost declaration is not `FileScope`.
+  auto containing() const -> const DeclState& {
+    CARBON_CHECK(stack_.size() >= 2);
+    return stack_[stack_.size() - 2];
+  }
+
+  // Exits a declaration of kind `k`.
+  auto Pop(DeclState::DeclKind k) -> void {
+    CARBON_CHECK(stack_.back().kind == k);
+    stack_.pop_back();
+    CARBON_CHECK(!stack_.empty());
+  }
+
+ private:
+  llvm::SmallVector<DeclState> stack_;
+};
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_DECL_STATE_H_

toolchain/check/handle_binding_pattern.cpp

@@ -59,6 +59,8 @@ auto HandleBindingPattern(Context& context, Parse::NodeId parse_node) -> bool {
 
   // Allocate an instruction of the appropriate kind, linked to the name for
   // error locations.
+  // TODO: The node stack is a fragile way of getting context information.
+  // Get this information from somewhere else.
   switch (auto context_parse_node_kind = context.parse_tree().node_kind(
               context.node_stack().PeekParseNode())) {
     case Parse::NodeKind::ReturnedModifier:

toolchain/check/handle_class.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/modifiers.h"
 
 namespace Carbon::Check {
 
@@ -13,44 +14,40 @@ auto HandleClassIntroducer(Context& context, Parse::NodeId parse_node) -> bool {
   context.inst_block_stack().Push();
   // Push the bracketing node.
   context.node_stack().Push(parse_node);
-  // A name should always follow.
+  // Optional modifiers and the name follow.
+  context.decl_state_stack().Push(DeclState::Class, parse_node);
   context.decl_name_stack().PushScopeAndStartName();
   return true;
 }
 
-auto HandleAbstractModifier(Context& context, Parse::NodeId parse_node)
-    -> bool {
-  context.node_stack().Push(parse_node);
-  return true;
-}
-
-auto HandleBaseModifier(Context& context, Parse::NodeId parse_node) -> bool {
-  context.node_stack().Push(parse_node);
-  return true;
-}
-
 static auto BuildClassDecl(Context& context)
     -> std::tuple<SemIR::ClassId, SemIR::InstId> {
   auto name_context = context.decl_name_stack().FinishName();
-  auto introducer = context.node_stack().PeekParseNode();
-  bool abstract =
-      context.node_stack()
-          .PopAndDiscardSoloParseNodeIf<Parse::NodeKind::AbstractModifier>();
-  bool base =
-      context.node_stack()
-          .PopAndDiscardSoloParseNodeIf<Parse::NodeKind::BaseModifier>();
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::ClassIntroducer>();
-  auto decl_block_id = context.inst_block_stack().Pop();
+  auto first_node = context.decl_state_stack().innermost().first_node;
+
+  // Process modifiers.
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Class);
+  LimitModifiersOnDecl(context,
+                       KeywordModifierSet::Class | KeywordModifierSet::Access,
+                       Lex::TokenKind::Class);
+
+  auto modifiers = context.decl_state_stack().innermost().modifier_set;
+  if (!!(modifiers & KeywordModifierSet::Access)) {
+    context.TODO(context.decl_state_stack().innermost().saw_access_modifier,
+                 "access modifier");
+  }
+  auto inheritance_kind =
+      !!(modifiers & KeywordModifierSet::Abstract) ? SemIR::Class::Abstract
+      : !!(modifiers & KeywordModifierSet::Base)   ? SemIR::Class::Base
+                                                   : SemIR::Class::Final;
 
-  CARBON_CHECK(!(abstract && base)) << "Cannot be both `abstract` and `base`";
-  auto inheritance_kind = abstract ? SemIR::Class::Abstract
-                          : base   ? SemIR::Class::Base
-                                   : SemIR::Class::Final;
+  auto decl_block_id = context.inst_block_stack().Pop();
 
   // Add the class declaration.
   auto class_decl =
-      SemIR::ClassDecl{introducer, SemIR::ClassId::Invalid, decl_block_id};
+      SemIR::ClassDecl{first_node, SemIR::ClassId::Invalid, decl_block_id};
   auto class_decl_id = context.AddInst(class_decl);
 
   // Check whether this is a redeclaration.
@@ -71,7 +68,7 @@ static auto BuildClassDecl(Context& context)
         CARBON_DIAGNOSTIC(ClassRedeclarationDifferentIntroducerPrevious, Note,
                           "Previously declared here.");
         context.emitter()
-            .Build(introducer, ClassRedeclarationDifferentIntroducer)
+            .Build(first_node, ClassRedeclarationDifferentIntroducer)
             .Note(existing_class_decl->parse_node,
                   ClassRedeclarationDifferentIntroducerPrevious)
             .Emit();
@@ -104,7 +101,7 @@ static auto BuildClassDecl(Context& context)
     auto& class_info = context.classes().Get(class_decl.class_id);
     class_info.self_type_id =
         context.CanonicalizeType(context.AddInst(SemIR::ClassType{
-            introducer, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
+            first_node, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
             class_decl.class_id}));
   }
 
@@ -117,6 +114,7 @@ static auto BuildClassDecl(Context& context)
 auto HandleClassDecl(Context& context, Parse::NodeId /*parse_node*/) -> bool {
   BuildClassDecl(context);
   context.decl_name_stack().PopScope();
+  context.decl_state_stack().Pop(DeclState::Class);
   return true;
 }
 
@@ -266,6 +264,7 @@ auto HandleClassDefinition(Context& context, Parse::NodeId parse_node) -> bool {
   context.inst_block_stack().Pop();
   context.PopScope();
   context.decl_name_stack().PopScope();
+  context.decl_state_stack().Pop(DeclState::Class);
 
   // The class type is now fully defined.
   auto& class_info = context.classes().Get(class_id);

toolchain/check/handle_function.cpp

@@ -4,10 +4,42 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/modifiers.h"
 #include "toolchain/sem_ir/entry_point.h"
 
 namespace Carbon::Check {
 
+static auto DiagnoseModifiers(Context& context) -> KeywordModifierSet {
+  Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
+  CheckAccessModifiersOnDecl(context, decl_kind);
+  LimitModifiersOnDecl(context,
+                       KeywordModifierSet::Access | KeywordModifierSet::Method |
+                           KeywordModifierSet::Interface,
+                       decl_kind);
+  // Rules for abstract, virtual, and impl, which are only allowed in classes.
+  auto containing_kind = context.decl_state_stack().containing().kind;
+  if (containing_kind != DeclState::Class) {
+    ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind,
+                          " outside of a class");
+  } else {
+    auto containing_decl_modifiers =
+        context.decl_state_stack().containing().modifier_set;
+    if (!(containing_decl_modifiers & KeywordModifierSet::Class)) {
+      ForbidModifiersOnDecl(context, KeywordModifierSet::Virtual, decl_kind,
+                            " in a non-abstract non-base `class` definition",
+                            context.decl_state_stack().containing().first_node);
+    }
+    if (!(containing_decl_modifiers & KeywordModifierSet::Abstract)) {
+      ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind,
+                            " in a non-abstract `class` definition",
+                            context.decl_state_stack().containing().first_node);
+    }
+  }
+  RequireDefaultFinalOnlyInInterfaces(context, decl_kind);
+
+  return context.decl_state_stack().innermost().modifier_set;
+}
+
 // Build a FunctionDecl describing the signature of a function. This
 // handles the common logic shared by function declaration syntax and function
 // definition syntax.
@@ -55,13 +87,29 @@ static auto BuildFunctionDecl(Context& context, bool is_definition)
       context.node_stack().PopIf<Parse::NodeKind::ImplicitParamList>().value_or(
           SemIR::InstBlockId::Empty);
   auto name_context = context.decl_name_stack().FinishName();
-  auto fn_node =
-      context.node_stack()
-          .PopForSoloParseNode<Parse::NodeKind::FunctionIntroducer>();
+  context.node_stack()
+      .PopAndDiscardSoloParseNode<Parse::NodeKind::FunctionIntroducer>();
+
+  auto first_node = context.decl_state_stack().innermost().first_node;
+
+  // Process modifiers.
+  auto modifiers = DiagnoseModifiers(context);
+  if (!!(modifiers & KeywordModifierSet::Access)) {
+    context.TODO(context.decl_state_stack().innermost().saw_access_modifier,
+                 "access modifier");
+  }
+  if (!!(modifiers & KeywordModifierSet::Method)) {
+    context.TODO(context.decl_state_stack().innermost().saw_decl_modifier,
+                 "method modifier");
+  }
+  if (!!(modifiers & KeywordModifierSet::Interface)) {
+    context.TODO(context.decl_state_stack().innermost().saw_decl_modifier,
+                 "interface modifier");
+  }
 
   // Add the function declaration.
   auto function_decl = SemIR::FunctionDecl{
-      fn_node, context.GetBuiltinType(SemIR::BuiltinKind::FunctionType),
+      first_node, context.GetBuiltinType(SemIR::BuiltinKind::FunctionType),
       SemIR::FunctionId::Invalid};
   auto function_decl_id = context.AddInst(function_decl);
 
@@ -116,11 +164,11 @@ static auto BuildFunctionDecl(Context& context, bool is_definition)
         (return_slot_id.is_valid() &&
          return_type_id !=
              context.GetBuiltinType(SemIR::BuiltinKind::BoolType) &&
-         return_type_id != context.CanonicalizeTupleType(fn_node, {}))) {
+         return_type_id != context.CanonicalizeTupleType(first_node, {}))) {
       CARBON_DIAGNOSTIC(InvalidMainRunSignature, Error,
                         "Invalid signature for `Main.Run` function. Expected "
                         "`fn ()` or `fn () -> i32`.");
-      context.emitter().Emit(fn_node, InvalidMainRunSignature);
+      context.emitter().Emit(first_node, InvalidMainRunSignature);
     }
   }
 
@@ -131,6 +179,7 @@ auto HandleFunctionDecl(Context& context, Parse::NodeId /*parse_node*/)
     -> bool {
   BuildFunctionDecl(context, /*is_definition=*/false);
   context.decl_name_stack().PopScope();
+  context.decl_state_stack().Pop(DeclState::Fn);
   return true;
 }
 
@@ -156,6 +205,7 @@ auto HandleFunctionDefinition(Context& context, Parse::NodeId parse_node)
   context.inst_block_stack().Pop();
   context.return_scope_stack().pop_back();
   context.decl_name_stack().PopScope();
+  context.decl_state_stack().Pop(DeclState::Fn);
   return true;
 }
 
@@ -228,7 +278,8 @@ auto HandleFunctionIntroducer(Context& context, Parse::NodeId parse_node)
   context.inst_block_stack().Push();
   // Push the bracketing node.
   context.node_stack().Push(parse_node);
-  // A name should always follow.
+  // Optional modifiers and the name follow.
+  context.decl_state_stack().Push(DeclState::Fn, parse_node);
   context.decl_name_stack().PushScopeAndStartName();
   return true;
 }

toolchain/check/handle_let.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/modifiers.h"
 #include "toolchain/sem_ir/inst.h"
 
 namespace Carbon::Check {
@@ -14,6 +15,23 @@ auto HandleLetDecl(Context& context, Parse::NodeId parse_node) -> bool {
       context.node_stack().Pop<Parse::NodeKind::BindingPattern>();
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::LetIntroducer>();
+  // Process declaration modifiers.
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Let);
+  RequireDefaultFinalOnlyInInterfaces(context, Lex::TokenKind::Let);
+  LimitModifiersOnDecl(
+      context, KeywordModifierSet::Access | KeywordModifierSet::Interface,
+      Lex::TokenKind::Let);
+
+  auto modifiers = context.decl_state_stack().innermost().modifier_set;
+  if (!!(modifiers & KeywordModifierSet::Access)) {
+    context.TODO(context.decl_state_stack().innermost().saw_access_modifier,
+                 "access modifier");
+  }
+  if (!!(modifiers & KeywordModifierSet::Interface)) {
+    context.TODO(context.decl_state_stack().innermost().saw_decl_modifier,
+                 "interface modifier");
+  }
+  context.decl_state_stack().Pop(DeclState::Let);
 
   // Convert the value to match the type of the pattern.
   auto pattern = context.insts().Get(pattern_id);
@@ -36,6 +54,7 @@ auto HandleLetDecl(Context& context, Parse::NodeId parse_node) -> bool {
 }
 
 auto HandleLetIntroducer(Context& context, Parse::NodeId parse_node) -> bool {
+  context.decl_state_stack().Push(DeclState::Let, parse_node);
   // Push a bracketing node to establish the pattern context.
   context.node_stack().Push(parse_node);
   return true;

toolchain/check/handle_loop_statement.cpp

@@ -58,6 +58,7 @@ auto HandleForHeaderStart(Context& context, Parse::NodeId parse_node) -> bool {
 }
 
 auto HandleForIn(Context& context, Parse::NodeId parse_node) -> bool {
+  context.decl_state_stack().Pop(DeclState::Var);
   return context.TODO(parse_node, "HandleForIn");
 }
 

toolchain/check/handle_modifier.cpp

@@ -0,0 +1,116 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/check/context.h"
+#include "toolchain/lex/token_kind.h"
+
+namespace Carbon::Check {
+
+CARBON_DIAGNOSTIC(ModifierPrevious, Note, "`{0}` previously appeared here.",
+                  Lex::TokenKind);
+
+static auto EmitRepeatedDiagnostic(Context& context, Parse::NodeId first_node,
+                                   Parse::NodeId second_node) -> void {
+  CARBON_DIAGNOSTIC(ModifierRepeated, Error, "`{0}` repeated on declaration.",
+                    Lex::TokenKind);
+  context.emitter()
+      .Build(second_node, ModifierRepeated, context.token_kind(second_node))
+      .Note(first_node, ModifierPrevious, context.token_kind(first_node))
+      .Emit();
+}
+
+static auto EmitNotAllowedWithDiagnostic(Context& context,
+                                         Parse::NodeId first_node,
+                                         Parse::NodeId second_node) -> void {
+  CARBON_DIAGNOSTIC(ModifierNotAllowedWith, Error,
+                    "`{0}` not allowed on declaration with `{1}`.",
+                    Lex::TokenKind, Lex::TokenKind);
+  context.emitter()
+      .Build(second_node, ModifierNotAllowedWith,
+             context.token_kind(second_node), context.token_kind(first_node))
+      .Note(first_node, ModifierPrevious, context.token_kind(first_node))
+      .Emit();
+}
+
+static auto GetAccessModifierEnum(Lex::TokenKind token_kind)
+    -> KeywordModifierSet {
+  switch (token_kind) {
+    case Lex::TokenKind::Private:
+      return KeywordModifierSet::Private;
+    case Lex::TokenKind::Protected:
+      return KeywordModifierSet::Protected;
+    default:
+      CARBON_FATAL() << "Unhandled access modifier keyword";
+  }
+}
+
+auto HandleAccessModifierKeyword(Context& context, Parse::NodeId parse_node)
+    -> bool {
+  auto keyword = GetAccessModifierEnum(
+      context.tokens().GetKind(context.parse_tree().node_token(parse_node)));
+  auto& s = context.decl_state_stack().innermost();
+  if (!!(s.modifier_set & keyword)) {
+    EmitRepeatedDiagnostic(context, s.saw_access_modifier, parse_node);
+  } else if (s.saw_access_modifier.is_valid()) {
+    EmitNotAllowedWithDiagnostic(context, s.saw_access_modifier, parse_node);
+  } else if (s.saw_decl_modifier.is_valid()) {
+    CARBON_DIAGNOSTIC(ModifierMustAppearBefore, Error,
+                      "`{0}` must appear before `{1}`.", Lex::TokenKind,
+                      Lex::TokenKind);
+    context.emitter()
+        .Build(parse_node, ModifierMustAppearBefore,
+               context.token_kind(parse_node),
+               context.token_kind(s.saw_decl_modifier))
+        .Note(s.saw_decl_modifier, ModifierPrevious,
+              context.token_kind(s.saw_decl_modifier))
+        .Emit();
+  } else {
+    s.modifier_set |= keyword;
+    s.saw_access_modifier = parse_node;
+    s.first_node = parse_node;
+  }
+
+  return true;
+}
+
+static auto GetDeclModifierEnum(Lex::TokenKind token_kind)
+    -> KeywordModifierSet {
+  switch (token_kind) {
+    case Lex::TokenKind::Abstract:
+      return KeywordModifierSet::Abstract;
+    case Lex::TokenKind::Base:
+      return KeywordModifierSet::Base;
+    case Lex::TokenKind::Default:
+      return KeywordModifierSet::Default;
+    case Lex::TokenKind::Final:
+      return KeywordModifierSet::Final;
+    case Lex::TokenKind::Impl:
+      return KeywordModifierSet::Impl;
+    case Lex::TokenKind::Virtual:
+      return KeywordModifierSet::Virtual;
+    default:
+      CARBON_FATAL() << "Unhandled declaration modifier keyword";
+  }
+}
+
+auto HandleDeclModifierKeyword(Context& context, Parse::NodeId parse_node)
+    -> bool {
+  auto keyword = GetDeclModifierEnum(
+      context.tokens().GetKind(context.parse_tree().node_token(parse_node)));
+  auto& s = context.decl_state_stack().innermost();
+  if (!!(s.modifier_set & keyword)) {
+    EmitRepeatedDiagnostic(context, s.saw_decl_modifier, parse_node);
+  } else if (s.saw_decl_modifier.is_valid()) {
+    EmitNotAllowedWithDiagnostic(context, s.saw_decl_modifier, parse_node);
+  } else {
+    s.modifier_set |= keyword;
+    s.saw_decl_modifier = parse_node;
+    if (s.saw_access_modifier == Parse::NodeId::Invalid) {
+      s.first_node = parse_node;
+    }
+  }
+  return true;
+}
+
+}  // namespace Carbon::Check

toolchain/check/handle_noop.cpp

@@ -16,4 +16,20 @@ auto HandleInvalidParse(Context& context, Parse::NodeId parse_node) -> bool {
   return context.TODO(parse_node, "HandleInvalidParse");
 }
 
+auto HandleInvalidParseStart(Context& context, Parse::NodeId parse_node)
+    -> bool {
+  return context.TODO(parse_node, "HandleInvalidParseStart");
+}
+
+auto HandleInvalidParseSubtree(Context& context, Parse::NodeId parse_node)
+    -> bool {
+  return context.TODO(parse_node, "HandleInvalidParseSubtree");
+}
+
+auto HandlePlaceholder(Context& /*context*/, Parse::NodeId /*parse_node*/)
+    -> bool {
+  CARBON_FATAL()
+      << "Placeholder node should always be replaced before parse completes";
+}
+
 }  // namespace Carbon::Check

toolchain/check/handle_variable.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/modifiers.h"
 #include "toolchain/sem_ir/inst.h"
 
 namespace Carbon::Check {
@@ -12,6 +13,7 @@ auto HandleVariableIntroducer(Context& context, Parse::NodeId parse_node)
     -> bool {
   // No action, just a bracketing node.
   context.node_stack().Push(parse_node);
+  context.decl_state_stack().Push(DeclState::Var, parse_node);
   return true;
 }
 
@@ -74,6 +76,18 @@ auto HandleVariableDecl(Context& context, Parse::NodeId parse_node) -> bool {
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::VariableIntroducer>();
 
+  // Process declaration modifiers.
+  CheckAccessModifiersOnDecl(context, Lex::TokenKind::Var);
+  LimitModifiersOnDecl(context, KeywordModifierSet::Access,
+                       Lex::TokenKind::Var);
+  auto modifiers = context.decl_state_stack().innermost().modifier_set;
+  if (!!(modifiers & KeywordModifierSet::Access)) {
+    context.TODO(context.decl_state_stack().innermost().saw_access_modifier,
+                 "access modifier");
+  }
+
+  context.decl_state_stack().Pop(DeclState::Var);
+
   return true;
 }
 

toolchain/check/modifiers.cpp

@@ -0,0 +1,97 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/check/modifiers.h"
+
+namespace Carbon::Check {
+
+static auto ReportNotAllowed(Context& context, Parse::NodeId modifier_node,
+                             Lex::TokenKind decl_kind,
+                             llvm::StringRef context_string,
+                             Parse::NodeId context_node) -> void {
+  CARBON_DIAGNOSTIC(ModifierNotAllowedOn, Error,
+                    "`{0}` not allowed on `{1}` declaration{2}.",
+                    Lex::TokenKind, Lex::TokenKind, std::string);
+  auto diag = context.emitter().Build(modifier_node, ModifierNotAllowedOn,
+                                      context.token_kind(modifier_node),
+                                      decl_kind, context_string.str());
+  if (context_node.is_valid()) {
+    CARBON_DIAGNOSTIC(ModifierNotInContext, Note,
+                      "Containing definition here.");
+    diag.Note(context_node, ModifierNotInContext);
+  }
+  diag.Emit();
+}
+
+auto LimitModifiersOnDecl(Context& context, KeywordModifierSet allowed,
+                          Lex::TokenKind decl_kind) -> void {
+  auto& s = context.decl_state_stack().innermost();
+  auto not_allowed = s.modifier_set & ~allowed;
+  if (!!(not_allowed & KeywordModifierSet::Access)) {
+    ReportNotAllowed(context, s.saw_access_modifier, decl_kind, "",
+                     Parse::NodeId::Invalid);
+    not_allowed = not_allowed & ~KeywordModifierSet::Access;
+    s.saw_access_modifier = Parse::NodeId::Invalid;
+  }
+  if (!!not_allowed) {
+    ReportNotAllowed(context, s.saw_decl_modifier, decl_kind, "",
+                     Parse::NodeId::Invalid);
+    s.saw_decl_modifier = Parse::NodeId::Invalid;
+  }
+  s.modifier_set &= allowed;
+}
+
+auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden,
+                           Lex::TokenKind decl_kind,
+                           llvm::StringRef context_string,
+                           Parse::NodeId context_node) -> void {
+  auto& s = context.decl_state_stack().innermost();
+  auto not_allowed = s.modifier_set & forbidden;
+  if (!!(not_allowed & KeywordModifierSet::Access)) {
+    ReportNotAllowed(context, s.saw_access_modifier, decl_kind, context_string,
+                     context_node);
+    not_allowed = not_allowed & ~KeywordModifierSet::Access;
+    s.saw_access_modifier = Parse::NodeId::Invalid;
+  }
+  if (!!not_allowed) {
+    ReportNotAllowed(context, s.saw_decl_modifier, decl_kind, context_string,
+                     context_node);
+    s.saw_decl_modifier = Parse::NodeId::Invalid;
+  }
+  s.modifier_set = s.modifier_set & ~forbidden;
+}
+
+auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind)
+    -> void {
+  switch (context.decl_state_stack().containing().kind) {
+    case DeclState::FileScope:
+      ForbidModifiersOnDecl(
+          context, KeywordModifierSet::Protected, decl_kind,
+          " at file scope, `protected` is only allowed on class members");
+      break;
+
+    case DeclState::Class:
+      // Both `private` and `protected` allowed in a class definition.
+      break;
+
+    default:
+      // Otherwise neither `private` nor `protected` allowed.
+      ForbidModifiersOnDecl(context, KeywordModifierSet::Protected, decl_kind,
+                            ", `protected` is only allowed on class members");
+      ForbidModifiersOnDecl(
+          context, KeywordModifierSet::Private, decl_kind,
+          ", `private` is only allowed on class members and at file scope");
+      break;
+  }
+}
+
+auto RequireDefaultFinalOnlyInInterfaces(Context& context,
+                                         Lex::TokenKind decl_kind) -> void {
+  if (context.decl_state_stack().containing().kind != DeclState::Interface) {
+    ForbidModifiersOnDecl(context, KeywordModifierSet::Interface, decl_kind,
+                          " outside of an interface");
+  }
+}
+
+}  // namespace Carbon::Check

toolchain/check/modifiers.h

@@ -0,0 +1,40 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TOOLCHAIN_CHECK_MODIFIERS_H_
+#define CARBON_TOOLCHAIN_CHECK_MODIFIERS_H_
+
+#include "toolchain/check/context.h"
+
+namespace Carbon::Check {
+
+// Reports a diagnostic (using `decl_name`) if access control modifiers on this
+// are not allowed, and updates the declaration state in `context`.
+auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind)
+    -> void;
+
+// Reports a diagnostic (using `decl_kind`) if modifiers on this declaration are
+// not in `allowed`. Updates the declaration state in
+// `context.decl_state_stack()`.
+auto LimitModifiersOnDecl(Context& context, KeywordModifierSet allowed,
+                          Lex::TokenKind decl_kind) -> void;
+
+// Like `LimitModifiersOnDecl`, except says which modifiers are forbidden, and a
+// `context_string` (and optional `context_node`) specifying the context in
+// which those modifiers are forbidden.
+auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden,
+                           Lex::TokenKind decl_kind,
+                           llvm::StringRef context_string,
+                           Parse::NodeId context_node = Parse::NodeId::Invalid)
+    -> void;
+
+// Report a diagonostic if `default` and `final` modifiers are used on
+// declarations where they are not allowed. Right now they are only allowed
+// inside interfaces.
+auto RequireDefaultFinalOnlyInInterfaces(Context& context,
+                                         Lex::TokenKind decl_kind) -> void;
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_MODIFIERS_H_

toolchain/check/node_stack.h

@@ -350,11 +350,11 @@ class NodeStack {
       case Parse::NodeKind::BaseName:
       case Parse::NodeKind::Name:
         return IdKind::NameId;
-      case Parse::NodeKind::AbstractModifier:
+      case Parse::NodeKind::AccessModifierKeyword:
       case Parse::NodeKind::ArrayExprSemi:
-      case Parse::NodeKind::BaseModifier:
       case Parse::NodeKind::ClassIntroducer:
       case Parse::NodeKind::CodeBlockStart:
+      case Parse::NodeKind::DeclModifierKeyword:
       case Parse::NodeKind::FunctionIntroducer:
       case Parse::NodeKind::IfStatementElse:
       case Parse::NodeKind::ImplicitParamListStart:

toolchain/check/testdata/class/fail_field_modifiers.carbon

@@ -0,0 +1,58 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+class Class {
+
+  // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: ERROR: `default` not allowed on `var` declaration.
+  // CHECK:STDERR:   default var j: i32;
+  // CHECK:STDERR:   ^~~~~~~
+  default var j: i32;
+
+  // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: ERROR: `final` not allowed on `var` declaration.
+  // CHECK:STDERR:   final var k: i32;
+  // CHECK:STDERR:   ^~~~~
+  final var k: i32;
+
+  // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: ERROR: `default` not allowed on `let` declaration outside of an interface.
+  // CHECK:STDERR:   default let l: i32 = 0;
+  // CHECK:STDERR:   ^~~~~~~
+  default let l: i32 = 0;
+
+  // CHECK:STDERR: fail_field_modifiers.carbon:[[@LINE+3]]:3: ERROR: `final` not allowed on `let` declaration outside of an interface.
+  // CHECK:STDERR:   final let m: i32 = 1;
+  // CHECK:STDERR:   ^~~~~
+  final let m: i32 = 1;
+}
+
+// CHECK:STDOUT: --- fail_field_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc28: type = struct_type {.j: i32, .k: i32}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.Class = %Class.decl}
+// CHECK:STDOUT:   %Class.decl = class_decl @Class, ()
+// CHECK:STDOUT:   %Class: type = class_type @Class
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Class {
+// CHECK:STDOUT:   %.loc12_16.1: type = unbound_element_type Class, i32
+// CHECK:STDOUT:   %.loc12_16.2: <unbound element of class Class> = field_decl j, element0
+// CHECK:STDOUT:   %j: <unbound element of class Class> = bind_name j, %.loc12_16.2
+// CHECK:STDOUT:   %.loc17_14.1: type = unbound_element_type Class, i32
+// CHECK:STDOUT:   %.loc17_14.2: <unbound element of class Class> = field_decl k, element1
+// CHECK:STDOUT:   %k: <unbound element of class Class> = bind_name k, %.loc17_14.2
+// CHECK:STDOUT:   %.loc22: i32 = int_literal 0
+// CHECK:STDOUT:   %l: i32 = bind_name l, %.loc22
+// CHECK:STDOUT:   %.loc27: i32 = int_literal 1
+// CHECK:STDOUT:   %m: i32 = bind_name m, %.loc27
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .j = %j
+// CHECK:STDOUT:   .k = %k
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/class/fail_method_modifiers.carbon

@@ -0,0 +1,100 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+class FinalClass {
+
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+6]]:3: ERROR: `abstract` not allowed on `fn` declaration in a non-abstract `class` definition.
+  // CHECK:STDERR:   abstract fn Abstract[self: Self]();
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-5]]:1: Containing definition here.
+  // CHECK:STDERR: class FinalClass {
+  // CHECK:STDERR: ^~~~~
+  abstract fn Abstract[self: Self]();
+
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+6]]:3: ERROR: `virtual` not allowed on `fn` declaration in a non-abstract non-base `class` definition.
+  // CHECK:STDERR:   virtual fn Virtual[self: Self]();
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-13]]:1: Containing definition here.
+  // CHECK:STDERR: class FinalClass {
+  // CHECK:STDERR: ^~~~~
+  virtual fn Virtual[self: Self]();
+}
+
+abstract class AbstractClass {
+
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+3]]:3: ERROR: `default` not allowed on `fn` declaration outside of an interface.
+  // CHECK:STDERR:   default fn Default[self: Self]();
+  // CHECK:STDERR:   ^~~~~~~
+  default fn Default[self: Self]();
+
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+3]]:3: ERROR: `final` not allowed on `fn` declaration outside of an interface.
+  // CHECK:STDERR:   final fn Final[self: Self]();
+  // CHECK:STDERR:   ^~~~~
+  final fn Final[self: Self]();
+}
+
+base class BaseClass {
+
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE+6]]:3: ERROR: `abstract` not allowed on `fn` declaration in a non-abstract `class` definition.
+  // CHECK:STDERR:   abstract fn Abstract[self: Self]();
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_method_modifiers.carbon:[[@LINE-5]]:1: Containing definition here.
+  // CHECK:STDERR: base class BaseClass {
+  // CHECK:STDERR: ^~~~
+  abstract fn Abstract[self: Self]();
+}
+
+// CHECK:STDOUT: --- fail_method_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc24: type = struct_type {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.FinalClass = %FinalClass.decl, .AbstractClass = %AbstractClass.decl, .BaseClass = %BaseClass.decl}
+// CHECK:STDOUT:   %FinalClass.decl = class_decl @FinalClass, ()
+// CHECK:STDOUT:   %FinalClass: type = class_type @FinalClass
+// CHECK:STDOUT:   %AbstractClass.decl = class_decl @AbstractClass, ()
+// CHECK:STDOUT:   %AbstractClass: type = class_type @AbstractClass
+// CHECK:STDOUT:   %BaseClass.decl = class_decl @BaseClass, ()
+// CHECK:STDOUT:   %BaseClass: type = class_type @BaseClass
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @FinalClass {
+// CHECK:STDOUT:   %Abstract: <function> = fn_decl @Abstract.1
+// CHECK:STDOUT:   %Virtual: <function> = fn_decl @Virtual
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Abstract = %Abstract
+// CHECK:STDOUT:   .Virtual = %Virtual
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @AbstractClass {
+// CHECK:STDOUT:   %Default: <function> = fn_decl @Default
+// CHECK:STDOUT:   %Final: <function> = fn_decl @Final
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Default = %Default
+// CHECK:STDOUT:   .Final = %Final
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @BaseClass {
+// CHECK:STDOUT:   %Abstract: <function> = fn_decl @Abstract.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Abstract = %Abstract
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Abstract.1[%self: FinalClass]();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Virtual[%self: FinalClass]();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Default[%self: AbstractClass]();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Final[%self: AbstractClass]();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Abstract.2[%self: BaseClass]();
+// CHECK:STDOUT:

toolchain/check/testdata/class/fail_modifiers.carbon

@@ -0,0 +1,109 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private abstract private class DuplicatePrivate;
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:18: ERROR: `private` repeated on declaration.
+// CHECK:STDERR: private abstract private class DuplicatePrivate;
+// CHECK:STDERR:                  ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `private` previously appeared here.
+// CHECK:STDERR: private abstract private class DuplicatePrivate;
+// CHECK:STDERR: ^~~~~~~
+private abstract private class DuplicatePrivate;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private base protected class TwoAccess {}
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:14: ERROR: `protected` not allowed on declaration with `private`.
+// CHECK:STDERR: private base protected class TwoAccess {}
+// CHECK:STDERR:              ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `private` previously appeared here.
+// CHECK:STDERR: private base protected class TwoAccess {}
+// CHECK:STDERR: ^~~~~~~
+private base protected class TwoAccess {}
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:10: ERROR: `abstract` repeated on declaration.
+// CHECK:STDERR: abstract abstract class TwoAbstract;
+// CHECK:STDERR:          ^~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `abstract` previously appeared here.
+// CHECK:STDERR: abstract abstract class TwoAbstract;
+// CHECK:STDERR: ^~~~~~~~
+abstract abstract class TwoAbstract;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+12]]:1: ERROR: `protected` not allowed on `class` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected virtual base class Virtual {}
+// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:11: ERROR: `virtual` not allowed on `class` declaration.
+// CHECK:STDERR: protected virtual base class Virtual {}
+// CHECK:STDERR:           ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:19: ERROR: `base` not allowed on declaration with `virtual`.
+// CHECK:STDERR: protected virtual base class Virtual {}
+// CHECK:STDERR:                   ^~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:11: `virtual` previously appeared here.
+// CHECK:STDERR: protected virtual base class Virtual {}
+// CHECK:STDERR:           ^~~~~~~
+protected virtual base class Virtual {}
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:10: ERROR: `protected` must appear before `abstract`.
+// CHECK:STDERR: abstract protected class WrongOrder;
+// CHECK:STDERR:          ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `abstract` previously appeared here.
+// CHECK:STDERR: abstract protected class WrongOrder;
+// CHECK:STDERR: ^~~~~~~~
+abstract protected class WrongOrder;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:10: ERROR: `base` not allowed on declaration with `abstract`.
+// CHECK:STDERR: abstract base class AbstractAndBase {}
+// CHECK:STDERR:          ^~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `abstract` previously appeared here.
+// CHECK:STDERR: abstract base class AbstractAndBase {}
+// CHECK:STDERR: ^~~~~~~~
+abstract base class AbstractAndBase {}
+
+// CHECK:STDOUT: --- fail_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc27: type = struct_type {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.DuplicatePrivate = %DuplicatePrivate.decl, .TwoAccess = %TwoAccess.decl, .TwoAbstract = %TwoAbstract.decl, .Virtual = %Virtual.decl, .WrongOrder = %WrongOrder.decl, .AbstractAndBase = %AbstractAndBase.decl}
+// CHECK:STDOUT:   %DuplicatePrivate.decl = class_decl @DuplicatePrivate, ()
+// CHECK:STDOUT:   %DuplicatePrivate: type = class_type @DuplicatePrivate
+// CHECK:STDOUT:   %TwoAccess.decl = class_decl @TwoAccess, ()
+// CHECK:STDOUT:   %TwoAccess: type = class_type @TwoAccess
+// CHECK:STDOUT:   %TwoAbstract.decl = class_decl @TwoAbstract, ()
+// CHECK:STDOUT:   %TwoAbstract: type = class_type @TwoAbstract
+// CHECK:STDOUT:   %Virtual.decl = class_decl @Virtual, ()
+// CHECK:STDOUT:   %Virtual: type = class_type @Virtual
+// CHECK:STDOUT:   %WrongOrder.decl = class_decl @WrongOrder, ()
+// CHECK:STDOUT:   %WrongOrder: type = class_type @WrongOrder
+// CHECK:STDOUT:   %AbstractAndBase.decl = class_decl @AbstractAndBase, ()
+// CHECK:STDOUT:   %AbstractAndBase: type = class_type @AbstractAndBase
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @DuplicatePrivate;
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @TwoAccess {
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @TwoAbstract;
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Virtual {
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @WrongOrder;
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @AbstractAndBase {
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/class/fail_todo_modifiers.carbon

@@ -0,0 +1,128 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+class Access {
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `access modifier`.
+  // CHECK:STDERR:   private fn F();
+  // CHECK:STDERR:   ^~~~~~~
+  private fn F();
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `access modifier`.
+  // CHECK:STDERR:   protected fn G();
+  // CHECK:STDERR:   ^~~~~~~~~
+  protected fn G();
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `access modifier`.
+  // CHECK:STDERR:   private var k: i32;
+  // CHECK:STDERR:   ^~~~~~~
+  private var k: i32;
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `access modifier`.
+  // CHECK:STDERR:   protected var l: i32;
+  // CHECK:STDERR:   ^~~~~~~~~
+  protected var l: i32;
+}
+
+base class Base {
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
+  // CHECK:STDERR:   virtual fn H();
+  // CHECK:STDERR:   ^~~~~~~
+  virtual fn H();
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
+  // CHECK:STDERR:   impl fn I();
+  // CHECK:STDERR:   ^~~~
+  impl fn I();
+}
+
+abstract class Abstract {
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
+  // CHECK:STDERR:   abstract fn J();
+  // CHECK:STDERR:   ^~~~~~~~
+  abstract fn J();
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
+  // CHECK:STDERR:   virtual fn K();
+  // CHECK:STDERR:   ^~~~~~~
+  virtual fn K();
+
+  // CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
+  // CHECK:STDERR:   impl fn L();
+  // CHECK:STDERR:   ^~~~
+  impl fn L();
+}
+
+// CHECK:STDOUT: --- fail_todo_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc28: type = struct_type {.k: i32, .l: i32}
+// CHECK:STDOUT:   %.loc41: type = struct_type {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.Access = %Access.decl, .Base = %Base.decl, .Abstract = %Abstract.decl}
+// CHECK:STDOUT:   %Access.decl = class_decl @Access, ()
+// CHECK:STDOUT:   %Access: type = class_type @Access
+// CHECK:STDOUT:   %Base.decl = class_decl @Base, ()
+// CHECK:STDOUT:   %Base: type = class_type @Base
+// CHECK:STDOUT:   %Abstract.decl = class_decl @Abstract, ()
+// CHECK:STDOUT:   %Abstract: type = class_type @Abstract
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Access {
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT:   %G: <function> = fn_decl @G
+// CHECK:STDOUT:   %.loc22_16.1: type = unbound_element_type Access, i32
+// CHECK:STDOUT:   %.loc22_16.2: <unbound element of class Access> = field_decl k, element0
+// CHECK:STDOUT:   %k: <unbound element of class Access> = bind_name k, %.loc22_16.2
+// CHECK:STDOUT:   %.loc27_18.1: type = unbound_element_type Access, i32
+// CHECK:STDOUT:   %.loc27_18.2: <unbound element of class Access> = field_decl l, element1
+// CHECK:STDOUT:   %l: <unbound element of class Access> = bind_name l, %.loc27_18.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .F = %F
+// CHECK:STDOUT:   .G = %G
+// CHECK:STDOUT:   .k = %k
+// CHECK:STDOUT:   .l = %l
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Base {
+// CHECK:STDOUT:   %H: <function> = fn_decl @H
+// CHECK:STDOUT:   %I: <function> = fn_decl @I
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .H = %H
+// CHECK:STDOUT:   .I = %I
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Abstract {
+// CHECK:STDOUT:   %J: <function> = fn_decl @J
+// CHECK:STDOUT:   %K: <function> = fn_decl @K
+// CHECK:STDOUT:   %L: <function> = fn_decl @L
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .J = %J
+// CHECK:STDOUT:   .K = %K
+// CHECK:STDOUT:   .L = %L
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @H();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @I();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @J();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @K();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @L();
+// CHECK:STDOUT:

toolchain/check/testdata/function/declaration/fail_modifiers.carbon

@@ -0,0 +1,105 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `default` not allowed on `fn` declaration outside of an interface.
+// CHECK:STDERR: default protected fn WrongOrder();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `protected` must appear before `default`.
+// CHECK:STDERR: default protected fn WrongOrder();
+// CHECK:STDERR:         ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `default` previously appeared here.
+// CHECK:STDERR: default protected fn WrongOrder();
+// CHECK:STDERR: ^~~~~~~
+default protected fn WrongOrder();
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `virtual` not allowed on `fn` declaration outside of a class.
+// CHECK:STDERR: virtual virtual fn DuplicateVirtual() {}
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `virtual` repeated on declaration.
+// CHECK:STDERR: virtual virtual fn DuplicateVirtual() {}
+// CHECK:STDERR:         ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `virtual` previously appeared here.
+// CHECK:STDERR: virtual virtual fn DuplicateVirtual() {}
+// CHECK:STDERR: ^~~~~~~
+virtual virtual fn DuplicateVirtual() {}
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private protected fn TwoAccess();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `protected` not allowed on declaration with `private`.
+// CHECK:STDERR: private protected fn TwoAccess();
+// CHECK:STDERR:         ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `private` previously appeared here.
+// CHECK:STDERR: private protected fn TwoAccess();
+// CHECK:STDERR: ^~~~~~~
+private protected fn TwoAccess();
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `abstract` not allowed on `fn` declaration outside of a class.
+// CHECK:STDERR: abstract virtual fn ModifiersConflict() {}
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:10: ERROR: `virtual` not allowed on declaration with `abstract`.
+// CHECK:STDERR: abstract virtual fn ModifiersConflict() {}
+// CHECK:STDERR:          ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `abstract` previously appeared here.
+// CHECK:STDERR: abstract virtual fn ModifiersConflict() {}
+// CHECK:STDERR: ^~~~~~~~
+abstract virtual fn ModifiersConflict() {}
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `base` not allowed on `fn` declaration.
+// CHECK:STDERR: base fn InvalidModifier();
+// CHECK:STDERR: ^~~~
+base fn InvalidModifier();
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+15]]:1: ERROR: `default` not allowed on `fn` declaration outside of an interface.
+// CHECK:STDERR: default final virtual fn ModifiersConflict2() {}
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+12]]:9: ERROR: `final` not allowed on declaration with `default`.
+// CHECK:STDERR: default final virtual fn ModifiersConflict2() {}
+// CHECK:STDERR:         ^~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: `default` previously appeared here.
+// CHECK:STDERR: default final virtual fn ModifiersConflict2() {}
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:15: ERROR: `virtual` not allowed on declaration with `default`.
+// CHECK:STDERR: default final virtual fn ModifiersConflict2() {}
+// CHECK:STDERR:               ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `default` previously appeared here.
+// CHECK:STDERR: default final virtual fn ModifiersConflict2() {}
+// CHECK:STDERR: ^~~~~~~
+default final virtual fn ModifiersConflict2() {}
+
+// CHECK:STDOUT: --- fail_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.WrongOrder = %WrongOrder, .DuplicateVirtual = %DuplicateVirtual, .TwoAccess = %TwoAccess, .ModifiersConflict = %ModifiersConflict, .InvalidModifier = %InvalidModifier, .ModifiersConflict2 = %ModifiersConflict2}
+// CHECK:STDOUT:   %WrongOrder: <function> = fn_decl @WrongOrder
+// CHECK:STDOUT:   %DuplicateVirtual: <function> = fn_decl @DuplicateVirtual
+// CHECK:STDOUT:   %TwoAccess: <function> = fn_decl @TwoAccess
+// CHECK:STDOUT:   %ModifiersConflict: <function> = fn_decl @ModifiersConflict
+// CHECK:STDOUT:   %InvalidModifier: <function> = fn_decl @InvalidModifier
+// CHECK:STDOUT:   %ModifiersConflict2: <function> = fn_decl @ModifiersConflict2
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @WrongOrder();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @DuplicateVirtual() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @TwoAccess();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @ModifiersConflict() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @InvalidModifier();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @ModifiersConflict2() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/function/declaration/fail_todo_modifiers.carbon

@@ -0,0 +1,20 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private fn F();
+// CHECK:STDERR: ^~~~~~~
+private fn F();
+
+// CHECK:STDOUT: --- fail_todo_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.F = %F}
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT:

toolchain/check/testdata/let/fail_modifiers.carbon

@@ -0,0 +1,92 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected let b: i32 = 1;
+// CHECK:STDERR: ^~~~~~~~~
+protected let b: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `default` not allowed on `let` declaration outside of an interface.
+// CHECK:STDERR: default let c: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+default let c: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `final` not allowed on `let` declaration outside of an interface.
+// CHECK:STDERR: final let d: i32 = 1;
+// CHECK:STDERR: ^~~~~
+final let d: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `virtual` not allowed on `let` declaration.
+// CHECK:STDERR: virtual let e: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+virtual let e: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `default` not allowed on `let` declaration outside of an interface.
+// CHECK:STDERR: default final let f: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `final` not allowed on declaration with `default`.
+// CHECK:STDERR: default final let f: i32 = 1;
+// CHECK:STDERR:         ^~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `default` previously appeared here.
+// CHECK:STDERR: default final let f: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+default final let f: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `default` not allowed on `let` declaration outside of an interface.
+// CHECK:STDERR: default default let g: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `default` repeated on declaration.
+// CHECK:STDERR: default default let g: i32 = 1;
+// CHECK:STDERR:         ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `default` previously appeared here.
+// CHECK:STDERR: default default let g: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+default default let g: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected private let h: i32 = 1;
+// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:11: ERROR: `private` not allowed on declaration with `protected`.
+// CHECK:STDERR: protected private let h: i32 = 1;
+// CHECK:STDERR:           ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `protected` previously appeared here.
+// CHECK:STDERR: protected private let h: i32 = 1;
+// CHECK:STDERR: ^~~~~~~~~
+protected private let h: i32 = 1;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `protected` not allowed on `let` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected protected let i: i32 = 1;
+// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:11: ERROR: `protected` repeated on declaration.
+// CHECK:STDERR: protected protected let i: i32 = 1;
+// CHECK:STDERR:           ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `protected` previously appeared here.
+// CHECK:STDERR: protected protected let i: i32 = 1;
+// CHECK:STDERR: ^~~~~~~~~
+protected protected let i: i32 = 1;
+
+// CHECK:STDOUT: --- fail_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {}
+// CHECK:STDOUT:   %.loc10: i32 = int_literal 1
+// CHECK:STDOUT:   %b: i32 = bind_name b, %.loc10
+// CHECK:STDOUT:   %.loc15: i32 = int_literal 1
+// CHECK:STDOUT:   %c: i32 = bind_name c, %.loc15
+// CHECK:STDOUT:   %.loc20: i32 = int_literal 1
+// CHECK:STDOUT:   %d: i32 = bind_name d, %.loc20
+// CHECK:STDOUT:   %.loc25: i32 = int_literal 1
+// CHECK:STDOUT:   %e: i32 = bind_name e, %.loc25
+// CHECK:STDOUT:   %.loc36: i32 = int_literal 1
+// CHECK:STDOUT:   %f: i32 = bind_name f, %.loc36
+// CHECK:STDOUT:   %.loc47: i32 = int_literal 1
+// CHECK:STDOUT:   %g: i32 = bind_name g, %.loc47
+// CHECK:STDOUT:   %.loc58: i32 = int_literal 1
+// CHECK:STDOUT:   %h: i32 = bind_name h, %.loc58
+// CHECK:STDOUT:   %.loc69: i32 = int_literal 1
+// CHECK:STDOUT:   %i: i32 = bind_name i, %.loc69
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/let/fail_todo_modifiers.carbon

@@ -0,0 +1,19 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private let a: i32 = 1;
+// CHECK:STDERR: ^~~~~~~
+private let a: i32 = 1;
+
+// CHECK:STDOUT: --- fail_todo_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {}
+// CHECK:STDOUT:   %.loc10: i32 = int_literal 1
+// CHECK:STDOUT:   %a: i32 = bind_name a, %.loc10
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/var/fail_modifiers.carbon

@@ -0,0 +1,52 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `protected` not allowed on `var` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected var b: i32;
+// CHECK:STDERR: ^~~~~~~~~
+protected var b: i32;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private protected var c: i32;
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:9: ERROR: `protected` not allowed on declaration with `private`.
+// CHECK:STDERR: private protected var c: i32;
+// CHECK:STDERR:         ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `private` previously appeared here.
+// CHECK:STDERR: private protected var c: i32;
+// CHECK:STDERR: ^~~~~~~
+private protected var c: i32;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+9]]:1: ERROR: `protected` not allowed on `var` declaration at file scope, `protected` is only allowed on class members.
+// CHECK:STDERR: protected protected var d: i32;
+// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+6]]:11: ERROR: `protected` repeated on declaration.
+// CHECK:STDERR: protected protected var d: i32;
+// CHECK:STDERR:           ^~~~~~~~~
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: `protected` previously appeared here.
+// CHECK:STDERR: protected protected var d: i32;
+// CHECK:STDERR: ^~~~~~~~~
+protected protected var d: i32;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: `abstract` not allowed on `var` declaration.
+// CHECK:STDERR: abstract var e: i32;
+// CHECK:STDERR: ^~~~~~~~
+abstract var e: i32;
+
+// CHECK:STDOUT: --- fail_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.b = %b, .c = %c, .d = %d, .e = %e}
+// CHECK:STDOUT:   %b.var: ref i32 = var b
+// CHECK:STDOUT:   %b: ref i32 = bind_name b, %b.var
+// CHECK:STDOUT:   %c.var: ref i32 = var c
+// CHECK:STDOUT:   %c: ref i32 = bind_name c, %c.var
+// CHECK:STDOUT:   %d.var: ref i32 = var d
+// CHECK:STDOUT:   %d: ref i32 = bind_name d, %d.var
+// CHECK:STDOUT:   %e.var: ref i32 = var e
+// CHECK:STDOUT:   %e: ref i32 = bind_name e, %e.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/var/fail_todo_modifiers.carbon

@@ -0,0 +1,19 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `access modifier`.
+// CHECK:STDERR: private var a: i32;
+// CHECK:STDERR: ^~~~~~~
+private var a: i32;
+
+// CHECK:STDOUT: --- fail_todo_modifiers.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace {.a = %a}
+// CHECK:STDOUT:   %a.var: ref i32 = var a
+// CHECK:STDOUT:   %a: ref i32 = bind_name a, %a.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/diagnostics/diagnostic_kind.def

@@ -109,6 +109,8 @@ CARBON_DIAGNOSTIC_KIND(ExpectedInitializerAfterLet)
 CARBON_DIAGNOSTIC_KIND(MethodImplNotAllowed)
 CARBON_DIAGNOSTIC_KIND(ParamsRequiredAfterImplicit)
 CARBON_DIAGNOSTIC_KIND(ParamsRequiredByIntroducer)
+CARBON_DIAGNOSTIC_KIND(ExpectedAfterBase)
+CARBON_DIAGNOSTIC_KIND(NamespaceAfterModifiers)
 
 // ============================================================================
 // Semantics diagnostics
@@ -216,6 +218,14 @@ CARBON_DIAGNOSTIC_KIND(QualifiedExprUnsupported)
 CARBON_DIAGNOSTIC_KIND(QualifiedExprNameNotFound)
 CARBON_DIAGNOSTIC_KIND(UseOfNonExprAsValue)
 
+// Modifier checking.
+CARBON_DIAGNOSTIC_KIND(ModifierNotAllowedOn)
+CARBON_DIAGNOSTIC_KIND(ModifierNotInContext)
+CARBON_DIAGNOSTIC_KIND(ModifierRepeated)
+CARBON_DIAGNOSTIC_KIND(ModifierNotAllowedWith)
+CARBON_DIAGNOSTIC_KIND(ModifierMustAppearBefore)
+CARBON_DIAGNOSTIC_KIND(ModifierPrevious)
+
 // ============================================================================
 // Other diagnostics
 // ============================================================================

toolchain/diagnostics/testdata/fail_multiline_token.carbon

@@ -2,7 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-// ARGS: compile %s
+// ARGS: compile --phase=parse %s
 //
 // AUTOUPDATE
 

toolchain/lex/testdata/keywords.carbon

@@ -89,49 +89,47 @@ observe
 // CHECK:STDOUT:     { index: 40, kind:             'Observe', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'observe', has_trailing_space: true },
 or
 // CHECK:STDOUT:     { index: 41, kind:                  'Or', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'or', has_trailing_space: true },
-override
-// CHECK:STDOUT:     { index: 42, kind:            'Override', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'override', has_trailing_space: true },
 package
-// CHECK:STDOUT:     { index: 43, kind:             'Package', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'package', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 42, kind:             'Package', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'package', has_trailing_space: true },
 partial
-// CHECK:STDOUT:     { index: 44, kind:             'Partial', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'partial', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 43, kind:             'Partial', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'partial', has_trailing_space: true },
 private
-// CHECK:STDOUT:     { index: 45, kind:             'Private', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'private', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 44, kind:             'Private', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'private', has_trailing_space: true },
 protected
-// CHECK:STDOUT:     { index: 46, kind:           'Protected', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'protected', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 45, kind:           'Protected', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'protected', has_trailing_space: true },
 require
-// CHECK:STDOUT:     { index: 47, kind:             'Require', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'require', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 46, kind:             'Require', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'require', has_trailing_space: true },
 return
-// CHECK:STDOUT:     { index: 48, kind:              'Return', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'return', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 47, kind:              'Return', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'return', has_trailing_space: true },
 returned
-// CHECK:STDOUT:     { index: 49, kind:            'Returned', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'returned', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 48, kind:            'Returned', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'returned', has_trailing_space: true },
 Self
-// CHECK:STDOUT:     { index: 50, kind:  'SelfTypeIdentifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'Self', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 49, kind:  'SelfTypeIdentifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'Self', has_trailing_space: true },
 self
-// CHECK:STDOUT:     { index: 51, kind: 'SelfValueIdentifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'self', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 50, kind: 'SelfValueIdentifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'self', has_trailing_space: true },
 String
-// CHECK:STDOUT:     { index: 52, kind:   'StringTypeLiteral', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'String', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 51, kind:   'StringTypeLiteral', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'String', has_trailing_space: true },
 template
-// CHECK:STDOUT:     { index: 53, kind:            'Template', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'template', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 52, kind:            'Template', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'template', has_trailing_space: true },
 then
-// CHECK:STDOUT:     { index: 54, kind:                'Then', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'then', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 53, kind:                'Then', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'then', has_trailing_space: true },
 true
-// CHECK:STDOUT:     { index: 55, kind:                'True', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'true', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 54, kind:                'True', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'true', has_trailing_space: true },
 type
-// CHECK:STDOUT:     { index: 56, kind:                'Type', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'type', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 55, kind:                'Type', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'type', has_trailing_space: true },
 _
-// CHECK:STDOUT:     { index: 57, kind:          'Underscore', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: '_', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 56, kind:          'Underscore', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: '_', has_trailing_space: true },
 var
-// CHECK:STDOUT:     { index: 58, kind:                 'Var', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'var', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 57, kind:                 'Var', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'var', has_trailing_space: true },
 virtual
-// CHECK:STDOUT:     { index: 59, kind:             'Virtual', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'virtual', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 58, kind:             'Virtual', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'virtual', has_trailing_space: true },
 where
-// CHECK:STDOUT:     { index: 60, kind:               'Where', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'where', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 59, kind:               'Where', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'where', has_trailing_space: true },
 while
-// CHECK:STDOUT:     { index: 61, kind:               'While', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'while', has_trailing_space: true },
+// CHECK:STDOUT:     { index: 60, kind:               'While', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'while', has_trailing_space: true },
 
 notakeyword
-// CHECK:STDOUT:     { index: 62, kind:          'Identifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'notakeyword', identifier: 0, has_trailing_space: true },
+// CHECK:STDOUT:     { index: 61, kind:          'Identifier', line: {{ *}}[[@LINE-1]], column:  1, indent: 1, spelling: 'notakeyword', identifier: 0, has_trailing_space: true },
 
-// CHECK:STDOUT:     { index: 63, kind:             'FileEnd', line: {{ *}}[[@LINE+1]], column: {{ *\d+}}, indent: 1, spelling: '' },
+// CHECK:STDOUT:     { index: 62, kind:             'FileEnd', line: {{ *}}[[@LINE+1]], column: {{ *\d+}}, indent: 1, spelling: '' },
 // CHECK:STDOUT:   ]

toolchain/lex/token_kind.def

@@ -181,7 +181,6 @@ CARBON_KEYWORD_TOKEN(Not,                 "not")
 CARBON_KEYWORD_TOKEN(Observe,             "observe")
 CARBON_TOKEN_WITH_VIRTUAL_NODE(
   CARBON_KEYWORD_TOKEN(Or,                "or"))
-CARBON_KEYWORD_TOKEN(Override,            "override")
 CARBON_KEYWORD_TOKEN(Package,             "package")
 CARBON_KEYWORD_TOKEN(Partial,             "partial")
 CARBON_KEYWORD_TOKEN(Private,             "private")

toolchain/lower/testdata/basics/fail_before_lowering.carbon

@@ -5,7 +5,10 @@
 // This validates that earlier errors prevent lowering, without crashing.
 // AUTOUPDATE
 
-// CHECK:STDERR: fail_before_lowering.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: fail_before_lowering.carbon:[[@LINE+6]]:1: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: a;
+// CHECK:STDERR: ^
+// CHECK:STDERR: fail_before_lowering.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `HandleInvalidParseStart`.
 // CHECK:STDERR: a;
 // CHECK:STDERR: ^
 a;

toolchain/parse/context.cpp

@@ -88,6 +88,22 @@ auto Context::AddNode(NodeKind kind, Lex::TokenIndex token, int subtree_start,
   }
 }
 
+auto Context::ReplacePlaceholderNode(int32_t position, NodeKind kind,
+                                     Lex::TokenIndex token, bool has_error)
+    -> void {
+  CARBON_CHECK(position >= 0 && position < tree_->size())
+      << "position: " << position << " size: " << tree_->size();
+  auto* node_impl = &tree_->node_impls_[position];
+  CARBON_CHECK(node_impl->subtree_size == 1);
+  CARBON_CHECK(node_impl->kind == NodeKind::Placeholder);
+  node_impl->kind = kind;
+  node_impl->has_error = has_error;
+  node_impl->token = token;
+  if (has_error) {
+    tree_->has_errors_ = true;
+  }
+}
+
 auto Context::ConsumeAndAddOpenParen(Lex::TokenIndex default_token,
                                      NodeKind start_kind)
     -> std::optional<Lex::TokenIndex> {

toolchain/parse/context.h

@@ -128,6 +128,25 @@ class Context {
   auto AddNode(NodeKind kind, Lex::TokenIndex token, int subtree_start,
                bool has_error) -> void;
 
+  // Replaces the placeholder node at the indicated position with a leaf node.
+  //
+  // To reserve a position in the parse tree, you may add a placeholder parse
+  // node using code like:
+  //   ```
+  //   context.PushState(State::WillFillInPlaceholder);
+  //   context.AddLeafNode(NodeKind::Placeholder, *context.position());
+  //   ```
+  // It may be replaced with the intended leaf parse node with code like:
+  //   ```
+  //   auto HandleWillFillInPlaceholder(Context& context) -> void {
+  //     auto state = context.PopState();
+  //     context.ReplacePlaceholderNode(state.subtree_start, /* replacement */);
+  //   }
+  //   ```
+  auto ReplacePlaceholderNode(int32_t position, NodeKind kind,
+                              Lex::TokenIndex token, bool has_error = false)
+      -> void;
+
   // Returns the current position and moves past it.
   auto Consume() -> Lex::TokenIndex { return *(position_++); }
 

toolchain/parse/handle_decl_scope_loop.cpp

@@ -6,24 +6,61 @@
 
 namespace Carbon::Parse {
 
-// Handles an unrecognized declaration, adding an error node.
-static auto HandleUnrecognizedDecl(Context& context) -> void {
+static auto OutputInvalidParseSubtree(Context& context, int32_t subtree_start)
+    -> void {
+  auto cursor = *context.position();
+  // Consume to the next `;` or end of line. We ignore the return value since
+  // we only care how much was consumed, not whether it ended with a `;`.
+  // TODO: adjust the return of SkipPastLikelyEnd or create a new function
+  // to avoid going through these hoops.
+  context.SkipPastLikelyEnd(cursor);
+  // Set `iter` to the last token consumed, one before the current position.
+  auto iter = context.position();
+  --iter;
+  // Output an invalid parse subtree including everything up to the last token
+  // consumed.
+  context.ReplacePlaceholderNode(subtree_start, NodeKind::InvalidParseStart,
+                                 cursor, /*has_error=*/true);
+  context.AddNode(NodeKind::InvalidParseSubtree, *iter, subtree_start,
+                  /*has_error=*/true);
+}
+
+// Handles an unrecognized declaration.
+static auto HandleUnrecognizedDecl(Context& context, int32_t subtree_start)
+    -> void {
   CARBON_DIAGNOSTIC(UnrecognizedDecl, Error,
                     "Unrecognized declaration introducer.");
   context.emitter().Emit(*context.position(), UnrecognizedDecl);
-  auto cursor = *context.position();
-  auto semi = context.SkipPastLikelyEnd(cursor);
-  // Locate the EmptyDecl at the semi when found, but use the
-  // original cursor location for an error when not.
-  context.AddLeafNode(NodeKind::EmptyDecl, semi ? *semi : cursor,
-                      /*has_error=*/true);
+  OutputInvalidParseSubtree(context, subtree_start);
+}
+
+static auto TokenIsModifierOrIntroducer(Lex::TokenKind token_kind) -> bool {
+  switch (token_kind) {
+    case Lex::TokenKind::Abstract:
+    case Lex::TokenKind::Base:
+    case Lex::TokenKind::Class:
+    case Lex::TokenKind::Constraint:
+    case Lex::TokenKind::Default:
+    case Lex::TokenKind::Final:
+    case Lex::TokenKind::Fn:
+    case Lex::TokenKind::Impl:
+    case Lex::TokenKind::Interface:
+    case Lex::TokenKind::Let:
+    case Lex::TokenKind::Private:
+    case Lex::TokenKind::Protected:
+    case Lex::TokenKind::Var:
+    case Lex::TokenKind::Virtual:
+      return true;
+
+    default:
+      return false;
+  }
 }
 
 auto HandleDeclScopeLoop(Context& context) -> void {
   // This maintains the current state unless we're at the end of the scope.
 
-  auto position_kind = context.PositionKind();
-  switch (position_kind) {
+  switch (context.PositionKind()) {
     case Lex::TokenKind::CloseCurlyBrace:
     case Lex::TokenKind::FileEnd: {
       // This is the end of the scope, so the loop state ends.
@@ -61,76 +98,161 @@ auto HandleDeclScopeLoop(Context& context) -> void {
   // Remaining keywords are only valid after imports are complete, and so all
   // result in a `set_packaging_state` call. Note, this may not always be
   // necessary but is probably cheaper than validating.
-  switch (position_kind) {
-    case Lex::TokenKind::Base: {
-      if (context.PositionIs(Lex::TokenKind::Colon, Lookahead::NextToken)) {
-        context.PushState(State::BaseDecl);
-        context.PushState(State::Expr);
-        context.AddLeafNode(NodeKind::BaseIntroducer, context.Consume());
-        context.AddLeafNode(NodeKind::BaseColon, context.Consume());
-        return;
+
+  // Create a state with the correct starting position, with a dummy kind until
+  // we see the declaration's introducer.
+  context.PushState(State::DeclScopeLoop);
+  auto state = context.PopState();
+
+  // Add a placeholder node, to be replaced by the declaration introducer once
+  // it is found.
+  context.AddLeafNode(NodeKind::Placeholder, *context.position());
+
+  auto introducer = [&](NodeKind node_kind, State next_state) {
+    context.ReplacePlaceholderNode(state.subtree_start, node_kind,
+                                   context.Consume());
+    // Reuse state here to retain its `subtree_start`
+    state.state = next_state;
+    context.PushState(state);
+  };
+
+  bool saw_modifier = false;
+  while (true) {
+    switch (context.PositionKind()) {
+      // If we see a access modifier keyword token, add it as a leaf node
+      // and repeat with the next token.
+      case Lex::TokenKind::Private:
+      case Lex::TokenKind::Protected: {
+        auto modifier_token = context.Consume();
+        context.AddLeafNode(NodeKind::AccessModifierKeyword, modifier_token);
+        saw_modifier = true;
+        break;
       }
-      // TODO: If the next token isn't a colon or `class`, try to recover based
-      // on whether we're in a class, whether we have an `extend` modifier, and
-      // the following tokens.
-      [[fallthrough]];
-    }
-    case Lex::TokenKind::Abstract: {
-      if (context.PositionIs(Lex::TokenKind::Class, Lookahead::NextToken)) {
-        context.PushState(State::TypeAfterIntroducerAsClass);
+
+      case Lex::TokenKind::Base:
+        // `base` may be followed by:
+        // - a colon
+        //   => assume it is an introducer, as in `extend base: BaseType;`.
+        // - a modifier or an introducer
+        //   => assume it is a modifier, as in `base class`; which is handled
+        //      by falling through to the next case.
+        // Anything else is an error.
+        if (context.PositionIs(Lex::TokenKind::Colon, Lookahead::NextToken)) {
+          context.ReplacePlaceholderNode(
+              state.subtree_start, NodeKind::BaseIntroducer, context.Consume());
+          // Reuse state here to retain its `subtree_start`
+          state.state = State::BaseDecl;
+          context.PushState(state);
+          context.PushState(State::Expr);
+          context.AddLeafNode(NodeKind::BaseColon, context.Consume());
+          return;
+        } else if (!TokenIsModifierOrIntroducer(
+                       context.PositionKind(Lookahead::NextToken))) {
+          // TODO: If the next token isn't a colon or `class`, try to recover
+          // based on whether we're in a class, whether we have an `extend`
+          // modifier, and the following tokens.
+          context.AddLeafNode(NodeKind::InvalidParse, context.Consume(),
+                              /*has_error=*/true);
+          CARBON_DIAGNOSTIC(ExpectedAfterBase, Error,
+                            "`class` or `:` expected after `base`.");
+          context.emitter().Emit(*context.position(), ExpectedAfterBase);
+          OutputInvalidParseSubtree(context, state.subtree_start);
+          return;
+        }
+        [[fallthrough]];
+
+      // If we see a declaration modifier keyword token, add it as a leaf node
+      // and repeat with the next token.
+      case Lex::TokenKind::Abstract:
+      case Lex::TokenKind::Default:
+      case Lex::TokenKind::Final:
+      case Lex::TokenKind::Virtual: {
         auto modifier_token = context.Consume();
-        auto class_token = context.Consume();
-        context.AddLeafNode(NodeKind::ClassIntroducer, class_token);
-        context.AddLeafNode(position_kind == Lex::TokenKind::Abstract
-                                ? NodeKind::AbstractModifier
-                                : NodeKind::BaseModifier,
-                            modifier_token);
+        context.AddLeafNode(NodeKind::DeclModifierKeyword, modifier_token);
+        saw_modifier = true;
+        break;
+      }
+
+      case Lex::TokenKind::Impl: {
+        // `impl` is considered a declaration modifier if it is followed by
+        // another modifier or an introducer.
+        if (TokenIsModifierOrIntroducer(
+                context.PositionKind(Lookahead::NextToken))) {
+          context.AddLeafNode(NodeKind::DeclModifierKeyword, context.Consume());
+          saw_modifier = true;
+        } else {
+          // TODO: Treat this `impl` token as a declaration introducer
+          HandleUnrecognizedDecl(context, state.subtree_start);
+          return;
+        }
+        break;
+      }
+
+      // If we see a declaration introducer keyword token, replace the
+      // placeholder node and switch to a state to parse the rest of the
+      // declaration. We don't allow namespace or empty declarations here since
+      // they can't have modifiers and don't use bracketing parse nodes that
+      // would allow a variable number of modifier nodes.
+      case Lex::TokenKind::Class: {
+        introducer(NodeKind::ClassIntroducer,
+                   State::TypeAfterIntroducerAsClass);
+        return;
+      }
+      case Lex::TokenKind::Constraint: {
+        introducer(NodeKind::NamedConstraintIntroducer,
+                   State::TypeAfterIntroducerAsNamedConstraint);
+        return;
+      }
+      case Lex::TokenKind::Fn: {
+        introducer(NodeKind::FunctionIntroducer, State::FunctionIntroducer);
+        return;
+      }
+      case Lex::TokenKind::Interface: {
+        introducer(NodeKind::InterfaceIntroducer,
+                   State::TypeAfterIntroducerAsInterface);
+        return;
+      }
+      case Lex::TokenKind::Var: {
+        introducer(NodeKind::VariableIntroducer, State::VarAsDecl);
+        return;
+      }
+      case Lex::TokenKind::Let: {
+        introducer(NodeKind::LetIntroducer, State::Let);
+        return;
+      }
+
+      // We don't allow namespace or empty declarations after a modifier since
+      // they can't have modifiers and don't use bracketing parse nodes that
+      // would allow a variable number of modifier nodes.
+      case Lex::TokenKind::Namespace: {
+        if (saw_modifier) {
+          CARBON_DIAGNOSTIC(NamespaceAfterModifiers, Error,
+                            "`namespace` unexpected after modifiers.");
+          context.emitter().Emit(*context.position(), NamespaceAfterModifiers);
+          OutputInvalidParseSubtree(context, state.subtree_start);
+        } else {
+          introducer(NodeKind::NamespaceStart, State::Namespace);
+        }
+        return;
+      }
+      case Lex::TokenKind::Semi: {
+        if (saw_modifier) {
+          HandleUnrecognizedDecl(context, state.subtree_start);
+        } else {
+          context.ReplacePlaceholderNode(
+              state.subtree_start, NodeKind::EmptyDecl, context.Consume());
+        }
+        return;
+      }
+
+      default: {
+        // For anything else, report an error and output an invalid parse node
+        // or subtree.
+        HandleUnrecognizedDecl(context, state.subtree_start);
         return;
       }
-      break;
-    }
-    case Lex::TokenKind::Class: {
-      context.PushState(State::TypeAfterIntroducerAsClass);
-      context.AddLeafNode(NodeKind::ClassIntroducer, context.Consume());
-      return;
-    }
-    case Lex::TokenKind::Constraint: {
-      context.PushState(State::TypeAfterIntroducerAsNamedConstraint);
-      context.AddLeafNode(NodeKind::NamedConstraintIntroducer,
-                          context.Consume());
-      return;
-    }
-    case Lex::TokenKind::Fn: {
-      context.PushState(State::FunctionIntroducer);
-      return;
-    }
-    case Lex::TokenKind::Interface: {
-      context.PushState(State::TypeAfterIntroducerAsInterface);
-      context.AddLeafNode(NodeKind::InterfaceIntroducer, context.Consume());
-      return;
-    }
-    case Lex::TokenKind::Namespace: {
-      context.PushState(State::Namespace);
-      return;
-    }
-    case Lex::TokenKind::Semi: {
-      context.AddLeafNode(NodeKind::EmptyDecl, context.Consume());
-      return;
-    }
-    case Lex::TokenKind::Var: {
-      context.PushState(State::VarAsDecl);
-      return;
-    }
-    case Lex::TokenKind::Let: {
-      context.PushState(State::Let);
-      return;
-    }
-    default: {
-      break;
     }
   }
-
-  HandleUnrecognizedDecl(context);
 }
 
 }  // namespace Carbon::Parse

toolchain/parse/handle_function.cpp

@@ -8,9 +8,6 @@ namespace Carbon::Parse {
 
 auto HandleFunctionIntroducer(Context& context) -> void {
   auto state = context.PopState();
-
-  context.AddLeafNode(NodeKind::FunctionIntroducer, context.Consume());
-
   state.state = State::FunctionAfterParams;
   context.PushState(state);
   context.PushState(State::DeclNameAndParamsAsRequired, state.token);

toolchain/parse/handle_let.cpp

@@ -7,13 +7,13 @@
 namespace Carbon::Parse {
 
 auto HandleLet(Context& context) -> void {
-  context.PopAndDiscardState();
+  auto state = context.PopState();
 
   // These will start at the `let`.
-  context.PushState(State::LetFinish);
-  context.PushState(State::LetAfterPattern);
-
-  context.AddLeafNode(NodeKind::LetIntroducer, context.Consume());
+  state.state = State::LetFinish;
+  context.PushState(state);
+  state.state = State::LetAfterPattern;
+  context.PushState(state);
 
   // This will start at the pattern.
   context.PushState(State::BindingPatternAsLet);

toolchain/parse/handle_namespace.cpp

@@ -8,9 +8,6 @@ namespace Carbon::Parse {
 
 auto HandleNamespace(Context& context) -> void {
   auto state = context.PopState();
-
-  context.AddLeafNode(NodeKind::NamespaceStart, context.Consume());
-
   state.state = State::NamespaceFinish;
   context.PushState(state);
 

toolchain/parse/handle_statement.cpp

@@ -32,6 +32,7 @@ auto HandleStatement(Context& context) -> void {
     }
     case Lex::TokenKind::Let: {
       context.PushState(State::Let);
+      context.AddLeafNode(NodeKind::LetIntroducer, context.Consume());
       break;
     }
     case Lex::TokenKind::Return: {
@@ -44,6 +45,7 @@ auto HandleStatement(Context& context) -> void {
     }
     case Lex::TokenKind::Var: {
       context.PushState(State::VarAsDecl);
+      context.AddLeafNode(NodeKind::VariableIntroducer, context.Consume());
       break;
     }
     case Lex::TokenKind::While: {
@@ -101,6 +103,7 @@ auto HandleStatementForHeader(Context& context) -> void {
   if (context.PositionIs(Lex::TokenKind::Var)) {
     context.PushState(state);
     context.PushState(State::VarAsFor);
+    context.AddLeafNode(NodeKind::VariableIntroducer, context.Consume());
   } else {
     CARBON_DIAGNOSTIC(ExpectedVariableDecl, Error,
                       "Expected `var` declaration.");

toolchain/parse/handle_var.cpp

@@ -16,9 +16,9 @@ static auto HandleVar(Context& context, State finish_state,
   state.state = finish_state;
   context.PushState(state);
 
-  context.PushState(State::VarAfterPattern);
+  state.state = State::VarAfterPattern;
+  context.PushState(state);
 
-  context.AddLeafNode(NodeKind::VariableIntroducer, context.Consume());
   if (returned_token.is_valid()) {
     context.AddLeafNode(NodeKind::ReturnedModifier, returned_token);
   }
@@ -44,6 +44,7 @@ auto HandleVarAsReturned(Context& context) -> void {
     return;
   }
 
+  context.AddLeafNode(NodeKind::VariableIntroducer, context.Consume());
   HandleVar(context, State::VarFinishAsDecl, returned_token);
 }
 

toolchain/parse/node_kind.def

@@ -70,6 +70,15 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(FileEnd, 0, CARBON_TOKEN(FileEnd))
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(InvalidParse, 0,
                                    CARBON_IF_ERROR(CARBON_ANY_TOKEN))
 
+// An invalid subtree. Always has an error.
+CARBON_PARSE_NODE_KIND_CHILD_COUNT(InvalidParseStart, 0,
+                                   CARBON_IF_ERROR(CARBON_ANY_TOKEN))
+CARBON_PARSE_NODE_KIND_BRACKET(InvalidParseSubtree, InvalidParseStart,
+                               CARBON_IF_ERROR(CARBON_ANY_TOKEN))
+
+// A placeholder node to be replaced.
+CARBON_PARSE_NODE_KIND_CHILD_COUNT(Placeholder, 0, CARBON_ANY_TOKEN)
+
 // An empty declaration, such as `;`.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(EmptyDecl, 0,
                                    CARBON_TOKEN(Semi)
@@ -98,10 +107,12 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(NameExpr, 0, CARBON_TOKEN(Identifier))
 // tree.h for more information.
 //
 // A parse node kind may be preceded by:
+// -  `_optional_` if this node (or nodes) may be present or omitted in valid
+//    parses, depending on which tokens are in the source code.
+// -  `_repeated_` if this node (or nodes) may be repeated or omitted in valid
+//    parses, depending on which tokens are in the source code.
 // -  `_external_:` if this node is the child of multiple kinds of nodes and
 //    is documented separately.
-// -  `_optional_` if this node may be present or omitted in valid parses,
-//    depending on which tokens are in the source code.
 //
 // There is generally a close correspondence between handling of tokens and the
 // creation of non-external nodes in a given block.
@@ -160,7 +171,7 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(Namespace, 2,
 
 // A code block:
 //   CodeBlockStart
-//   _external_: statements
+//   _repeated_ _external_: statement
 // CodeBlock
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(CodeBlockStart, 0,
                                    CARBON_TOKEN(OpenCurlyBrace)
@@ -171,12 +182,13 @@ CARBON_PARSE_NODE_KIND_BRACKET(CodeBlock, CodeBlockStart,
 
 // `fn`:
 //     FunctionIntroducer
+//     _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //     _external_: Name or QualifiedDecl
 //     _external_: ParamList
 //       _external_: type expression
 //     ReturnType
 //   FunctionDefinitionStart
-//   _external_: statements
+//   _repeated_ _external_: statement
 // FunctionDefinition
 //
 // The above is the structure for a definition; for a declaration,
@@ -194,12 +206,13 @@ CARBON_PARSE_NODE_KIND_BRACKET(FunctionDecl, FunctionIntroducer,
 
 // A parameter list, possibly implicit:
 //   [Implicit]ParamListStart
-//   _external_: [Generic]BindingPattern
-//   ParamListComma
+//     _external_: [Generic]BindingPattern
+//     ParamListComma
+//   _repeated_
 // [Implicit]ParamList
 //
-// Exprs and ParamListComma may repeat with ParamListComma as a
-// separator.
+// [Generic]PatternBinding and ParamListComma may repeat with ParamListComma
+// as a separator.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(ParamListStart, 0, CARBON_TOKEN(OpenParen))
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(ImplicitParamListStart, 0,
                                    CARBON_TOKEN(OpenSquareBracket))
@@ -239,10 +252,13 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(Template, 1, CARBON_TOKEN(Template))
 
 // `let`:
 //   LetIntroducer
+//   _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //   _external_: BindingPattern
 //   LetInitializer
 //   _external_: expression
 // LetDecl
+//
+// Modifier keywords only appear for `let` declarations, not `let` statements.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(LetIntroducer, 0, CARBON_TOKEN(Let))
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(LetInitializer, 0, CARBON_TOKEN(Equal))
 CARBON_PARSE_NODE_KIND_BRACKET(LetDecl, LetIntroducer,
@@ -251,12 +267,16 @@ CARBON_PARSE_NODE_KIND_BRACKET(LetDecl, LetIntroducer,
 
 // `var` and `returned var`:
 //   VariableIntroducer
+//   _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //   _optional_ ReturnedModifier
 //   _external_: BindingPattern
-//   _optional_ VariableInitializer
-//   _optional_ _external_: expression
+//     VariableInitializer
+//     _external_: expression
+//   _optional_
 // VariableDecl
 //
+// Access and declaration modifier keywords only appear for `var` declarations,
+// whereas the returned modifier only appears on `var` statements.
 // The VariableInitializer and following expression are paired: either both will
 // be present, or neither will.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(VariableIntroducer, 0,
@@ -372,11 +392,12 @@ CARBON_PARSE_NODE_KIND_BRACKET(IndexExpr, IndexExprStart,
 //
 // Tuples, such as `(1, 2)`:
 //   ParenExprOrTupleLiteralStart
-//   _external_: expression
-//   TupleLiteralComma
+//     _external_: expression
+//     TupleLiteralComma
+//   _repeated_
 // TupleLiteral
 //
-// Exprs and TupleLiteralComma may repeat with TupleLiteralComma as a
+// Expressions and TupleLiteralComma may repeat with TupleLiteralComma as a
 // separator.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(ParenExprOrTupleLiteralStart, 0,
                                    CARBON_TOKEN(OpenParen))
@@ -389,12 +410,12 @@ CARBON_PARSE_NODE_KIND_BRACKET(TupleLiteral, ParenExprOrTupleLiteralStart,
 // Call expressions, such as `a()`:
 //     _external_: expression
 //   CallExprStart
-//   _external_: expression
-//   CallExprComma
+//     _external_: expression
+//     CallExprComma
+//   _repeated_
 // CallExpr
 //
-// Exprs and CallExprComma may repeat with CallExprComma as a
-// separator.
+// Exprs and CallExprComma may repeat with CallExprComma as a separator.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(CallExprStart, 1, CARBON_TOKEN(OpenParen))
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(CallExprComma, 0, CARBON_TOKEN(Comma))
 CARBON_PARSE_NODE_KIND_BRACKET(CallExpr, CallExprStart,
@@ -525,19 +546,22 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(IfExprElse, 3,
 
 // Struct literals, such as `{.a = 0}`:
 //   StructLiteralOrStructTypeLiteralStart
-//       _external_: Name
-//     StructFieldDesignator
-//     _external_: expression
-//   StructFieldValue
-//   StructComma
+//         _external_: Name
+//       StructFieldDesignator
+//       _external_: expression
+//     StructFieldValue
+//     StructComma
+//   _repeated_
 // StructLiteral
 //
 // Struct type literals, such as `{.a: i32}`:
-//       _external_: Name
-//     StructFieldDesignator
-//     _external_: type expression
-//   StructFieldType
-//   StructComma
+//   StructLiteralOrStructTypeLiteralStart
+//         _external_: Name
+//       StructFieldDesignator
+//       _external_: type expression
+//     StructFieldType
+//     StructComma
+//   _repeated_
 // StructTypeLiteral
 //
 // Elements (StructFieldValue and StructFieldType, respectively) and StructComma
@@ -562,9 +586,34 @@ CARBON_PARSE_NODE_KIND_BRACKET(StructTypeLiteral,
                                StructLiteralOrStructTypeLiteralStart,
                                CARBON_TOKEN(CloseCurlyBrace))
 
+// TODO: Is there a benefit to having different parse node kinds for every
+// different modifier token?
+
+// Access modifier keywords `private` or `protected`:
+// AccessModifierKeyword
+//
+// The kind of keyword is determined by looking at the token kind.
+// These may be repeated, including possibly zero times.
+CARBON_PARSE_NODE_KIND_CHILD_COUNT(AccessModifierKeyword, 0,
+                                   CARBON_TOKEN(Private)
+                                   CARBON_TOKEN(Protected))
+
+// Declaration modifier keywords such as `abstract` or `virtual`:
+// DeclModifierKeyword
+//
+// The kind of keyword is determined by looking at the token kind.
+// These may be repeated, including possibly zero times.
+CARBON_PARSE_NODE_KIND_CHILD_COUNT(DeclModifierKeyword, 0,
+                                   CARBON_TOKEN(Abstract)
+                                   CARBON_TOKEN(Base)
+                                   CARBON_TOKEN(Default)
+                                   CARBON_TOKEN(Final)
+                                   CARBON_TOKEN(Impl)
+                                   CARBON_TOKEN(Virtual))
+
 // `class`:
 //     ClassIntroducer
-//     _optional_ AbstractModifier or BaseModifier
+//     _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //     _external_: Name or QualifiedDecl
 //   ClassDefinitionStart
 //   _external_: declarations
@@ -574,8 +623,6 @@ CARBON_PARSE_NODE_KIND_BRACKET(StructTypeLiteral,
 // ClassDefinitionStart and later nodes are removed and replaced by
 // ClassDecl.
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(ClassIntroducer, 0, CARBON_TOKEN(Class))
-CARBON_PARSE_NODE_KIND_CHILD_COUNT(AbstractModifier, 0, CARBON_TOKEN(Abstract))
-CARBON_PARSE_NODE_KIND_CHILD_COUNT(BaseModifier, 0, CARBON_TOKEN(Base))
 CARBON_PARSE_NODE_KIND_BRACKET(ClassDefinitionStart, ClassIntroducer,
                                CARBON_TOKEN(OpenCurlyBrace))
 CARBON_PARSE_NODE_KIND_BRACKET(ClassDefinition, ClassDefinitionStart,
@@ -598,6 +645,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(BaseDecl, BaseIntroducer,
 
 // `interface`:
 //     InterfaceIntroducer
+//     _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //     _external_: Name or QualifiedDecl
 //   InterfaceDefinitionStart
 //   _external_: declarations
@@ -618,6 +666,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(InterfaceDecl, InterfaceIntroducer,
 
 // `constraint`:
 //     NamedConstraintIntroducer
+//     _repeated_ _external_: AccessModifierKeyword or DeclModifierKeyword
 //     _external_: Name or QualifiedDecl
 //   NamedConstraintDefinitionStart
 //   _external_: declarations

toolchain/parse/state.def

@@ -283,12 +283,26 @@ CARBON_PARSE_STATE(DeclNameAndParamsAfterImplicit)
 // Handles processing of a declaration scope. Things like fn, class, interface,
 // and so on.
 //
+// abstract
+// ^~~~~~~~
+// base
+// ^~~~
+// default
+// ^~~~~~~
+// final
+// ^~~~~
+// override
+// ^~~~~~~~
+// private
+// ^~~~~~~
+// protected
+// ^~~~~~~~~
+// virtual
+// ^~~~~~~
+//   1. DeclScopeLoop
+//
 // class ...
 // ^~~~~
-// abstract class ...
-// ^~~~~~~~~~~~~~
-// base class ...
-// ^~~~~~~~~~
 //   1. TypeAfterIntroducerAsClass
 //   2. DeclScopeLoop
 //
@@ -303,8 +317,8 @@ CARBON_PARSE_STATE(DeclNameAndParamsAfterImplicit)
 //   1. TypeAfterIntroducerAsNamedConstraint
 //   2. DeclScopeLoop
 //
-//  fn ...
-// ^
+// fn ...
+// ^~
 //   1. FunctionIntroducer
 //   2. DeclScopeLoop
 //
@@ -313,8 +327,8 @@ CARBON_PARSE_STATE(DeclNameAndParamsAfterImplicit)
 //   1. TypeAfterIntroducerAsInterface
 //   2. DeclScopeLoop
 //
-//  namespace ...
-// ^
+// namespace ...
+// ^~~~~~~~~
 //   1. Namespace
 //   2. DeclScopeLoop
 //
@@ -322,13 +336,13 @@ CARBON_PARSE_STATE(DeclNameAndParamsAfterImplicit)
 // ^
 //   1. DeclScopeLoop
 //
-//  var ...
-// ^
+// var ...
+// ^~~
 //   1. VarAsDecl
 //   2. DeclScopeLoop
 //
-//  let ...
-// ^
+// let ...
+// ^~~
 //   1. Let
 //   2. DeclScopeLoop
 //
@@ -535,7 +549,7 @@ CARBON_PARSE_STATE(ExprStatementFinish)
 // Handles a function's introducer.
 //
 // fn ...
-// ^~
+//   ^
 //   1. DeclNameAndParamsAsRequired
 //   2. FunctionAfterParams
 CARBON_PARSE_STATE(FunctionIntroducer)
@@ -606,7 +620,7 @@ CARBON_PARSE_STATE(Library)
 // Handles `namespace`.
 //
 // namespace ...
-// ^~~~~~~~~
+//          ^
 //   1. DeclNameAndParamsAsNone
 //   2. NamespaceFinish
 CARBON_PARSE_STATE(Namespace)
@@ -1008,7 +1022,7 @@ CARBON_PARSE_STATE_VARIANTS3(TypeDefinitionFinish, Class, Interface,
 // Handles processing of a type after its introducer.
 //
 // class/interface/constraint ...
-// ^~~~~~~~~~~~~~~~~~~~~~~~~~
+//                           ^
 //   1. DeclNameAndParamsAsOptional
 //   2. TypeAfterParamsAs(Class|Interface|NamedConstraint)
 CARBON_PARSE_STATE_VARIANTS3(TypeAfterIntroducer, Class, Interface, NamedConstraint)
@@ -1039,7 +1053,7 @@ CARBON_PARSE_STATE(BaseDecl)
 // Handles the start of a `var` or `returned var`.
 //
 // var ...             (variant is not Returned)
-// ^~~
+//    ^
 //   1. BindingPatternAsVariable
 //   2. VarAfterPattern
 //   3. VarFinishAs(Decl|For)
@@ -1087,7 +1101,7 @@ CARBON_PARSE_STATE_VARIANTS2(VarFinish, Decl, For)
 // Handles the start of a `let`.
 //
 // let ...
-// ^~~
+//    ^
 //   1. BindingPatternAsLet
 //   2. LetAfterPattern
 //   3. LetFinish

toolchain/parse/testdata/basics/empty_declaration.carbon → toolchain/parse/testdata/basics/empty_decl.carbon

@@ -6,7 +6,7 @@
 
 ;
 
-// CHECK:STDOUT: - filename: empty_declaration.carbon
+// CHECK:STDOUT: - filename: empty_decl.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:     {kind: 'EmptyDecl', text: ';'},

toolchain/parse/testdata/basics/fail_modifiers_before_semi.carbon

@@ -0,0 +1,20 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers_before_semi.carbon:[[@LINE+3]]:18: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: protected virtual;
+// CHECK:STDERR:                  ^
+protected virtual;
+
+// CHECK:STDOUT: - filename: fail_modifiers_before_semi.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: ';', has_error: yes},
+// CHECK:STDOUT:       {kind: 'AccessModifierKeyword', text: 'protected'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'virtual'},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 4},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]

toolchain/parse/testdata/basics/fail_no_intro_with_semi.carbon

@@ -12,6 +12,7 @@ foo;
 // CHECK:STDOUT: - filename: fail_no_intro_with_semi.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: ';', has_error: yes},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'foo', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 2},
 // CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT:   ]

toolchain/parse/testdata/basics/fail_no_intro_without_semi.carbon

@@ -12,6 +12,7 @@ foo bar baz
 // CHECK:STDOUT: - filename: fail_no_intro_without_semi.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: 'foo', has_error: yes},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'foo', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: 'baz', has_error: yes, subtree_size: 2},
 // CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT:   ]

toolchain/parse/testdata/class/base.carbon

@@ -15,7 +15,7 @@ class D {
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:         {kind: 'BaseModifier', text: 'base'},
+// CHECK:STDOUT:         {kind: 'DeclModifierKeyword', text: 'base'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'B'},
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 4},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 5},
@@ -27,7 +27,7 @@ class D {
 // CHECK:STDOUT:         {kind: 'NameExpr', text: 'B'},
 // CHECK:STDOUT:       {kind: 'BaseDecl', text: ';', subtree_size: 4},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:         {kind: 'BaseModifier', text: 'base'},
+// CHECK:STDOUT:         {kind: 'DeclModifierKeyword', text: 'base'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'Nested'},
 // CHECK:STDOUT:       {kind: 'ClassDecl', text: ';', subtree_size: 4},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 12},

toolchain/parse/testdata/class/fail_base.carbon

@@ -5,9 +5,9 @@
 // AUTOUPDATE
 
 class A {
-  // CHECK:STDERR: fail_base.carbon:[[@LINE+3]]:3: ERROR: Unrecognized declaration introducer.
+  // CHECK:STDERR: fail_base.carbon:[[@LINE+3]]:7: ERROR: `class` or `:` expected after `base`.
   // CHECK:STDERR:   base;
-  // CHECK:STDERR:   ^~~~
+  // CHECK:STDERR:       ^
   base;
 
   // CHECK:STDERR: fail_base.carbon:[[@LINE+3]]:9: ERROR: Expected expression.
@@ -29,17 +29,20 @@ class A {
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'A'},
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 3},
-// CHECK:STDOUT:       {kind: 'EmptyDecl', text: ';', has_error: yes},
+// CHECK:STDOUT:         {kind: 'InvalidParseStart', text: ';', has_error: yes},
+// CHECK:STDOUT:         {kind: 'InvalidParse', text: 'base', has_error: yes},
+// CHECK:STDOUT:       {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 3},
 // CHECK:STDOUT:         {kind: 'BaseIntroducer', text: 'base'},
 // CHECK:STDOUT:         {kind: 'BaseColon', text: ':'},
 // CHECK:STDOUT:         {kind: 'InvalidParse', text: ';', has_error: yes},
 // CHECK:STDOUT:       {kind: 'BaseDecl', text: 'base', has_error: yes, subtree_size: 4},
-// CHECK:STDOUT:       {kind: 'EmptyDecl', text: ':', has_error: yes},
+// CHECK:STDOUT:         {kind: 'InvalidParseStart', text: ':', has_error: yes},
+// CHECK:STDOUT:       {kind: 'InvalidParseSubtree', text: '}', has_error: yes, subtree_size: 2},
 // CHECK:STDOUT:         {kind: 'VariableIntroducer', text: 'var'},
 // CHECK:STDOUT:           {kind: 'Name', text: 'n'},
 // CHECK:STDOUT:           {kind: 'IntTypeLiteral', text: 'i32'},
 // CHECK:STDOUT:         {kind: 'BindingPattern', text: ':', subtree_size: 3},
 // CHECK:STDOUT:       {kind: 'VariableDecl', text: ';', subtree_size: 5},
-// CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 15},
+// CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 18},
 // CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT:   ]

toolchain/parse/testdata/class/fail_base_misplaced.carbon

@@ -17,7 +17,7 @@ fn F() {
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:         {kind: 'BaseModifier', text: 'base'},
+// CHECK:STDOUT:         {kind: 'DeclModifierKeyword', text: 'base'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'B'},
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 4},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 5},

toolchain/parse/testdata/class/fail_introducer.carbon

@@ -1,31 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-// AUTOUPDATE
-
-// TODO: Improve the diagnostic to say that the `abstract` modifier can't be
-// used on an `interface`.
-// CHECK:STDERR: fail_introducer.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
-// CHECK:STDERR: abstract interface I;
-// CHECK:STDERR: ^~~~~~~~
-abstract interface I;
-
-// CHECK:STDERR: fail_introducer.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
-// CHECK:STDERR: base fn F();
-// CHECK:STDERR: ^~~~
-base fn F();
-
-// CHECK:STDERR: fail_introducer.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
-// CHECK:STDERR: abstract base class C;
-// CHECK:STDERR: ^~~~~~~~
-abstract base class C;
-
-// CHECK:STDOUT: - filename: fail_introducer.carbon
-// CHECK:STDOUT:   parse_tree: [
-// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: ';', has_error: yes},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: ';', has_error: yes},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: ';', has_error: yes},
-// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
-// CHECK:STDOUT:   ]

toolchain/parse/testdata/class/introducer.carbon

@@ -19,11 +19,11 @@ abstract class C {}
 // CHECK:STDOUT:       {kind: 'Name', text: 'A'},
 // CHECK:STDOUT:     {kind: 'ClassDecl', text: ';', subtree_size: 3},
 // CHECK:STDOUT:       {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:       {kind: 'BaseModifier', text: 'base'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'base'},
 // CHECK:STDOUT:       {kind: 'Name', text: 'B'},
 // CHECK:STDOUT:     {kind: 'ClassDecl', text: ';', subtree_size: 4},
 // CHECK:STDOUT:       {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:       {kind: 'AbstractModifier', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
 // CHECK:STDOUT:       {kind: 'Name', text: 'C'},
 // CHECK:STDOUT:     {kind: 'ClassDecl', text: ';', subtree_size: 4},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
@@ -31,12 +31,12 @@ abstract class C {}
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 3},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 4},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:         {kind: 'BaseModifier', text: 'base'},
+// CHECK:STDOUT:         {kind: 'DeclModifierKeyword', text: 'base'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'B'},
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 4},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 5},
 // CHECK:STDOUT:         {kind: 'ClassIntroducer', text: 'class'},
-// CHECK:STDOUT:         {kind: 'AbstractModifier', text: 'abstract'},
+// CHECK:STDOUT:         {kind: 'DeclModifierKeyword', text: 'abstract'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'C'},
 // CHECK:STDOUT:       {kind: 'ClassDefinitionStart', text: '{', subtree_size: 4},
 // CHECK:STDOUT:     {kind: 'ClassDefinition', text: '}', subtree_size: 5},

toolchain/parse/testdata/class/mismatched_introducer.carbon

@@ -0,0 +1,31 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// Note: invalid modifiers are diagnosed in check not parse.
+abstract interface I;
+base fn F();
+abstract base class C;
+
+// CHECK:STDOUT: - filename: mismatched_introducer.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:       {kind: 'InterfaceIntroducer', text: 'interface'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'I'},
+// CHECK:STDOUT:     {kind: 'InterfaceDecl', text: ';', subtree_size: 4},
+// CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'base'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'F'},
+// CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'ParamList', text: ')', subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FunctionDecl', text: ';', subtree_size: 6},
+// CHECK:STDOUT:       {kind: 'ClassIntroducer', text: 'class'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'base'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'C'},
+// CHECK:STDOUT:     {kind: 'ClassDecl', text: ';', subtree_size: 5},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]

toolchain/parse/testdata/function/declaration/fail_impl.carbon

@@ -0,0 +1,24 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_impl.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: impl foo bar;
+// CHECK:STDERR: ^~~~
+impl foo bar;
+// CHECK:STDERR: fail_impl.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: impl
+// CHECK:STDERR: ^~~~
+impl
+
+// CHECK:STDOUT: - filename: fail_impl.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'impl', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 2},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'impl', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: 'impl', has_error: yes, subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]

toolchain/parse/testdata/function/declaration/fail_skip_without_semi_to_curly.carbon

@@ -13,7 +13,8 @@ fn F();
 // CHECK:STDOUT: - filename: fail_skip_without_semi_to_curly.carbon
 // CHECK:STDOUT:   parse_tree: [
 // CHECK:STDOUT:     {kind: 'FileStart', text: ''},
-// CHECK:STDOUT:     {kind: 'EmptyDecl', text: 'struct', has_error: yes},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'struct', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: '}', has_error: yes, subtree_size: 2},
 // CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
 // CHECK:STDOUT:       {kind: 'Name', text: 'F'},
 // CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},

toolchain/parse/testdata/function/declaration/impl_fn.carbon

@@ -0,0 +1,44 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+impl fn F();
+abstract impl fn G();
+impl abstract fn H();
+private impl default fn I();
+
+// CHECK:STDOUT: - filename: impl_fn.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'impl'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'F'},
+// CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'ParamList', text: ')', subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FunctionDecl', text: ';', subtree_size: 6},
+// CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'impl'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'G'},
+// CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'ParamList', text: ')', subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FunctionDecl', text: ';', subtree_size: 7},
+// CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'impl'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'H'},
+// CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'ParamList', text: ')', subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FunctionDecl', text: ';', subtree_size: 7},
+// CHECK:STDOUT:       {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:       {kind: 'AccessModifierKeyword', text: 'private'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'impl'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'default'},
+// CHECK:STDOUT:       {kind: 'Name', text: 'I'},
+// CHECK:STDOUT:         {kind: 'ParamListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'ParamList', text: ')', subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FunctionDecl', text: ';', subtree_size: 8},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]

toolchain/parse/testdata/namespace/fail_modifiers.carbon

@@ -0,0 +1,30 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:37: ERROR: `namespace` unexpected after modifiers.
+// CHECK:STDERR: private abstract base default final namespace Foo;
+// CHECK:STDERR:                                     ^~~~~~~~~
+private abstract base default final namespace Foo;
+
+// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:1: ERROR: Unrecognized declaration introducer.
+// CHECK:STDERR: impl namespace
+// CHECK:STDERR: ^~~~
+impl namespace
+
+// CHECK:STDOUT: - filename: fail_modifiers.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'namespace', has_error: yes},
+// CHECK:STDOUT:       {kind: 'AccessModifierKeyword', text: 'private'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'abstract'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'base'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'default'},
+// CHECK:STDOUT:       {kind: 'DeclModifierKeyword', text: 'final'},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 7},
+// CHECK:STDOUT:       {kind: 'InvalidParseStart', text: 'impl', has_error: yes},
+// CHECK:STDOUT:     {kind: 'InvalidParseSubtree', text: 'namespace', has_error: yes, subtree_size: 2},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]

toolchain/parse/tree.cpp

@@ -222,6 +222,11 @@ auto Tree::Verify() const -> ErrorOr<Success> {
           n.index));
     }
 
+    if (n_impl.kind == NodeKind::Placeholder) {
+      return Error(llvm::formatv(
+          "Node #{0} is a placeholder node that wasn't replaced.", n.index));
+    }
+
     int subtree_size = 1;
     if (n_impl.kind.has_bracket()) {
       while (true) {