Fix _Ptr source location bugs and remove workarounds from 3C.

clang/include/clang/Sema/DeclSpec.h

@@ -410,6 +410,8 @@ class DeclSpec {
   /// otherwise, it is the same as TSTLoc. Hence, the pair TSTLoc and
   /// TSTNameLoc provides source range info for tag types.
   SourceLocation TSTNameLoc;
+  // Corresponds to PointerTypeLoc.
+  SourceLocation CheckedPtrKWLoc, CheckedPtrLeftSymLoc, CheckedPtrRightSymLoc;
   SourceRange TypeofParensRange;
   SourceLocation TQ_constLoc, TQ_restrictLoc, TQ_volatileLoc, TQ_atomicLoc,
       TQ_unalignedLoc;
@@ -562,6 +564,13 @@ class DeclSpec {
     assert(isDeclRep((TST) TypeSpecType) || TypeSpecType == TST_typename);
     return TSTNameLoc;
   }
+  SourceLocation getCheckedPtrKWLoc() const { return CheckedPtrKWLoc; }
+  SourceLocation getCheckedPtrLeftSymLoc() const {
+    return CheckedPtrLeftSymLoc;
+  }
+  SourceLocation getCheckedPtrRightSymLoc() const {
+    return CheckedPtrRightSymLoc;
+  }
 
   SourceRange getTypeofParensRange() const { return TypeofParensRange; }
   void setTypeofParensRange(SourceRange range) { TypeofParensRange = range; }
@@ -843,6 +852,9 @@ class DeclSpec {
   bool SetConstexprSpec(ConstexprSpecKind ConstexprKind, SourceLocation Loc,
                         const char *&PrevSpec, unsigned &DiagID);
 
+  void setSpecCheckedPtr(SourceLocation KWLoc, SourceLocation LeftSymLoc,
+                         SourceLocation RightSymLoc);
+
   bool isFriendSpecified() const { return Friend_specified; }
   SourceLocation getFriendSpecLoc() const { return FriendLoc; }
 

clang/lib/3C/ConstraintVariables.cpp

@@ -2267,7 +2267,7 @@ FVComponentVariable::FVComponentVariable(const QualType &QT,
       const SourceManager &SM = C.getSourceManager();
       SourceLocation DLoc = D->getLocation();
       CharSourceRange CSR;
-      if (SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)) {
+      if (false /*SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)*/) {
         // It's not clear to me why, but the end of the SourceRange for the
         // declaration can come before the SourceLocation for the declaration.
         // This can result in SourceDeclaration failing to capture the whole

clang/lib/3C/Utils.cpp

@@ -622,6 +622,7 @@ SourceRange getDeclSourceRangeWithAnnotations(
     return SR;
   SourceLocation DeclEnd = SR.getEnd();
 
+#if 0
   // Partial workaround for a compiler bug where if D has certain checked
   // pointer types such as `_Ptr<int *(void)>` (seen in the partial_checked.c
   // regression test), D->getSourceRange() returns only the _Ptr token (TODO:
@@ -631,6 +632,7 @@ SourceRange getDeclSourceRangeWithAnnotations(
   SourceLocation DeclLoc = D->getLocation();
   if (SM.isBeforeInTranslationUnit(DeclEnd, DeclLoc))
     DeclEnd = DeclLoc;
+#endif
 
   SourceLocation AnnotationsEnd = getCheckedCAnnotationsEnd(D);
   if (AnnotationsEnd.isValid() &&

clang/lib/AST/Decl.cpp

@@ -1900,9 +1900,14 @@ static bool typeIsPostfix(QualType QT) {
     switch (T->getTypeClass()) {
     default:
       return false;
-    case Type::Pointer:
-      QT = cast<PointerType>(T)->getPointeeType();
+    case Type::Pointer: {
+      const PointerType *PT = cast<PointerType>(T);
+      if (PT->isChecked())
+        // See the comment about checked pointers in TypeLoc::getBeginLoc.
+        return false;
+      QT = PT->getPointeeType();
       break;
+    }
     case Type::BlockPointer:
       QT = cast<BlockPointerType>(T)->getPointeeType();
       break;

clang/lib/AST/TypeLoc.cpp

@@ -212,6 +212,19 @@ SourceLocation TypeLoc::getBeginLoc() const {
     case Qualified:
       Cur = Cur.getNextTypeLoc();
       continue;
+    case Pointer:
+      if (Cur.castAs<PointerTypeLoc>().getTypePtr()->isChecked()) {
+        // Unlike the usual "inside-out" C declarator syntax, checked pointer
+        // types (_Ptr<T>, etc.) are "right-side-out". Since T is syntactically
+        // contained in _Ptr<T>, it can't affect the source range of the whole
+        // type. Thus, the source range code can ignore T and treat _Ptr<T> the
+        // same way as a single-token type such as a primitive or typedef. Here,
+        // a single-token type would hit the `default` case with
+        // Cur.getNextTypeLoc().isNull() returning true.
+        LeftMost = Cur;
+        break;
+      }
+      LLVM_FALLTHROUGH;
     default:
       if (Cur.getLocalSourceRange().getBegin().isValid())
         LeftMost = Cur;
@@ -249,6 +262,13 @@ SourceLocation TypeLoc::getEndLoc() const {
         Last = Cur;
       break;
     case Pointer:
+      if (Cur.castAs<PointerTypeLoc>().getTypePtr()->isChecked()) {
+        // See the comment about checked pointers in TypeLoc::getBeginLoc.
+        if (!Last)
+          Last = Cur;
+        return Last.getLocalSourceRange().getEnd();
+      }
+      LLVM_FALLTHROUGH;
     case BlockPointer:
     case MemberPointer:
     case LValueReference:

clang/lib/Parse/ParseDecl.cpp

@@ -7837,6 +7837,7 @@ void Parser::ParseCheckedPointerSpecifiers(DeclSpec &DS) {
            "Not a checked pointer specifier");
     tok::TokenKind Kind = Tok.getKind();
     SourceLocation StartLoc = ConsumeToken();
+    SourceLocation LeftLoc = Tok.getLocation();
     if (ExpectAndConsume(tok::less)) {
         return;
     }
@@ -7884,6 +7885,7 @@ void Parser::ParseCheckedPointerSpecifiers(DeclSpec &DS) {
         DiagID, Result.get(),
         Actions.getASTContext().getPrintingPolicy()))
         Diag(StartLoc, DiagID) << PrevSpec;
+    DS.setSpecCheckedPtr(StartLoc, LeftLoc, EndLoc);
 }
 
 /// [Checked C] Parse a specifier for an existential type.

clang/lib/Sema/DeclSpec.cpp

@@ -1184,6 +1184,14 @@ bool DeclSpec::SetConstexprSpec(ConstexprSpecKind ConstexprKind,
   return false;
 }
 
+void DeclSpec::setSpecCheckedPtr(SourceLocation KWLoc,
+                                 SourceLocation LeftSymLoc,
+                                 SourceLocation RightSymLoc) {
+  this->CheckedPtrKWLoc = KWLoc;
+  this->CheckedPtrLeftSymLoc = LeftSymLoc;
+  this->CheckedPtrRightSymLoc = RightSymLoc;
+}
+
 void DeclSpec::SaveWrittenBuiltinSpecs() {
   writtenBS.Sign = static_cast<int>(getTypeSpecSign());
   writtenBS.Width = static_cast<int>(getTypeSpecWidth());

clang/lib/Sema/SemaType.cpp

@@ -6289,6 +6289,16 @@ namespace {
       TL.setNameLoc(DS.getTypeSpecTypeLoc());
     }
 
+    void VisitPointerTypeLoc(PointerTypeLoc TL) {
+      // REVIEW: Should we have any error checks here?
+      TL.setKWLoc(DS.getCheckedPtrKWLoc());
+      TL.setLeftSymLoc(DS.getCheckedPtrLeftSymLoc());
+      TL.setRightSymLoc(DS.getCheckedPtrRightSymLoc());
+      TypeSourceInfo *TInfo = nullptr;
+      Sema::GetTypeFromParser(DS.getRepAsType(), &TInfo);
+      TL.getNextTypeLoc().initializeFullCopy(TInfo->getTypeLoc());
+    }
+
     void VisitTypeLoc(TypeLoc TL) {
       // FIXME: add other typespec types and change this to an assert.
       TL.initialize(Context, DS.getTypeSpecTypeLoc());

clang/test/3C/partial_checked.c

@@ -76,6 +76,13 @@ void test4() {
 // getDeclSourceRangeWithAnnotations for more information.
 _Ptr<int *(void)> gm;
 // CHECK: _Ptr<_Ptr<int> (void)> gm = ((void *)0);
+_Ptr<int *(void)> gma[10];
+// CHECK_NOALL: _Ptr<_Ptr<int> (void)> gma[10] = {((void *)0)};
+// CHECK_ALL:   _Ptr<_Ptr<int> (void)> gma _Checked[10] = {((void *)0)};
+
+// TODO: Better names and CHECK comments
+void gmf(_Ptr<int (void)> a, int *b) {}
+void gmaf(_Ptr<int (void)> a[10], int *b) {}
 
 void test5(_Ptr<int *> a, _Ptr<int *> b, _Ptr<_Ptr<int>> c, int **d) {
   // CHECK: void test5(_Ptr<_Ptr<int>> a, _Ptr<int *> b : itype(_Ptr<_Ptr<int>>), _Ptr<_Ptr<int>> c, _Ptr<_Ptr<int>> d) {

clang/unittests/AST/SourceLocationTest.cpp

@@ -293,6 +293,80 @@ TEST(TypeLoc, LongLongConstRange) {
   EXPECT_TRUE(Verifier.match("long long const a = 0;", typeLoc()));
 }
 
+// For multi-level types such as pointers, if we just used typeLoc() as above,
+// it would match the TypeLocs of all the levels and the RangeVerifier would
+// verify an arbitrary one of them. Instead, we want to verify a specific level.
+internal::BindableMatcher<TypeLoc> typeLocAtDepth(unsigned int Depth) {
+  internal::BindableMatcher<TypeLoc> Matcher = typeLoc(hasParent(varDecl()));
+  for (unsigned int I = 0; I < Depth; I++)
+    Matcher = typeLoc(hasParent(Matcher));
+  return Matcher;
+}
+
+// Test the source range of a checked pointer type and all levels of its
+// pointee.
+TEST(CheckedPtr, TypeLoc) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 1, 1, 11);
+  EXPECT_TRUE(Verifier.match("_Ptr<int *> p;", typeLocAtDepth(0), Lang_C99));
+}
+TEST(CheckedPtr, TypeLocPointee) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 6, 1, 10);
+  EXPECT_TRUE(Verifier.match("_Ptr<int *> p;", typeLocAtDepth(1), Lang_C99));
+}
+TEST(CheckedPtr, TypeLocPointee2) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 6, 1, 6);
+  EXPECT_TRUE(Verifier.match("_Ptr<int *> p;", typeLocAtDepth(2), Lang_C99));
+}
+
+// Tests of checked pointers in combination with arrays (as an example of a type
+// layer that uses the usual inside-out C declarator syntax). Also, some
+// corresponding tests of unchecked pointers to verify that the special handling
+// of checked pointers doesn't unintentionally affect unchecked pointers.
+//
+// For each example declaration, we test the source ranges of both the TypeLoc
+// and the VarDecl. The VarDecl uses DeclaratorDecl::getSourceRange, which uses
+// typeIsPostfix to decide whether the type extends past the name in order to
+// choose either the name or the end of the type as the end of the source range.
+// typeIsPostfix has logic roughly parallel to TypeLoc::getEndLoc. (Could the
+// code be refactored to remove this duplication?)
+
+TEST(CheckedPtr, ArrayTypeLoc) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 1, 1, 13);
+  EXPECT_TRUE(Verifier.match("_Ptr<int[10]> p;", typeLocAtDepth(0), Lang_C99));
+}
+TEST(CheckedPtr, ArrayDecl) {
+  RangeVerifier<VarDecl> Verifier;
+  Verifier.expectRange(1, 1, 1, 15);
+  EXPECT_TRUE(Verifier.match("_Ptr<int[10]> p;", varDecl(), Lang_C99));
+}
+TEST(CheckedPtr, UncheckedArrayTypeLoc) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 1, 1, 12);
+  EXPECT_TRUE(Verifier.match("int (*p)[10];", typeLocAtDepth(0), Lang_C99));
+}
+TEST(CheckedPtr, UncheckedArrayDecl) {
+  RangeVerifier<VarDecl> Verifier;
+  Verifier.expectRange(1, 1, 1, 12);
+  EXPECT_TRUE(Verifier.match("int (*p)[10];", varDecl(), Lang_C99));
+}
+
+// Test with array levels both inside and outside a checked pointer level.
+TEST(CheckedPtr, SandwichArrayTypeLoc) {
+  RangeVerifier<TypeLoc> Verifier;
+  Verifier.expectRange(1, 1, 1, 18);
+  EXPECT_TRUE(
+      Verifier.match("_Ptr<int[10]> p[1];", typeLocAtDepth(0), Lang_C99));
+}
+TEST(CheckedPtr, SandwichArrayDecl) {
+  RangeVerifier<VarDecl> Verifier;
+  Verifier.expectRange(1, 1, 1, 18);
+  EXPECT_TRUE(Verifier.match("_Ptr<int[10]> p[1];", varDecl(), Lang_C99));
+}
+
 TEST(CXXConstructorDecl, NoRetFunTypeLocRange) {
   RangeVerifier<CXXConstructorDecl> Verifier;
   Verifier.expectRange(1, 11, 1, 13);