fix up tests, get templates working better

Author: EricWF

clang/include/clang/AST/Decl.h

@@ -2745,10 +2745,6 @@ class FunctionDecl : public DeclaratorDecl,
   /// parameter packs are not treated specially here.
   bool hasOneParamOrDefaultArgs() const;
 
-
-  bool hasResultNameIntroducer();
-  ResultNameDecl *getCanonicalResultNameIntroducer();
-
   /// Find the source location information for how the type of this function
   /// was written. May be absent (for example if the function was declared via
   /// a typedef) and may contain a different type from that of the function

clang/include/clang/AST/DeclCXX.h

@@ -4408,63 +4408,67 @@ const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
 ///
 /// Where `r` refers to the value returned by the function
 class ResultNameDecl : public ValueDecl {
-
+  friend class ContractSpecifierDecl;
   /// The canonical declaration of a result name is the result name declared in
   /// the first post condition (with a result name) on a particular function.
   ///
   /// The canonical decl is used as the key for the value of the return value
   /// during codegen and constant evaluation. This is necessary because the
   /// changes to the return value in the post conditions must be visible to
   /// subsequent post conditions.
-  ResultNameDecl *CanonicalResultNameDecl = nullptr;
+  ///
+  // FIXME(EricWF): Remove this? I think we can dig the canonical result name
+  // out of the decl context? But I think that will be rather bug prone. Maybe
+  // we could make the ContractSpecifierDecl a DeclContext and dig it up from
+  // there?
+  ResultNameDecl *CanonicalResultName = nullptr;
+
+  /// Whether this result name is using a dummy placeholder type to represent
+  /// the deduced return type of a non-template function until the actual return
+  /// type is known.
+  ///
+  /// Result names are only allowed on non-template functions with deduced
+  /// return types if that function declaration is also a definition.
   bool HasInventedPlaceholderType = false;
 
   ResultNameDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
-                 QualType T, ResultNameDecl *CanonicalResultNameDecl = nullptr,
+                 QualType T, ResultNameDecl *CanonicalDecl = nullptr,
                  bool HasInventedPlaceholderType = false)
       : ValueDecl(Decl::ResultName, DC, IdLoc, Id, T),
-        CanonicalResultNameDecl(CanonicalResultNameDecl),
-        HasInventedPlaceholderType(HasInventedPlaceholderType) {
-    assert(!CanonicalResultNameDecl ||
-           CanonicalResultNameDecl->CanonicalResultNameDecl == nullptr);
-    assert(!CanonicalResultNameDecl || CanonicalResultNameDecl->getType() == T);
+        HasInventedPlaceholderType(HasInventedPlaceholderType) {}
+
+  void setCanonicalResultName(ResultNameDecl *CRND) {
+    assert(CRND != this &&
+           "setCanonicalResultName called on the canonical result");
+    this->CanonicalResultName = CRND;
   }
 
   void anchor() override;
 
 public:
   friend class ASTDeclReader;
 
-  static ResultNameDecl *
-  Create(ASTContext &C, DeclContext *DC, SourceLocation IdLoc,
-         IdentifierInfo *Id, QualType T,
-         ResultNameDecl *CanonicalResultNameDecl = nullptr,
-         bool HasInventedPlaceholderType = false);
+  static ResultNameDecl *Create(ASTContext &C, DeclContext *DC,
+                                SourceLocation IdLoc, IdentifierInfo *Id,
+                                QualType T, ResultNameDecl *CRND = nullptr,
+                                bool HasInventedPlaceholderType = false);
   static ResultNameDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
 
   using ValueDecl::getDeclName;
   using ValueDecl::setType;
 
-  void setCanonicalResultNameDecl(ResultNameDecl *D) {
-    CanonicalResultNameDecl = D;
-  }
-
   /// Returns true if this declaration is the canonical result name declaration
   /// (This is true if it doesn't reference another result name).
-  bool isCanonicalResultNameDecl() const {
-    return CanonicalResultNameDecl == nullptr;
+  bool isCanonicalResultName() const {
+    return getCanonicalResultName() == this;
   }
 
-  ResultNameDecl *getCanonicalResultNameDecl() {
-    if (CanonicalResultNameDecl)
-      return CanonicalResultNameDecl;
-    return this;
+  ResultNameDecl *getCanonicalResultName() {
+    return CanonicalResultName ? CanonicalResultName : this;
   }
 
-  const ResultNameDecl *getCanonicalResultNameDecl() const {
-    if (CanonicalResultNameDecl)
-      return CanonicalResultNameDecl;
-    return this;
+  const ResultNameDecl *getCanonicalResultName() const {
+    return CanonicalResultName ? CanonicalResultName : this;
   }
 
   bool hasInventedPlaceholderType() const { return HasInventedPlaceholderType; }
@@ -4473,13 +4477,20 @@ class ResultNameDecl : public ValueDecl {
   static bool classofKind(Kind K) { return K == Decl::ResultName; }
 };
 
-/// Represents a C++11 static_assert declaration.
+/// Represents a series of contracts on a function declaration.
+/// For instance:
+///
+///   int foo(const int x) pre(x) post(r : x < r);
+///
+/// This declaration also stores whether any of the contracts are invalid.
 class ContractSpecifierDecl final
     : public Decl,
       private llvm::TrailingObjects<ContractSpecifierDecl, ContractStmt *> {
+  friend class TrailingObjects;
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
+  /// The number of contracts in this sequence.
   unsigned NumContracts;
 
   static bool IsPreconditionPred(const ContractStmt *);
@@ -4496,65 +4507,64 @@ class ContractSpecifierDecl final
   ContractSpecifierDecl(DeclContext *DC, SourceLocation Loc,
                         ArrayRef<ContractStmt *> Contracts, bool IsInvalid);
 
-  void setContracts(ArrayRef<ContractStmt *> Contracts) {
-    assert(*getTrailingObjects<ContractStmt *>() == nullptr);
-    std::copy(Contracts.begin(), Contracts.end(),
-              getTrailingObjects<ContractStmt *>());
-  }
+  void setContracts(ArrayRef<ContractStmt *> Contracts);
 
   using FilterRangeT = llvm::iterator_range<llvm::filter_iterator<
       ArrayRef<ContractStmt *>::iterator, bool (*)(const ContractStmt *)>>;
 
   virtual void anchor();
 
 public:
-  friend class TrailingObjects;
-
-  using PreconditionRangeT = FilterRangeT;
-  using PostconditionRangeT = FilterRangeT;
+  static ContractSpecifierDecl *Create(ASTContext &C, DeclContext *DC,
+                                       SourceLocation Loc,
+                                       ArrayRef<ContractStmt *> Contracts,
+                                       bool IsInvalid);
+  static ContractSpecifierDecl *
+  CreateDeserialized(ASTContext &C, GlobalDeclID ID, unsigned NumContracts);
 
+public:
   ArrayRef<ContractStmt *> contracts() const {
     return llvm::ArrayRef(getTrailingObjects<ContractStmt *>(), NumContracts);
   }
 
+  /// Returns a range representing the preconditions in this contract sequence
+  /// (in order of declaration)
   auto preconditions() const {
     return llvm::make_filter_range(contracts(), IsPreconditionPred);
   }
 
+  /// Returns a range representing the postconditions in this contract sequence
+  /// (in order of declaration).
   auto postconditions() const {
     return llvm::make_filter_range(contracts(), IsPostconditionPred);
   }
 
+  /// Returns a range representing the result names in this contract sequence
+  /// (in order of declaration).
   auto result_names() const {
     return llvm::make_filter_range(
         llvm::map_range(postconditions(), ExtractResultName),
         [](ResultNameDecl *R) { return R != nullptr; });
   }
 
+  /// Returns true if this function contract sequence contains result names &
+  /// those result names use an invented placeholder type to allow us to delay
+  /// the deduction of the return type.
   bool hasInventedPlaceholdersTypes() const;
 
   unsigned getNumContracts() const { return NumContracts; }
-  unsigned getNumPreconditions() const {
-    return std::distance(preconditions().begin(), preconditions().end());
-  }
-
-  unsigned getNumPostconditions() const {
-    return std::distance(postconditions().begin(), postconditions().end());
-  }
 
+  /// True if and only if there is a postcondition with a result name in this
+  /// contract sequence.
   bool hasCanonicalResultName() const;
-  ResultNameDecl *getCanonicalResultName() const;
 
-  SourceRange getSourceRange() const override LLVM_READONLY;
+  /// Returns the canonical result name for this contract sequence.
+  const ResultNameDecl *getCanonicalResultName() const;
 
-  friend class ASTDeclReader;
+  /// Update the declaration context of this contract sequence and of any result name declarations contained within it.
+  void setOwningFunction(DeclContext *FD);
 
-  static ContractSpecifierDecl *Create(ASTContext &C, DeclContext *DC,
-                                       SourceLocation Loc,
-                                       ArrayRef<ContractStmt *> Contracts,
-                                       bool IsInvalid);
-  static ContractSpecifierDecl *
-  CreateDeserialized(ASTContext &C, GlobalDeclID ID, unsigned NumContracts);
+  SourceRange getSourceRange() const override LLVM_READONLY;
 
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == Decl::ContractSpecifier; }

clang/include/clang/Basic/LangOptions.def

@@ -160,7 +160,6 @@ LANGOPT(GNUAsm            , 1, 1, "GNU-style inline assembly")
 LANGOPT(Coroutines        , 1, 0, "C++20 coroutines")
 LANGOPT(Contracts         , 1, 1, "C++2c contracts")
 LANGOPT(ContractExceptions, 1, 1, "Contracts report a violation on exception")
-LANGOPT(LateParsedContracts, 1, 0, "Late Parse function contracts")
 LANGOPT(CoroAlignedAllocation, 1, 0, "prefer Aligned Allocation according to P2014 Option 2")
 LANGOPT(DllExportInlines  , 1, 1, "dllexported classes dllexport inline methods")
 LANGOPT(RelaxedTemplateTemplateArgs, 1, 1, "C++17 relaxed matching of template template arguments")

clang/include/clang/Driver/Options.td

@@ -1703,12 +1703,6 @@ defm contract_exceptions : BoolFOption<"contract-exceptions",
           "Enable contract violation on exception">,
   NegFlag<SetFalse, [], [ClangOption, CC1Option],
     "Disable contract violations on exception">>;
-defm late_parsed_contracts : BoolFOption<"late-parsed-contracts",
-  LangOpts<"LateParsedContracts">, DefaultFalse,
-  PosFlag<SetTrue, [], [ClangOption, CC1Option],
-          "Enable contract violation on exception">,
-  NegFlag<SetFalse, [], [ClangOption, CC1Option],
-    "Disable contract violations on exception">>;
 // C++ Coroutines
 defm coroutines : BoolFOption<"coroutines",
   LangOpts<"Coroutines">, Default<cpp20.KeyPath>,

clang/include/clang/Sema/DeclSpec.h

@@ -2137,6 +2137,9 @@ class Declarator {
     CommaLoc = SourceLocation();
     EllipsisLoc = SourceLocation();
     PackIndexingExpr = nullptr;
+    Contracts = nullptr;
+    assert(LateParsedContracts.empty() && "Late-parsed contracts unhandled");
+    LateParsedContracts.clear();
   }
 
   /// mayOmitIdentifier - Return true if the identifier is either optional or
@@ -2800,53 +2803,6 @@ struct FieldDeclarator {
         BitfieldSize(nullptr) {}
 };
 
-class ContractSpecifiers {
-public:
-  enum Specifier { CS_None = 0, CS_Pre, CS_Post };
-
-  struct ContractInfo {
-    Specifier Kind;
-    // Either the parsed expression or token soup for the contract.
-    const IdentifierInfo *ReturnValueIdent;
-    SourceLocation ReturnValueIdentLoc;
-
-    Expr *ParsedContract;
-    std::unique_ptr<CachedTokens> ContractTokens;
-
-  public:
-    ContractInfo(Specifier Kind, const IdentifierInfo *ReturnValueIdent,
-                 SourceLocation ReturnValueIdentLoc, Expr *ParsedContract,
-                 std::unique_ptr<CachedTokens> xContractTokens)
-        : Kind(Kind), ReturnValueIdent(ReturnValueIdent),
-          ReturnValueIdentLoc(ReturnValueIdentLoc),
-          ParsedContract(ParsedContract),
-          ContractTokens(std::move(xContractTokens)) {
-
-      assert(Kind == CS_Post || ReturnValueIdent == nullptr);
-      assert((ContractTokens == nullptr) != (ParsedContract == nullptr));
-    }
-
-    bool isDelayed() const {
-      assert(ParsedContract == nullptr || ContractTokens == nullptr);
-      return ContractTokens != nullptr;
-    }
-    bool isPre() const { return Kind == CS_Pre; }
-    bool isPost() const { return Kind == CS_Post; }
-    bool hasReturnIdentifier() const { return ReturnValueIdent != nullptr; }
-  };
-
-  void addContract(ContractInfo CI) { Contracts.push_back(std::move(CI)); }
-
-  const SmallVectorImpl<ContractInfo> &getContracts() const {
-    return Contracts;
-  }
-
-  bool hasContracts() const { return !Contracts.empty(); }
-
-private:
-  SmallVector<ContractInfo, 2> Contracts;
-};
-
 /// Represents a C++11 virt-specifier-seq.
 class VirtSpecifiers {
 public:

clang/include/clang/Sema/Sema.h

@@ -2746,29 +2746,34 @@ class Sema final : public SemaBase {
 
   ContractSpecifierDecl *
   BuildContractSpecifierDecl(ArrayRef<ContractStmt *> Contracts,
-                             SourceLocation Loc, bool IsInvalid);
+                             DeclContext *DC, SourceLocation Loc,
+                             bool IsInvalid);
 
   // Check two function declarations for equivalent contract sequences.
   // Return true if a diagnostic was issued, false otherwise.
   bool CheckEquivalentContractSequence(FunctionDecl *OrigDecl,
                                        FunctionDecl *NewDecl);
+  bool CheckContractsOnRedeclaration(FunctionDecl *OrigDecl,
+                                     FunctionDecl *NewDecl);
+
+
+  /// Perform semantic analysis for a contract specifier on the specified function.
+  /// For function templates, these checks should be performed with the instantiation of
+  /// the body, and not the declaration.
+  void CheckFunctionContracts(FunctionDecl *FD, bool IsDefinition,
+    bool IsInstantiation);
 
   // FIXME(EricWF): Remove me. These are just convinence hooks while I move
   // things around in the implementation.
   ContractSpecifierDecl *
   ActOnFinishContractSpecifierSequence(ArrayRef<ContractStmt *> ContractStmts,
                                        SourceLocation Loc, bool IsInvalid);
 
-  void CheckFunctionContractSpecifier(FunctionDecl *FD);
-
-  void ActOnContractsOnStartOfFunctionDef(Scope *S, FunctionDecl *FD);
   void ActOnContractsOnFinishFunctionDecl(FunctionDecl *FD, bool IsDefinition);
   void ActOnContractsOnFinishFunctionBody(FunctionDecl *FD);
-  void ActOnContractsOnMergeFunctionDecl(FunctionDecl *OrigDecl,
-                                         FunctionDecl *NewDecl);
 
-  /// Rebuild the contract specifier written on one declaration in the context
-  /// of a the definition. This rebinds parameters and result names as needed.
+  /// Rebuild the contract specifier against another declaration of the function
+  /// (using the new functions parameters)
   ContractSpecifierDecl *
   RebuildContractSpecifierForDecl(FunctionDecl *FirstDecl,
                                   FunctionDecl *Definition);
@@ -13916,11 +13921,6 @@ class Sema final : public SemaBase {
       LocalInstantiationScope &Scope,
       const MultiLevelTemplateArgumentList &TemplateArgs);
 
-  bool addInstantiatedResultNamesToScope(
-      FunctionDecl *Function, const FunctionDecl *PatternDecl,
-      LocalInstantiationScope &Scope,
-      const MultiLevelTemplateArgumentList &TemplateArgs);
-
   int ParsingClassDepth = 0;
 
   class SavePendingParsedClassStateRAII {

clang/lib/AST/Decl.cpp

@@ -2025,29 +2025,7 @@ void DeclaratorDecl::setContracts(ContractSpecifierDecl *NewContracts) {
     getExtInfo()->TInfo = savedTInfo;
   }
 
-  // FIXME(EricWF): This should be empty already
-  auto &Contracts = getExtInfo()->Contracts;
-
-#if 0
-  assert(
-      Contracts.empty() || NewContracts.size() == Contracts.size() ||
-      NewContracts.empty() ||
-      this->isInvalidDecl() &&
-          "Adding a different amount of contracts than were initially present");
-#endif
-  ERICWF_DEBUG_BLOCK_QUIET {
-    if (Contracts && NewContracts && !NewContracts->isInvalidDecl()) {
-      if (Contracts != NewContracts &&
-          Contracts->getNumContracts() != NewContracts->getNumContracts()) {
-        llvm::errs() << "\n\nOverwriting existing contracts!!!\n";
-        Contracts->dumpColor();
-        NewContracts->dumpColor();
-        llvm::errs() << "\n\n==============================\n\n";
-      }
-    }
-  }
-
-  Contracts = NewContracts;
+  getExtInfo()->Contracts = NewContracts;
 }
 
 void DeclaratorDecl::setTemplateParameterListsInfo(