shuaiwang added inline comments.
================
Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:34-38
+const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt, CXXTypeidExpr>
+ cxxTypeidExpr;
+
+const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt,
CXXNoexceptExpr>
+ cxxNoexceptExpr;
----------------
aaron.ballman wrote:
> I think these should be exposed as public AST matchers, as they're both
> generally useful. It can be done in a follow-up patch, or done before landing
> this patch, either is fine by me.
Will leave as follow up patch.
================
Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:67
+
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+ return selectFirst<Expr>(
----------------
aaron.ballman wrote:
> What about other unevaluated expressions, like `typeof`, `_Generic`, etc? You
> should search for `ExpressionEvaluationContext::Unevaluated` in Clang to see
> where we set up an unevaluated expression evaluation context to find all of
> the situations.
I checked around and I think these are all we care about:
- decltype/typeof
- sizeof/alignof
- noexcept
- _Generic
- typeid
I've added TODO for _Generic and typeid for now because I think those need a
bit more work to implement, while at the same time not supporting them for now
only creates false positives from isMutated which is what we prefer over false
negatives.
================
Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:72
+ findAll(expr(equalsNode(Exp),
+ anyOf(hasAncestor(typeLoc()),
+ hasAncestor(expr(anyOf(
----------------
aaron.ballman wrote:
> What is this trying to match with the `typeLoc()` matcher?
Added comment to explain.
================
Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:74-75
+ hasAncestor(expr(anyOf(
+ unaryExprOrTypeTraitExpr(),
+ cxxTypeidExpr(), cxxNoexceptExpr())))))
+ .bind("expr")),
----------------
aaron.ballman wrote:
> I think these are only approximations to testing whether it's unevaluated.
> For instance, won't this match these expressions, despite them being
> evaluated?
> ```
> // C++
> #include <typeinfo>
>
> struct B {
> virtual ~B() = default;
> };
>
> struct D : B {};
>
> B& get() {
> static B *b = new D;
> return *b;
> }
>
> void f() {
> (void)typeid(get()); // Evaluated, calls get()
> }
>
> /* C99 and later */
> #include <stddef.h>
>
> void f(size_t n) {
> (void)sizeof(int[++n]); // Evaluated, increments n
> }
> ```
Fixed handling of sizeof(VLA)
Added TODO for typeid
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45679
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
