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()) ---------------- urnathan wrote: > 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' also, the same functionality (with more generality) is needed for comparing regular function default arguments with multiple definitions in the GM. How is that done (or is it yet to be implemented?) 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
