alexander-shaposhnikov added inline comments.
================ Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) + return SubstitutedAtomicExpr; ---------------- erichkeane wrote: > alexander-shaposhnikov wrote: > > 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) > Hmm.... that seems wrong for me, but I'm not sure how. It doesn't seem right > for `AreConstraintExpressionsEqual`to try to calculate the constraint > satisfaction... I think we get there from https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateInstantiate.cpp#L2375 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
