r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

Douglas Gregor via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 5 14:05:25 PDT 2017


> On Jul 5, 2017, at 1:33 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 
> On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits
> <cfe-commits at lists.llvm.org <mailto:cfe-commits at 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. 

	- 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 at +4 {{default template arguments for a function template are a C++11 extension}}
>> +  // expected-warning at +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 at +4 {{default template arguments for a function template are a C++11 extension}}
>> +  // expected-warning at +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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170705/0093e776/attachment-0001.html>


More information about the cfe-commits mailing list