浏览代码

Add tests and range enforcement for current LocId use-cases (#5274)

This is trying to document the status quo. Note
https://github.com/carbon-language/carbon-lang/pull/4497 placed
restrictions on a bit, and this is in part flowing back to restrictions
on both NodeId and ImportIRInstId limits.
Jon Ross-Perkins 1 年之前
父节点
当前提交
a527626d87
共有 5 个文件被更改,包括 99 次插入7 次删除
  1. 2 1
      toolchain/lex/token_index.h
  2. 11 1
      toolchain/parse/node_ids.h
  3. 10 0
      toolchain/sem_ir/BUILD
  4. 13 5
      toolchain/sem_ir/ids.h
  5. 63 0
      toolchain/sem_ir/ids_test.cpp

+ 2 - 1
toolchain/lex/token_index.h

@@ -27,13 +27,14 @@ struct TokenIndex : public IndexBase<TokenIndex> {
   // The number of bits which must be allotted for `TokenIndex`.
   static constexpr int Bits = 23;
   // The maximum number of tokens that can be stored, including the FileStart
-  // and FileEnd tokens.
+  // and FileEnd tokens. Exceeding this is diagnosed by `TooManyTokens`.
   static constexpr int Max = 1 << Bits;
 
   static constexpr llvm::StringLiteral Label = "token";
   static const TokenIndex None;
   // Comments aren't tokenized, so this is the first token after FileStart.
   static const TokenIndex FirstNonCommentToken;
+
   using IndexBase::IndexBase;
 };
 

+ 11 - 1
toolchain/parse/node_ids.h

@@ -22,10 +22,20 @@ struct NoneNodeId {};
 struct NodeId : public IdBase<NodeId> {
   static constexpr llvm::StringLiteral Label = "node";
 
+  // We give NodeId a bit in addition to TokenIndex in order to account for
+  // virtual nodes, where a token may produce two nodes.
+  static constexpr int32_t Bits = Lex::TokenIndex::Bits + 1;
+
+  // The maximum ID, non-inclusive.
+  static constexpr int Max = 1 << Bits;
+
   // A node ID with no value.
   static constexpr NoneNodeId None;
 
-  using IdBase::IdBase;
+  constexpr explicit NodeId(int32_t index) : IdBase(index) {
+    CARBON_DCHECK(index < Max, "Index out of range: {0}", index);
+  }
+
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr NodeId(NoneNodeId /*none*/) : IdBase(NoneIndex) {}
 };

+ 10 - 0
toolchain/sem_ir/BUILD

@@ -45,6 +45,16 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "ids_test",
+    srcs = ["ids_test.cpp"],
+    deps = [
+        ":typed_insts",
+        "//testing/base:gtest_main",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "inst",
     srcs = ["inst.cpp"],

+ 13 - 5
toolchain/sem_ir/ids.h

@@ -787,7 +787,15 @@ struct ImportIRInstId : public IdBase<ImportIRInstId> {
   static constexpr llvm::StringLiteral Label = "import_ir_inst";
   using ValueType = ImportIRInst;
 
-  using IdBase::IdBase;
+  // ImportIRInstId is restricted so that it can fit into LocId.
+  static constexpr int32_t Bits = 29;
+
+  // The maximum ID, non-inclusive.
+  static constexpr int Max = 1 << Bits;
+
+  constexpr explicit ImportIRInstId(int32_t index) : IdBase(index) {
+    CARBON_DCHECK(index < Max, "Index out of range: {0}", index);
+  }
 };
 
 // A SemIR location used as the location of instructions.
@@ -810,15 +818,15 @@ struct LocId : public IdBase<LocId> {
 
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr LocId(Parse::NodeId node_id) : IdBase(node_id.index) {
-    CARBON_CHECK(node_id.has_value() == has_value());
-    CARBON_CHECK(!is_implicit());
+    CARBON_CHECK(node_id.has_value() == has_value(), "{0}", index);
+    CARBON_CHECK(!is_implicit(), "{0}", index);
   }
 
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr LocId(ImportIRInstId inst_id)
       : IdBase(NoneIndex + ImportIRInstId::NoneIndex - inst_id.index) {
-    CARBON_CHECK(inst_id.has_value() == has_value());
-    CARBON_CHECK(index & ImplicitBit);
+    CARBON_CHECK(inst_id.has_value() == has_value(), "{0}", index);
+    CARBON_CHECK(index & ImplicitBit, "{0}", index);
   }
 
   // Forms an equivalent LocId for an implicit location.

+ 63 - 0
toolchain/sem_ir/ids_test.cpp

@@ -0,0 +1,63 @@
+// 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/sem_ir/ids.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <limits>
+
+namespace Carbon::SemIR {
+namespace {
+
+using ::testing::Eq;
+
+TEST(IdsTest, LocIdIsNone) {
+  LocId loc_id = Parse::NodeId::None;
+  EXPECT_FALSE(loc_id.has_value());
+  EXPECT_FALSE(loc_id.is_node_id());
+  EXPECT_FALSE(loc_id.is_import_ir_inst_id());
+  EXPECT_FALSE(loc_id.is_implicit());
+  EXPECT_THAT(loc_id.node_id(),
+              // The actual type is NoneNodeId, so cast to NodeId.
+              Eq<Parse::NodeId>(Parse::NodeId::None));
+  EXPECT_THAT(loc_id.import_ir_inst_id(), Eq(ImportIRInstId::None));
+}
+
+TEST(IdsTest, LocIdIsNodeId) {
+  for (auto index : {0, 1, Parse::NodeId::Max - 2, Parse::NodeId::Max - 1}) {
+    SCOPED_TRACE(llvm::formatv("Index: {0}", index));
+    Parse::NodeId node_id(index);
+    LocId loc_id = node_id;
+    EXPECT_TRUE(loc_id.has_value());
+    EXPECT_TRUE(loc_id.is_node_id());
+    EXPECT_FALSE(loc_id.is_import_ir_inst_id());
+    EXPECT_FALSE(loc_id.is_implicit());
+    EXPECT_THAT(loc_id.node_id(), node_id);
+
+    loc_id = loc_id.ToImplicit();
+    EXPECT_TRUE(loc_id.has_value());
+    EXPECT_TRUE(loc_id.is_node_id());
+    EXPECT_FALSE(loc_id.is_import_ir_inst_id());
+    EXPECT_TRUE(loc_id.is_implicit());
+    EXPECT_THAT(loc_id.node_id(), node_id);
+  }
+}
+
+TEST(IdsTest, LocIdIsImportIRInstId) {
+  for (auto index : {0, 1, ImportIRInstId::Max - 2, ImportIRInstId::Max - 1}) {
+    SCOPED_TRACE(llvm::formatv("Index: {0}", index));
+    ImportIRInstId import_ir_inst_id(index);
+    LocId loc_id = import_ir_inst_id;
+    EXPECT_TRUE(loc_id.has_value());
+    EXPECT_FALSE(loc_id.is_node_id());
+    EXPECT_TRUE(loc_id.is_import_ir_inst_id());
+    EXPECT_FALSE(loc_id.is_implicit());
+    EXPECT_THAT(loc_id.import_ir_inst_id(), import_ir_inst_id);
+  }
+}
+
+}  // namespace
+}  // namespace Carbon::SemIR