rtrieu marked 2 inline comments as done. rtrieu added inline comments.
================ Comment at: lib/AST/Expr.cpp:3931-3932 + case DeclRefExprClass: { + // DeclRefExpr without an ImplicitCastExpr can happen for integral + // template parameters. + const auto *DRE1 = cast<DeclRefExpr>(E1); ---------------- NoQ wrote: > Does this actually happen *after* instantiation? If not - do we even need to > emit warnings on templates that aren't instantiated? I guess we still need > this branch because `isSameComparisonOperand()` lives in the `Expr` class and > should always work regardless, but in this case i guess this deserves a > unittest(?) This happens before instantiation. The test cases are in (test/SemaTemplate/self-comparison.cpp). The basic idea is to allow integral template parameters to be treated as variables. ``` template<int A> void foo(int x) { bool b1 = A == A; bool b2 = x == x; } ``` If we treat template parameter A the same as variable x, we can diagnose a few more tautological types, even without instantiation. ================ Comment at: lib/AST/Expr.cpp:3976 + if (const auto *D = dyn_cast<VarDecl>(ME1->getMemberDecl())) + if (D->isStaticDataMember()) + return true; ---------------- NoQ wrote: > Mmm, what are other cases when a `MemberExpr` doesn't have a `FieldDecl` as > its `getMemberDecl()`? Can this be turned into an `assert`? I had to look this one up too. From the docs for this method: ``` /// Retrieve the member declaration to which this expression refers. /// /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for /// static data members), a CXXMethodDecl, or an EnumConstantDecl. ValueDecl *getMemberDecl() const { return MemberDecl; } ``` So, FieldDecl, VarDecl, CXXMethodDecl, and EnumConstantDecl are all valid return types. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66045/new/ https://reviews.llvm.org/D66045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits