Skip to content

Commit

Permalink
[clang][Sema] Remove irrelevant diagnostics from constraint satisfact…
Browse files Browse the repository at this point in the history
…ion failure

BEFORE this patch, when clang handles constraints like C1 || C2 where C1 evaluates to false and C2 evaluates to true, it emitted irrelevant diagnostics about the falsity of C1.
This patch removes the irrelevant diagnostic information generated during the evaluation of C1 if C2 evaluates to true.

Fixes llvm#54678

Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D157526
  • Loading branch information
hazohelet committed Sep 18, 2023
1 parent dd55ece commit b2cd9db
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 0 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ Improvements to Clang's diagnostics
Also clang no longer emits false positive warnings about the output length of
``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
- Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
- Clang no longer emits irrelevant notes about unsatisfied constraint expressions
on the left-hand side of ``||`` when the right-hand side constraint is satisfied.
(`#54678: <https://github.com/llvm/llvm-project/issues/54678>`_).

Bug Fixes in This Version
-------------------------
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
ConstraintExpr = ConstraintExpr->IgnoreParenImpCasts();

if (LogicalBinOp BO = ConstraintExpr) {
auto EffectiveDetailEnd = Satisfaction.Details.end();
ExprResult LHSRes = calculateConstraintSatisfaction(
S, BO.getLHS(), Satisfaction, Evaluator);

Expand Down Expand Up @@ -218,6 +219,19 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
if (RHSRes.isInvalid())
return ExprError();

bool IsRHSSatisfied = Satisfaction.IsSatisfied;
// Current implementation adds diagnostic information about the falsity
// of each false atomic constraint expression when it evaluates them.
// When the evaluation results to `false || true`, the information
// generated during the evaluation of left-hand side is meaningless
// because the whole expression evaluates to true.
// The following code removes the irrelevant diagnostic information.
// FIXME: We should probably delay the addition of diagnostic information
// until we know the entire expression is false.
if (BO.isOr() && IsRHSSatisfied)
Satisfaction.Details.erase(EffectiveDetailEnd,
Satisfaction.Details.end());

return BO.recreateBinOp(S, LHSRes, RHSRes);
}

Expand Down
37 changes: 37 additions & 0 deletions clang/test/SemaTemplate/concepts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -994,3 +994,40 @@ void test_params() {
}

}

namespace GH54678 {
template<class>
concept True = true;

template<class>
concept False = false; // expected-note 9 {{'false' evaluated to false}}

template<class>
concept Irrelevant = false;

template <typename T>
concept ErrorRequires = requires(ErrorRequires auto x) { x; }; // expected-error {{unknown type name 'ErrorRequires'}}

template<class T> void aaa(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires (False<T> || False<T>) || False<T> {} // expected-note 3 {{'int' does not satisfy 'False'}}
template<class T> void bbb(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires (False<T> || False<T>) && True<T> {} // expected-note 2 {{'long' does not satisfy 'False'}}
template<class T> void ccc(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires (True<T> || Irrelevant<T>) && False<T> {} // expected-note {{'unsigned long' does not satisfy 'False'}}
template<class T> void ddd(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires (Irrelevant<T> || True<T>) && False<T> {} // expected-note {{'int' does not satisfy 'False'}}
template<class T> void eee(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires (Irrelevant<T> || Irrelevant<T> || True<T>) && False<T> {} // expected-note {{'long' does not satisfy 'False'}}

template<class T> void fff(T t) // expected-note {{candidate template ignored: constraints not satisfied}}
requires((ErrorRequires<T> || False<T> || True<T>) && False<T>) {} // expected-note {{'unsigned long' does not satisfy 'False'}}

void test() {
aaa(42); // expected-error {{no matching function}}
bbb(42L); // expected-error{{no matching function}}
ccc(42UL); // expected-error {{no matching function}}
ddd(42); // expected-error {{no matching function}}
eee(42L); // expected-error {{no matching function}}
fff(42UL); // expected-error {{no matching function}}
}
}

0 comments on commit b2cd9db

Please sign in to comment.