On 5 July 2017 at 14:05, Douglas Gregor via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> > On Jul 5, 2017, at 1:33 PM, Aaron Ballman <aa...@aaronballman.com> wrote: > > On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > Author: dgregor > Date: Wed Jul 5 13:20:15 2017 > New Revision: 307197 > > URL: http://llvm.org/viewvc/llvm-project?rev=307197&view=rev > Log: > Cope with Range-v3's CONCEPT_REQUIRES idiom > > Modified: > cfe/trunk/lib/Sema/SemaTemplate.cpp > cfe/trunk/test/SemaTemplate/overload-candidates.cpp > > Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaTemplate.cpp?rev=307197&r1=307196&r2=307197&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul 5 13:20:15 2017 > @@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr > Terms.push_back(Clause); > } > > +// The ranges-v3 library uses an odd pattern of a top-level "||" with > +// a left-hand side that is value-dependent but never true. Identify > +// the idiom and ignore that term. > +static Expr *lookThroughRangesV3Condition(Preprocessor &PP, Expr *Cond) { > + // Top-level '||'. > + auto *BinOp = dyn_cast<BinaryOperator>(Cond->IgnoreParenImpCasts()); > + if (!BinOp) return Cond; > + > + if (BinOp->getOpcode() != BO_LOr) return Cond; > + > + // With an inner '==' that has a literal on the right-hand side. > + Expr *LHS = BinOp->getLHS(); > + auto InnerBinOp = dyn_cast<BinaryOperator>(LHS->IgnoreParenImpCasts()); > > > auto * please (or better yet, const auto * here and above). > > > Sure, commit incoming. > > > + if (!InnerBinOp) return Cond; > + > + if (InnerBinOp->getOpcode() != BO_EQ || > + !isa<IntegerLiteral>(InnerBinOp->getRHS())) > + return Cond; > + > + // If the inner binary operation came from a macro expansion named > + // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side > + // of the '||', which is the real, user-provided condition. > + auto Loc = InnerBinOp->getExprLoc(); > > > Please do not use auto here. > > > I’m fine with spelling it out, although I don’t know what criteria you’re > applying here. Presumably it’s okay to use “auto” when the initializer is > some form of cast to that type, but you prefer not to use “auto” elsewhere, > despite the “Loc” in the variable name and in the initializer? > > It's unfortunate that we have this special case instead of a more > general mechanism. While I love Ranges v3, I'm not keen on a compiler > hack just for it. These "tricks" have a habit of leaking out into > other user code, which will behave differently than what happens with > > Ranges. > > > It’s possible that we can do something more general here by converting the > expression to disjunctive normal form and identifying when some of the > top-level terms will never succeed. > Hmm. Ranges v3 produces things like: template<int N = 42, enable_if<N == 43 || some_condition>> ... for which the top-level "N == 43" check can succeed if N is explicitly specified. Perhaps the trick we need here is to notice that N was not, in fact, explicitly specified, and so a requirement on N is not likely to be interesting. - Doug > > ~Aaron > > + if (!Loc.isMacroID()) return Cond; > + > + StringRef MacroName = PP.getImmediateMacroName(Loc); > + if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_") > + return BinOp->getRHS(); > + > + return Cond; > +} > + > /// Find the failed subexpression within enable_if, and describe it > /// with a string. > static std::pair<Expr *, std::string> > findFailedEnableIfCondition(Sema &S, Expr *Cond) { > + Cond = lookThroughRangesV3Condition(S.PP, Cond); > + > // Separate out all of the terms in a conjunction. > SmallVector<Expr *, 4> Terms; > collectConjunctionTerms(Cond, Terms); > > Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > SemaTemplate/overload-candidates.cpp?rev=307197&r1= > 307196&r2=307197&view=diff > ============================================================ > ================== > --- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original) > +++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul 5 > 13:20:15 2017 > @@ -137,4 +137,30 @@ namespace PR15673 { > #endif > void wibble() {} > void wobble() { wibble<int>(); } // expected-error {{no matching > function for call to 'wibble'}} > + > + template<typename T> > + struct some_passing_trait : std::true_type {}; > + > +#if __cplusplus <= 199711L > + // expected-warning@+4 {{default template arguments for a function > template are a C++11 extension}} > + // expected-warning@+4 {{default template arguments for a function > template are a C++11 extension}} > +#endif > + template<typename T, > + int n = 42, > + typename std::enable_if<n == 43 || > (some_passing_trait<T>::value && some_trait<T>::value), int>::type = 0> > + void almost_rangesv3(); // expected-note{{candidate template ignored: > requirement '42 == 43 || (some_passing_trait<int>::value && > some_trait<int>::value)' was not satisfied}} > + void test_almost_rangesv3() { almost_rangesv3<int>(); } // > expected-error{{no matching function for call to 'almost_rangesv3'}} > + > + #define CONCEPT_REQUIRES_(...) \ > + int x = 42, \ > + typename std::enable_if<(x == 43) || (__VA_ARGS__)>::type = 0 > + > +#if __cplusplus <= 199711L > + // expected-warning@+4 {{default template arguments for a function > template are a C++11 extension}} > + // expected-warning@+4 {{default template arguments for a function > template are a C++11 extension}} > +#endif > + template<typename T, > + CONCEPT_REQUIRES_(some_passing_trait<T>::value && > some_trait<T>::value)> > + void rangesv3(); // expected-note{{candidate template ignored: > requirement 'some_trait<int>::value' was not satisfied [with T = int, x = > 42]}} > + void test_rangesv3() { rangesv3<int>(); } // expected-error{{no > matching function for call to 'rangesv3'}} > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
