gAlfonso-bit updated this revision to Diff 365224.
gAlfonso-bit added a comment.

Moved warnReturnTypestateForUnconsumableType back to SemaDeclCXX


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107723/new/

https://reviews.llvm.org/D107723

Files:
  clang/include/clang/Analysis/Analyses/Consumed.h
  clang/lib/Analysis/Consumed.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/warn-consumed-analysis.cpp
  clang/test/SemaCXX/warn-consumed-parsing.cpp

Index: clang/test/SemaCXX/warn-consumed-parsing.cpp
===================================================================
--- clang/test/SemaCXX/warn-consumed-parsing.cpp
+++ clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -6,16 +6,6 @@
 #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}}
Index: clang/test/SemaCXX/warn-consumed-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-consumed-analysis.cpp
+++ clang/test/SemaCXX/warn-consumed-analysis.cpp
@@ -74,6 +74,11 @@
 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'}}
 }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1190,17 +1190,14 @@
     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));
 }
@@ -1222,30 +1219,27 @@
     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));
 }
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2109,14 +2109,6 @@
     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 {
 
Index: clang/lib/Analysis/Consumed.cpp
===================================================================
--- clang/lib/Analysis/Consumed.cpp
+++ clang/lib/Analysis/Consumed.cpp
@@ -1206,12 +1206,6 @@
   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);
Index: clang/include/clang/Analysis/Analyses/Consumed.h
===================================================================
--- clang/include/clang/Analysis/Analyses/Consumed.h
+++ clang/include/clang/Analysis/Analyses/Consumed.h
@@ -89,16 +89,6 @@
                                             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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to