comex created this revision.
comex added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Also, fix the order of `if` statements so that an explicit `return_typestate`
annotation takes precedence over the default behavior for rvalue refs.
Note that for by-value arguments, the object being consumed is always a
temporary. If you write:
`void a(X); a((X &&)x);`
...this first constructs a temporary X by moving from `x`, then calls `a` with
a pointer to the temporary. Before and after this change, the move copies
`x`'s typestate to the temporary and marks `x` itself as consumed. But before
this change, if the temporary started out unconsumed (because `x` was
unconsumed before the move), it would still be unconsumed when its destructor
was run after the call. Now it will be considered consumed at that point.
Also note that currently, only parameters explicitly marked `return_typestate`
have their state-on-return checked on the callee side. Parameters which are
implicitly consuming due to being rvalue references – or, after this patch,
by-value – are checked only on the caller side. This discrepancy was very
surprising to me as a user. I wrote a fix, but in my codebase it broke some
code that was using `std::forward`. Therefore, I plan to hold off on
submitting the fix until a followup patch, which will generalize the current
`std::move` special case into an attribute that can be applied to any 'cast'
function like `std::move` and `std::forward`.
Repository:
rC Clang
https://reviews.llvm.org/D67743
Files:
lib/Analysis/Consumed.cpp
test/SemaCXX/warn-consumed-analysis.cpp
Index: test/SemaCXX/warn-consumed-analysis.cpp
===================================================================
--- test/SemaCXX/warn-consumed-analysis.cpp
+++ test/SemaCXX/warn-consumed-analysis.cpp
@@ -53,10 +53,15 @@
public:
DestructorTester();
DestructorTester(int);
+ DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+ DestructorTester(DestructorTester &&);
void operator*() CALLABLE_WHEN("unconsumed");
~DestructorTester() CALLABLE_WHEN("consumed");
+
+ static void byVal(DestructorTester);
+ static void byValMarkUnconsumed(DestructorTester
RETURN_TYPESTATE(unconsumed));
};
void baf0(const ConsumableClass<int> var);
@@ -120,6 +125,19 @@
expected-warning {{invalid invocation of method
'~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
}
+void testDestructionByVal() {
+ {
+ // both the var and the temporary are consumed:
+ DestructorTester D0(nullptr);
+ DestructorTester::byVal((DestructorTester &&)D0);
+ }
+ {
+ // the var is consumed but the temporary isn't:
+ DestructorTester D1(nullptr);
+ DestructorTester::byValMarkUnconsumed((DestructorTester &&)D1); //
expected-warning {{invalid invocation of method '~DestructorTester' on a
temporary object while it is in the 'unconsumed' state}}
+ }
+}
+
void testTempValue() {
*ConsumableClass<int>(); // expected-warning {{invalid invocation of method
'operator*' on a temporary object while it is in the 'consumed' state}}
}
@@ -413,10 +431,15 @@
Param.consume();
}
+void testRvalueRefParamReturnTypestateCallee(ConsumableClass<int> &&Param
RETURN_TYPESTATE(unconsumed)) {
+ Param.unconsume();
+}
+
void testParamReturnTypestateCaller() {
ConsumableClass<int> var;
testParamReturnTypestateCallee(true, var);
+ testRvalueRefParamReturnTypestateCallee((ConsumableClass<int> &&)var);
*var;
}
@@ -480,6 +503,9 @@
baf2(&var);
*var;
+
+ baf3(var);
+ *var;
baf4(var);
*var; // expected-warning {{invalid invocation of method 'operator*' on
object 'var' while it is in the 'unknown' state}}
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ lib/Analysis/Consumed.cpp
@@ -645,10 +645,10 @@
continue;
// Adjust state on the caller side.
- if (isRValueRef(ParamType))
- setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
- else if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
+ if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+ else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+ setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
else if (isPointerOrRef(ParamType) &&
(!ParamType->getPointeeType().isConstQualified() ||
isSetOnReadPtrType(ParamType)))
Index: test/SemaCXX/warn-consumed-analysis.cpp
===================================================================
--- test/SemaCXX/warn-consumed-analysis.cpp
+++ test/SemaCXX/warn-consumed-analysis.cpp
@@ -53,10 +53,15 @@
public:
DestructorTester();
DestructorTester(int);
+ DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+ DestructorTester(DestructorTester &&);
void operator*() CALLABLE_WHEN("unconsumed");
~DestructorTester() CALLABLE_WHEN("consumed");
+
+ static void byVal(DestructorTester);
+ static void byValMarkUnconsumed(DestructorTester RETURN_TYPESTATE(unconsumed));
};
void baf0(const ConsumableClass<int> var);
@@ -120,6 +125,19 @@
expected-warning {{invalid invocation of method '~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
}
+void testDestructionByVal() {
+ {
+ // both the var and the temporary are consumed:
+ DestructorTester D0(nullptr);
+ DestructorTester::byVal((DestructorTester &&)D0);
+ }
+ {
+ // the var is consumed but the temporary isn't:
+ DestructorTester D1(nullptr);
+ DestructorTester::byValMarkUnconsumed((DestructorTester &&)D1); // expected-warning {{invalid invocation of method '~DestructorTester' on a temporary object while it is in the 'unconsumed' state}}
+ }
+}
+
void testTempValue() {
*ConsumableClass<int>(); // expected-warning {{invalid invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}
}
@@ -413,10 +431,15 @@
Param.consume();
}
+void testRvalueRefParamReturnTypestateCallee(ConsumableClass<int> &&Param RETURN_TYPESTATE(unconsumed)) {
+ Param.unconsume();
+}
+
void testParamReturnTypestateCaller() {
ConsumableClass<int> var;
testParamReturnTypestateCallee(true, var);
+ testRvalueRefParamReturnTypestateCallee((ConsumableClass<int> &&)var);
*var;
}
@@ -480,6 +503,9 @@
baf2(&var);
*var;
+
+ baf3(var);
+ *var;
baf4(var);
*var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ lib/Analysis/Consumed.cpp
@@ -645,10 +645,10 @@
continue;
// Adjust state on the caller side.
- if (isRValueRef(ParamType))
- setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
- else if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
+ if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+ else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+ setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
else if (isPointerOrRef(ParamType) &&
(!ParamType->getPointeeType().isConstQualified() ||
isSetOnReadPtrType(ParamType)))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
