[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 13 10:58:44 PDT 2018


JonasToth added a comment.

In https://reviews.llvm.org/D52008#1233667, @shuaiwang wrote:

> Just some quick comments, I'll take a deeper look into other comments later.
>
> This diff along unfortunately won't be able to handle `emplace_back` just yet, the reason (I believe, haven't fully tested) is that `std::forward` is not handled properly and almost all std functions involving forwarding references at least `std::forward`'ed once.
>  I have some more changes locally that treats `std::move` and `std::forward` just as casts and that should be able to really push the analysis further down the forwarding chain instead of stopping at `std::forward` call.
>  Rephased diff description to be more clear. Sorry for the confusion.


I did look up the rules for resolved the universal references.

  void f() {
    int x;
    std::move(x);
  }

Will create a normal reference to `x`. This would be the point were have to recursivly follow all references created from a value and check if they are mutated. Do you see another possibility on handling universal references correctly?



================
Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:60
+public:
+  FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context);
+
----------------
shuaiwang wrote:
> JonasToth wrote:
> > Why do we need a separate class for this?
> > I think you can just create another object of `ExprMutAnalyzer` and do the analysis with `findDeclMutation(FunctioParm)`.
> > 
> > The `Stmt` is the `functionDecl().getBody()`. Right now it could be that you pass in an functionDecl without body.
> > 
> > Could there something happen with extern templates that the body is not accessible?
> It's mostly for the special handling of constructors in which case initializer list also could mutate param.
Hmm, still why not within `ExprMutAnalyzer`?
You could make it a class living within ExprMutAnalyzer in the private section. Then its clear its an implementation detail.


Repository:
  rC Clang

https://reviews.llvm.org/D52008





More information about the cfe-commits mailing list