urnathan added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 + Expr::EvalResult EVRX, EVRY; + if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || + !DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + + APValue VX = EVRX.Val, VY = EVRY.Val; + if (VX.getKind() != VY.getKind()) ---------------- ChuanqiXu wrote: > urnathan wrote: > > ChuanqiXu wrote: > > > urnathan wrote: > > > > I'm kind of surprised how complex this check is. Isn't there an AST > > > > comparator available somewhere? > > > I found ODRHash. I think it is much better now. > > hm that suggests there there must be a comparator too -- this isn't a > > cryptographically strong hash is it? How would the compiler currently make > > use of 'definitely different' and 'probably the same' without such a > > comparator? > Yeah, I am sure there is not an such comparator. Or it has some methods like: > `ASTContext::hasSameType` for type and `ASTContext::isSameEntity()` for Decl. > But it lacks such methods now for Stmt and Expr. > > > How would the compiler currently make use of 'definitely different' and > > 'probably the same' without such a comparator? > > Now it uses the two methods I listed above and ODRHash to compare. I think > the two methods works for 'definitely different' and ODRHash works for > 'probably the same'. So it's the reason why my previous implementation looks > lengthy. Since I want to handle it by hand. (The previous method only works > for simple Expr. I think it would be large work to implement comparator for > whole Expr or Stmt). Hm, how do template instantations work -- there must be some way of determining 'is this instantation just here the same as one I've already seen' CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/D118034 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits