r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 5 14:19:07 PDT 2017


On 5 July 2017 at 14:05, Douglas Gregor via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

>
> 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> 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 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
>
>
>
> _______________________________________________
> 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/1f80bdf7/attachment-0001.html>


More information about the cfe-commits mailing list