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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits