[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:263
 
+  if (SubstitutedAtomicExpr.get()->isValueDependent())
+    return SubstitutedAtomicExpr;
----------------
erichkeane wrote:
> So this bit is concerning to me... we have been catching a ton of bugs in the MLTAL stuff by trying to constant evaluate an expression that isn't correctly constexpr, and this will defeat it.  We shouldn't be calling this function at all on non-fully-instantiated expressions.  What is the case that ends up coming through here, and should be be calling this at all?
This happens e.g. for concepts-PR54629.cpp

```
Old:
FunctionDecl 0x555564d90378 llvm-project/clang/test/SemaTemplate/concepts-PR54629.cpp:30:1, line:32:2> line:30:6 foo 'void ()'
|-RequiresExpr 0x555564d90318 <line:31:12, col:53> 'bool'
| |-ParmVarDecl 0x555564d90150 <col:21, col:24> col:24 referenced t 'T &'
| `-NestedRequirement 0x555564d902d8 dependent
|   `-BinaryOperator 0x555564d902b8 <col:38, col:50> 'bool' '<'
|     |-UnaryExprOrTypeTraitExpr 0x555564d90260 <col:38, col:46> 'unsigned long' sizeof
|     | `-ParenExpr 0x555564d90240 <col:44, col:46> 'T' lvalue
|     |   `-DeclRefExpr 0x555564d90220 <col:45> 'T' lvalue ParmVar 0x555564d90150 't' 'T &' non_odr_use_unevaluated
|     `-ImplicitCastExpr 0x555564d902a0 <col:50> 'unsigned long' <IntegralCast>
|       `-IntegerLiteral 0x555564d90280 <col:50> 'int' 4
`-CompoundStmt 0x555564d90f70 <line:32:1, col:2>

while MLTAL is empty.
```
(clang::Sema::CheckOverload calls clang::Sema::IsOverload, while clang::Sema::IsOverload invokes clang::Sema::AreConstraintExpressionsEqual)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146178/new/

https://reviews.llvm.org/D146178