r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 5 14:27:08 PDT 2017
On Wed, Jul 5, 2017 at 5:05 PM, Douglas Gregor <dgregor at apple.com> 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.
Thanks!
>
>
> + 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?
I believe we usually only use auto when the type is explicitly spelled
out in the initializer (dyn_cast, getAs, etc), when it's overly
complicated to spell (iterators), or when it's in a range-based for
loop (because it's generally quite obvious what the type will be based
on the container).
Loc here is somewhat obvious, but not entirely; we have a lot of
things named "location" (SourceLocation, PresumedLoc, FullSourceLoc,
and more).
>
> 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.
That might work. Or Richard's suggestion below. I'm mostly worried
about people "trying this at home" and getting different results
because that seems inexplicable. Also, if the Ranges TS decides to
rename their macro, our compiler stops behaving nicely, which isn't
ideal.
~Aaron
>
> - 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
>
>
More information about the cfe-commits
mailing list