> 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 <mailto: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. 

        - 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

Reply via email to