[Clang] Moved warning of warnReturnTypestateForUnconsumableType back to SemaDeclAttr

[aaron.ballman]

It looks like there are still test failures happening from these changes. We're now getting more return state set for an unconsumable type <foo> diagnostics.

[gAlfonso-bit]

Moved warnReturnTypestateForUnconsumableType back to SemaDeclCXX

clang/include/clang/Analysis/Analyses/Consumed.h

virtual void warnParamReturnTypestateMismatch(SourceLocation Loc,
                                                   StringRef ExpectedState,
                                                   StringRef ObservedState) {}
 
     // FIXME: Add documentation.
     virtual void warnParamTypestateMismatch(SourceLocation LOC,
                                             StringRef ExpectedState,
                                             StringRef ObservedState) {}
 
-    // FIXME: This can be removed when the attr propagation fix for templated
-    //        classes lands.
-    /// Warn about return typestates set for unconsumable types.
-    ///
-    /// \param Loc -- The location of the attributes.
-    ///
-    /// \param TypeName -- The name of the unconsumable type.
-    virtual void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
-                                                        StringRef TypeName) {}
-
     /// Warn about return typestate mismatches.
     ///
     /// \param Loc -- The SourceLocation of the return statement.
     ///
     /// \param ExpectedState -- The state the return value was expected to be
     /// in.
     ///
     /// \param ObservedState -- The state the return value was observed to be

clang/lib/Analysis/Consumed.cpp

void ConsumedAnalyzer::determineExpectedReturnState(AnalysisDeclContext &AC,
   if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(D)) {
     ReturnType = Constructor->getThisType()->getPointeeType();
   } else
     ReturnType = D->getCallResultType();
 
   if (const ReturnTypestateAttr *RTSAttr = D->getAttr<ReturnTypestateAttr>()) {
     const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
     if (!RD || !RD->hasAttr<ConsumableAttr>()) {
-      // FIXME: This should be removed when template instantiation propagates
-      //        attributes at template specialization definition, not
-      //        declaration. When it is removed the test needs to be enabled
-      //        in SemaDeclAttr.cpp.
-      WarningsHandler.warnReturnTypestateForUnconsumableType(
-          RTSAttr->getLocation(), ReturnType.getAsString());
       ExpectedReturnState = CS_None;
     } else
       ExpectedReturnState = mapReturnTypestateAttrState(RTSAttr);
   } else if (isConsumableType(ReturnType)) {
     if (isAutoCastType(ReturnType))   // We can auto-cast the state to the
       ExpectedReturnState = CS_None;  // expected state.
     else
       ExpectedReturnState = mapConsumableAttrState(ReturnType);

clang/lib/Sema/AnalysisBasedWarnings.cpp

void warnParamTypestateMismatch(SourceLocation Loc, StringRef ExpectedState,
                                   StringRef ObservedState) override {
 
     PartialDiagnosticAt Warning(Loc, S.PDiag(
       diag::warn_param_typestate_mismatch) << ExpectedState << ObservedState);
 
     Warnings.emplace_back(std::move(Warning), OptionalNotes());
   }
 
-  void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
-                                              StringRef TypeName) override {
-    PartialDiagnosticAt Warning(Loc, S.PDiag(
-      diag::warn_return_typestate_for_unconsumable_type) << TypeName);
-
-    Warnings.emplace_back(std::move(Warning), OptionalNotes());
-  }
-
   void warnReturnTypestateMismatch(SourceLocation Loc, StringRef ExpectedState,
                                    StringRef ObservedState) override {
 
     PartialDiagnosticAt Warning(Loc, S.PDiag(
       diag::warn_return_typestate_mismatch) << ExpectedState << ObservedState);
 
     Warnings.emplace_back(std::move(Warning), OptionalNotes());
   }

clang/lib/Sema/SemaDeclAttr.cpp

if (!ParamTypestateAttr::ConvertStrToConsumedState(StateString,
       return;
     }
   } else {
     S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
         << AL << AANT_ArgumentIdentifier;
     return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //        enabled here only after the parser propagates attributes at
-  //        template specialization definition, not declaration.
-  //QualType ReturnType = cast<ParmVarDecl>(D)->getType();
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr<ConsumableAttr>()) {
-  //    S.Diag(AL.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //      ReturnType.getAsString();
-  //    return;
-  //}
+  QualType ReturnType = cast<ParmVarDecl>(D)->getType();
+  const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
+
+  if (!RD || !RD->hasAttr<ConsumableAttr>()) {
+    S.Diag(AL.getLoc(), diag::warn_return_typestate_for_unconsumable_type)
+        << ReturnType.getAsString();
+    return;
+  }
 
   D->addAttr(::new (S.Context) ParamTypestateAttr(S.Context, AL, ParamState));
 }
 
 static void handleReturnTypestateAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   ReturnTypestateAttr::ConsumedState ReturnState;
 
   if (AL.isArgIdent(0)) {
     IdentifierLoc *IL = AL.getArgAsIdent(0);
     if (!ReturnTypestateAttr::ConvertStrToConsumedState(IL->Ident->getName(),
                                                         ReturnState)) {
       S.Diag(IL->Loc, diag::warn_attribute_type_not_supported) << AL
                                                                << IL->Ident;
       return;
     }
   } else {
     S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
         << AL << AANT_ArgumentIdentifier;
     return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //        enabled here only after the parser propagates attributes at
-  //        template specialization definition, not declaration.
-  //QualType ReturnType;
-  //
-  //if (const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D)) {
-  //  ReturnType = Param->getType();
-  //
-  //} else if (const CXXConstructorDecl *Constructor =
-  //             dyn_cast<CXXConstructorDecl>(D)) {
-  //  ReturnType = Constructor->getThisType()->getPointeeType();
-  //
-  //} else {
-  //
-  //  ReturnType = cast<FunctionDecl>(D)->getCallResultType();
-  //}
-  //
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr<ConsumableAttr>()) {
-  //    S.Diag(Attr.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //      ReturnType.getAsString();
-  //    return;
-  //}
+  QualType ReturnType;
+
+  if (const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D)) {
+    ReturnType = Param->getType();
+
+  } else if (const CXXConstructorDecl *Constructor =
+                 dyn_cast<CXXConstructorDecl>(D)) {
+    ReturnType = Constructor->getThisType()->getPointeeType();
+
+  } else {
+
+    ReturnType = cast<FunctionDecl>(D)->getCallResultType();
+  }
+
+  const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
+
+  if (!RD || !RD->hasAttr<ConsumableAttr>()) {
+    S.Diag(AL.getLoc(), diag::warn_return_typestate_for_unconsumable_type)
+        << ReturnType.getAsString();
+    return;
+  }
 
   D->addAttr(::new (S.Context) ReturnTypestateAttr(S.Context, AL, ReturnState));
 }
 
 static void handleSetTypestateAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), AL))
     return;
 

clang/test/SemaCXX/warn-consumed-analysis.cpp

 void baf1(const ConsumableClass<int> &var);
 void baf2(const ConsumableClass<int> *var);
 
 void baf3(ConsumableClass<int>   var);
 void baf4(ConsumableClass<int>  &var);
 void baf5(ConsumableClass<int>  *var);
 void baf6(ConsumableClass<int> &&var);
 
+int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
+int returnTypestateForUnconsumable() {
+  return 0;
+}
+
 ConsumableClass<int> returnsUnconsumed() {
   return ConsumableClass<int>(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}}
 }
 
 ConsumableClass<int> returnsConsumed() RETURN_TYPESTATE(consumed);
 ConsumableClass<int> returnsConsumed() {
   return ConsumableClass<int>();
 }
if (var0 && var1 && var2) {
     *var1;
     *var2;
     
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
     *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
   }
-  
-#if 0
-  // FIXME: Get this test to pass.
   if (var0 || var1 || var2) {
     *var0;
     *var1;
     *var2;
     
   } else {
     *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}
     *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
     *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
   }
-#endif
 }
 
 void testComplexConditionals1() {
   ConsumableClass<int> var0, var1, var2;
   
   // Coerce all variables into the unknown state.
   baf4(var0);
   baf4(var1);

clang/test/SemaCXX/warn-consumed-parsing.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
 
 #define CALLABLE_WHEN(...)      __attribute__ ((callable_when(__VA_ARGS__)))
 #define CONSUMABLE(state)       __attribute__ ((consumable(state)))
 #define SET_TYPESTATE(state)    __attribute__ ((set_typestate(state)))
 #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
 #define TEST_TYPESTATE(state)   __attribute__ ((test_typestate(state)))
 
-// FIXME: This test is here because the warning is issued by the Consumed
-//        analysis, not SemaDeclAttr.  The analysis won't run after an error
-//        has been issued.  Once the attribute propagation for template
-//        instantiation has been fixed, this can be moved somewhere else and the
-//        definition can be removed.
-int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
-int returnTypestateForUnconsumable() {
-  return 0;
-}
-
 class AttrTester0 {
   void consumes()       __attribute__ ((set_typestate())); // expected-error {{'set_typestate' attribute takes one argument}}
   bool testUnconsumed() __attribute__ ((test_typestate())); // expected-error {{'test_typestate' attribute takes one argument}}
   void callableWhen()   __attribute__ ((callable_when())); // expected-error {{'callable_when' attribute takes at least 1 argument}}
 };
 
 int var0 SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to functions}}
 int var1 TEST_TYPESTATE(consumed); // expected-warning {{'test_typestate' attribute only applies to}}