comex updated this revision to Diff 225557.
comex marked 2 inline comments as done.
comex added a comment.
So, I landed this patch but had to revert it as it broke the build on MSVC (and
only MSVC). That was almost a month ago, but I haven't gotten back around to
this until now – sorry :|
In this updated patch, I switched is_random_iterator from a constexpr function:
template <typename T>
constexpr bool is_random_iterator() {
return std::is_same<
typename std::iterator_traits<T>::iterator_category,
std::random_access_iterator_tag>::value;
}
to a type alias:
template <typename T>
using is_random_iterator =
std::is_same<typename std::iterator_traits<T>::iterator_category,
std::random_access_iterator_tag>;
and changed the uses accordingly. For some reason, MSVC thought the two
overloads of `drop_begin`, one with `enable_if<!is_random_iterator<T>()>` and
one with `enable_if<is_random_iterator<T>()>`, were duplicate definitions. But
with `is_random_iterator` changed to a type alias, MSVC is fine with them. GCC
and Clang think both versions are valid, so I think it's just an MSVC bug.
Simplified example for reference: https://gcc.godbolt.org/z/niXCy4
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67647/new/
https://reviews.llvm.org/D67647
Files:
clang/include/clang/AST/Stmt.h
clang/lib/Analysis/Consumed.cpp
llvm/include/llvm/ADT/STLExtras.h
llvm/include/llvm/ADT/iterator_range.h
Index: llvm/include/llvm/ADT/iterator_range.h
===================================================================
--- llvm/include/llvm/ADT/iterator_range.h
+++ llvm/include/llvm/ADT/iterator_range.h
@@ -20,9 +20,15 @@
#include <iterator>
#include <utility>
+#include <cassert>
namespace llvm {
+template <typename T>
+using is_random_iterator =
+ std::is_same<typename std::iterator_traits<T>::iterator_category,
+ std::random_access_iterator_tag>;
+
/// A range adaptor for a pair of iterators.
///
/// This just wraps two iterators into a range-compatible interface. Nothing
@@ -59,11 +65,31 @@
return iterator_range<T>(std::move(p.first), std::move(p.second));
}
+/// Non-random-iterator version
+template <typename T>
+auto drop_begin(T &&t, int n) ->
+ typename std::enable_if<!is_random_iterator<decltype(adl_begin(t))>::value,
+ iterator_range<decltype(adl_begin(t))>>::type {
+ auto begin = adl_begin(t);
+ auto end = adl_end(t);
+ for (int i = 0; i < n; i++) {
+ assert(begin != end);
+ ++begin;
+ }
+ return make_range(begin, end);
+}
+
+/// Optimized version for random iterators
template <typename T>
-iterator_range<decltype(adl_begin(std::declval<T>()))> drop_begin(T &&t,
- int n) {
- return make_range(std::next(adl_begin(t), n), adl_end(t));
+auto drop_begin(T &&t, int n) ->
+ typename std::enable_if<is_random_iterator<decltype(adl_begin(t))>::value,
+ iterator_range<decltype(adl_begin(t))>>::type {
+ auto begin = adl_begin(t);
+ auto end = adl_end(t);
+ assert(end - begin >= n && "Dropping more elements than exist!");
+ return make_range(std::next(begin, n), end);
}
+
}
#endif
Index: llvm/include/llvm/ADT/STLExtras.h
===================================================================
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1573,6 +1573,14 @@
}
template <class T> constexpr T *to_address(T *P) { return P; }
+template <typename R>
+auto index(R &&TheRange,
+ typename std::iterator_traits<detail::IterOfRange<R>>::difference_type N)
+ -> decltype(TheRange.begin()[N]) {
+ assert(N < TheRange.end() - TheRange.begin() && "Index out of range!");
+ return TheRange.begin()[N];
+}
+
} // end namespace llvm
#endif // LLVM_ADT_STLEXTRAS_H
Index: clang/lib/Analysis/Consumed.cpp
===================================================================
--- clang/lib/Analysis/Consumed.cpp
+++ clang/lib/Analysis/Consumed.cpp
@@ -494,8 +494,10 @@
void checkCallability(const PropagationInfo &PInfo,
const FunctionDecl *FunDecl,
SourceLocation BlameLoc);
- bool handleCall(const CallExpr *Call, const Expr *ObjArg,
- const FunctionDecl *FunD);
+
+ using ArgRange = llvm::iterator_range<CallExpr::const_arg_iterator>;
+ bool handleCall(const Expr *Call, const Expr *ObjArg,
+ ArgRange args, const FunctionDecl *FunD);
void VisitBinaryOperator(const BinaryOperator *BinOp);
void VisitCallExpr(const CallExpr *Call);
@@ -608,22 +610,21 @@
// Factors out common behavior for function, method, and operator calls.
// Check parameters and set parameter state if necessary.
// Returns true if the state of ObjArg is set, or false otherwise.
-bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg,
+bool ConsumedStmtVisitor::handleCall(const Expr *Call,
+ const Expr *ObjArg,
+ ArgRange Args,
const FunctionDecl *FunD) {
- unsigned Offset = 0;
- if (isa<CXXOperatorCallExpr>(Call) && isa<CXXMethodDecl>(FunD))
- Offset = 1; // first argument is 'this'
-
// check explicit parameters
- for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
+ unsigned Index = 0;
+ for (const Expr *Arg : Args) {
// Skip variable argument lists.
- if (Index - Offset >= FunD->getNumParams())
+ if (Index >= FunD->getNumParams())
break;
- const ParmVarDecl *Param = FunD->getParamDecl(Index - Offset);
+ const ParmVarDecl *Param = FunD->getParamDecl(Index++);
QualType ParamType = Param->getType();
- InfoEntry Entry = findInfo(Call->getArg(Index));
+ InfoEntry Entry = findInfo(Arg);
if (Entry == PropagationMap.end() || Entry->second.isTest())
continue;
@@ -636,7 +637,7 @@
if (ParamState != ExpectedState)
Analyzer.WarningsHandler.warnParamTypestateMismatch(
- Call->getArg(Index)->getExprLoc(),
+ Arg->getExprLoc(),
stateToString(ExpectedState), stateToString(ParamState));
}
@@ -749,7 +750,7 @@
return;
}
- handleCall(Call, nullptr, FunDecl);
+ handleCall(Call, nullptr, Call->arguments(), FunDecl);
propagateReturnType(Call, FunDecl);
}
@@ -805,7 +806,7 @@
if (!MD)
return;
- handleCall(Call, Call->getImplicitObjectArgument(), MD);
+ handleCall(Call, Call->getImplicitObjectArgument(), Call->arguments(), MD);
propagateReturnType(Call, MD);
}
@@ -813,18 +814,20 @@
const CXXOperatorCallExpr *Call) {
const auto *FunDecl = dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee());
if (!FunDecl) return;
+ ArgRange Args = Call->arguments();
if (Call->getOperator() == OO_Equal) {
- ConsumedState CS = getInfo(Call->getArg(1));
- if (!handleCall(Call, Call->getArg(0), FunDecl))
- setInfo(Call->getArg(0), CS);
+ ConsumedState CS = getInfo(llvm::index(Args, 1));
+ if (!handleCall(Call, llvm::index(Args, 0), llvm::drop_begin(Args, 1),
+ FunDecl))
+ setInfo(llvm::index(Args, 0), CS);
return;
}
- if (const auto *MCall = dyn_cast<CXXMemberCallExpr>(Call))
- handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl);
+ if (isa<CXXMethodDecl>(FunDecl))
+ handleCall(Call, llvm::index(Args, 0), llvm::drop_begin(Args, 1), FunDecl);
else
- handleCall(Call, Call->getArg(0), FunDecl);
+ handleCall(Call, nullptr, Args, FunDecl);
propagateReturnType(Call, FunDecl);
}
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1041,7 +1041,8 @@
template<typename T, typename TPtr = T *, typename StmtPtr = Stmt *>
struct CastIterator
: llvm::iterator_adaptor_base<CastIterator<T, TPtr, StmtPtr>, StmtPtr *,
- std::random_access_iterator_tag, TPtr> {
+ std::random_access_iterator_tag, TPtr,
+ int, void, TPtr> {
using Base = typename CastIterator::iterator_adaptor_base;
CastIterator() : Base(nullptr) {}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
