Switch Ptr to a C++ pointer using the nonnull attribute (#831)

The advantage is a C++ pointer is special, and this approach eliminates the Ptr class type that was causing problems in conversions. Attribute suggestion was courtesy of chandlerc. We're sticking with the Ptr name because it's shorter than Nonnull, and we're likely to keep this in lots of places.

executable_semantics/ast/expression_test.cpp

@@ -39,7 +39,7 @@ TEST_F(ExpressionTest, EmptyAsExpression) {
   ParenContents<Expression> contents = {.elements = {},
                                         .has_trailing_comma = false};
   Ptr<const Expression> expression =
-      ExpressionFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(expression->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*expression).Fields(), IsEmpty());
@@ -48,8 +48,8 @@ TEST_F(ExpressionTest, EmptyAsExpression) {
 TEST_F(ExpressionTest, EmptyAsTuple) {
   ParenContents<Expression> contents = {.elements = {},
                                         .has_trailing_comma = false};
-  Ptr<const Expression> tuple = TupleExpressionFromParenContents(
-      PtrTo(arena), FakeSourceLoc(1), contents);
+  Ptr<const Expression> tuple =
+      TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*tuple).Fields(), IsEmpty());
@@ -68,7 +68,7 @@ TEST_F(ExpressionTest, UnaryNoCommaAsExpression) {
       .has_trailing_comma = false};
 
   Ptr<const Expression> expression =
-      ExpressionFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->SourceLoc(), FakeSourceLoc(2));
   ASSERT_EQ(expression->Tag(), Expression::Kind::IntLiteral);
 }
@@ -79,8 +79,8 @@ TEST_F(ExpressionTest, UnaryNoCommaAsTuple) {
                     .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
       .has_trailing_comma = false};
 
-  Ptr<const Expression> tuple = TupleExpressionFromParenContents(
-      PtrTo(arena), FakeSourceLoc(1), contents);
+  Ptr<const Expression> tuple =
+      TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*tuple).Fields(),
@@ -94,7 +94,7 @@ TEST_F(ExpressionTest, UnaryWithCommaAsExpression) {
       .has_trailing_comma = true};
 
   Ptr<const Expression> expression =
-      ExpressionFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(expression->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*expression).Fields(),
@@ -107,8 +107,8 @@ TEST_F(ExpressionTest, UnaryWithCommaAsTuple) {
                     .term = arena.New<IntLiteral>(FakeSourceLoc(2), 42)}},
       .has_trailing_comma = true};
 
-  Ptr<const Expression> tuple = TupleExpressionFromParenContents(
-      PtrTo(arena), FakeSourceLoc(1), contents);
+  Ptr<const Expression> tuple =
+      TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*tuple).Fields(),
@@ -124,7 +124,7 @@ TEST_F(ExpressionTest, BinaryAsExpression) {
       .has_trailing_comma = true};
 
   Ptr<const Expression> expression =
-      ExpressionFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      ExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(expression->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(expression->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*expression).Fields(),
@@ -139,8 +139,8 @@ TEST_F(ExpressionTest, BinaryAsTuple) {
                     .term = arena.New<IntLiteral>(FakeSourceLoc(3), 42)}},
       .has_trailing_comma = true};
 
-  Ptr<const Expression> tuple = TupleExpressionFromParenContents(
-      PtrTo(arena), FakeSourceLoc(1), contents);
+  Ptr<const Expression> tuple =
+      TupleExpressionFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   ASSERT_EQ(tuple->Tag(), Expression::Kind::TupleLiteral);
   EXPECT_THAT(cast<TupleLiteral>(*tuple).Fields(),

executable_semantics/ast/pattern_test.cpp

@@ -38,7 +38,7 @@ TEST_F(PatternTest, EmptyAsPattern) {
   ParenContents<Pattern> contents = {.elements = {},
                                      .has_trailing_comma = false};
   Ptr<const Pattern> pattern =
-      PatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->SourceLoc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
   EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(), IsEmpty());
@@ -48,7 +48,7 @@ TEST_F(PatternTest, EmptyAsTuplePattern) {
   ParenContents<Pattern> contents = {.elements = {},
                                      .has_trailing_comma = false};
   Ptr<const TuplePattern> tuple =
-      TuplePatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   EXPECT_THAT(tuple->Fields(), IsEmpty());
 }
@@ -66,7 +66,7 @@ TEST_F(PatternTest, UnaryNoCommaAsPattern) {
       .has_trailing_comma = false};
 
   Ptr<const Pattern> pattern =
-      PatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->SourceLoc(), FakeSourceLoc(2));
   ASSERT_TRUE(isa<AutoPattern>(*pattern));
 }
@@ -78,7 +78,7 @@ TEST_F(PatternTest, UnaryNoCommaAsTuplePattern) {
       .has_trailing_comma = false};
 
   Ptr<const TuplePattern> tuple =
-      TuplePatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   EXPECT_THAT(tuple->Fields(), ElementsAre(AutoFieldNamed("0")));
 }
@@ -90,7 +90,7 @@ TEST_F(PatternTest, UnaryWithCommaAsPattern) {
       .has_trailing_comma = true};
 
   Ptr<const Pattern> pattern =
-      PatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->SourceLoc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
   EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(),
@@ -104,7 +104,7 @@ TEST_F(PatternTest, UnaryWithCommaAsTuplePattern) {
       .has_trailing_comma = true};
 
   Ptr<const TuplePattern> tuple =
-      TuplePatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   EXPECT_THAT(tuple->Fields(), ElementsAre(AutoFieldNamed("0")));
 }
@@ -118,7 +118,7 @@ TEST_F(PatternTest, BinaryAsPattern) {
       .has_trailing_comma = true};
 
   Ptr<const Pattern> pattern =
-      PatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      PatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(pattern->SourceLoc(), FakeSourceLoc(1));
   ASSERT_TRUE(isa<TuplePattern>(*pattern));
   EXPECT_THAT(cast<TuplePattern>(*pattern).Fields(),
@@ -134,7 +134,7 @@ TEST_F(PatternTest, BinaryAsTuplePattern) {
       .has_trailing_comma = true};
 
   Ptr<const TuplePattern> tuple =
-      TuplePatternFromParenContents(PtrTo(arena), FakeSourceLoc(1), contents);
+      TuplePatternFromParenContents(&arena, FakeSourceLoc(1), contents);
   EXPECT_EQ(tuple->SourceLoc(), FakeSourceLoc(1));
   EXPECT_THAT(tuple->Fields(),
               ElementsAre(AutoFieldNamed("0"), AutoFieldNamed("1")));

executable_semantics/common/ptr.h

@@ -5,44 +5,11 @@
 #ifndef EXECUTABLE_SEMANTICS_COMMON_PTR_H_
 #define EXECUTABLE_SEMANTICS_COMMON_PTR_H_
 
-#include <memory>
-#include <vector>
-
-#include "common/check.h"
-
 namespace Carbon {
 
 // A non-nullable pointer. Written as `Ptr<T>` instead of `T*`.
 template <typename T>
-class Ptr {
- public:
-  explicit Ptr(T* ptr) : ptr(ptr) { CHECK(ptr != nullptr); }
-
-  template <typename OtherT,
-            std::enable_if_t<std::is_convertible_v<OtherT*, T*>>* = nullptr>
-  Ptr(Ptr<OtherT> other) : ptr(other.Get()) {}
-
-  Ptr(std::nullptr_t) = delete;
-
-  Ptr(const Ptr& other) = default;
-  Ptr& operator=(const Ptr& rhs) = default;
-
-  auto operator*() const -> T& { return *ptr; }
-  auto operator->() const -> T* { return ptr; }
-
-  T* Get() const { return ptr; }
-
-  friend auto operator==(Ptr lhs, Ptr rhs) { return lhs.ptr == rhs.ptr; }
-  friend auto operator!=(Ptr lhs, Ptr rhs) { return lhs.ptr != rhs.ptr; }
-
- private:
-  T* ptr;
-};
-
-template <typename T>
-auto PtrTo(T& obj) -> Ptr<T> {
-  return Ptr<T>(&obj);
-}
+using Ptr = T* _Nonnull __attribute__((nonnull));
 
 }  // namespace Carbon
 

executable_semantics/interpreter/interpreter.cpp

@@ -465,7 +465,7 @@ auto Interpreter::StepExp() -> Transition {
       } else {
         //    { { v :: [][i] :: C, E, F} :: S, H}
         // -> { { v_i :: C, E, F} : S, H}
-        auto* tuple = dyn_cast<TupleValue>(act->Results()[0].Get());
+        auto* tuple = dyn_cast<TupleValue>(act->Results()[0]);
         if (tuple == nullptr) {
           FATAL_RUNTIME_ERROR_NO_LINE()
               << "expected a tuple in field access, not " << *act->Results()[0];
@@ -578,7 +578,7 @@ auto Interpreter::StepExp() -> Transition {
                 // TODO: Think about a cleaner way to cast between Ptr types.
                 // (multiple TODOs)
                 .function = Ptr<const FunctionValue>(
-                    cast<FunctionValue>(act->Results()[0].Get())),
+                    cast<FunctionValue>(act->Results()[0])),
                 .args = act->Results()[1],
                 .loc = exp->SourceLoc()};
           default:

executable_semantics/interpreter/type_checker.cpp

@@ -601,7 +601,7 @@ auto TypeChecker::TypeCheckPattern(Ptr<const Pattern> p, TypeEnv types,
       // TODO: Think about a cleaner way to cast between Ptr types.
       // (multiple TODOs)
       auto arguments = Ptr<const TuplePattern>(
-          cast<const TuplePattern>(arg_results.pattern.Get()));
+          cast<const TuplePattern>(arg_results.pattern));
       return {.pattern = arena->New<AlternativePattern>(
                   alternative.SourceLoc(),
                   ReifyType(choice_type, alternative.SourceLoc()),
@@ -922,8 +922,7 @@ auto TypeChecker::TypeOfClassDef(const ClassDefinition* sd, TypeEnv /*types*/,
           FATAL_COMPILATION_ERROR(binding->SourceLoc())
               << "Struct members must have names";
         }
-        const auto* binding_type =
-            dyn_cast<ExpressionPattern>(binding->Type().Get());
+        const auto* binding_type = dyn_cast<ExpressionPattern>(binding->Type());
         if (binding_type == nullptr) {
           FATAL_COMPILATION_ERROR(binding->SourceLoc())
               << "Struct members must have explicit types";
@@ -993,7 +992,7 @@ auto TypeChecker::MakeTypeChecked(const Ptr<const Declaration> d,
       TCExpression type_checked_initializer =
           TypeCheckExp(var.Initializer(), types, values);
       const auto* binding_type =
-          dyn_cast<ExpressionPattern>(var.Binding()->Type().Get());
+          dyn_cast<ExpressionPattern>(var.Binding()->Type());
       if (binding_type == nullptr) {
         // TODO: consider adding support for `auto`
         FATAL_COMPILATION_ERROR(var.SourceLoc())

executable_semantics/main.cpp

@@ -32,7 +32,7 @@ int main(int argc, char* argv[]) {
 
   Carbon::Arena arena;
   std::variant<Carbon::AST, Carbon::SyntaxErrorCode> ast_or_error =
-      Carbon::Parse(PtrTo(arena), input_file_name);
+      Carbon::Parse(&arena, input_file_name);
 
   if (auto* error = std::get_if<Carbon::SyntaxErrorCode>(&ast_or_error)) {
     // Diagnostic already reported to std::cerr; this is just a return code.
@@ -40,5 +40,5 @@ int main(int argc, char* argv[]) {
   }
 
   // Typecheck and run the parsed program.
-  Carbon::ExecProgram(PtrTo(arena), std::get<Carbon::AST>(ast_or_error));
+  Carbon::ExecProgram(&arena, std::get<Carbon::AST>(ast_or_error));
 }