[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

[Nicholas Allegra via Phabricator via cfe-commits]

comex created this revision.
comex added a reviewer: dblaikie.
Herald added subscribers: cfe-commits, dmgreen, kristof.beyls.
Herald added a project: clang.

Currently, `handleCall` takes a `CallExpr` as an argument and retrieves the arguments from there.  This changes it to take the argument list as a separate argument of type `ArrayRef`.

The main motivation that I want `VisitCXXConstructExpr` to also be able to invoke `handleCall`, even though `CXXConstructExpr` is not a subclass of `CallExpr`.  But actually adding that will wait for a future patch.

However, this also fixes a minor bug in `VisitCXXOperatorCallExpr`:

  if (const auto *MCall = dyn_cast<CXXMemberCallExpr>(Call))
    handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl);
  else
    handleCall(Call, Call->getArg(0), FunDecl);

`Call` here is a `CXXOperatorCallExpr`, and `CXXMemberCallExpr` is a sibling class, so the cast will never succeed.  CXXMemberCallExprs go through a separate visitor, `VisitCXXMemberCallExpr`.

On the other hand, this logic may be intended to reflect the fact that C++ operators can be declared as either methods or free functions.  The correct way to differentiate these is shown at the beginning of handleCall:

  unsigned Offset = 0;
  if (isa<CXXOperatorCallExpr>(Call) && isa<CXXMethodDecl>(FunD))
    Offset = 1;  // first argument is 'this'

For `CXXOperatorCallExpr`s, if the decl is a `CXXMethodDecl`, the first argument is `this`; otherwise there is no `this`.

Going back to the other first: currently, since the `dyn_cast` always fails, it always passes `Call->getArg(0)` as `ObjArg` (i.e. the expression representing `this`), even for operators declared as free functions.  However, this is harmless, because `ObjArg` is only used if the function is marked as one of `set_typestate` or `test_typestate`, or `callable_when`, yet those attributes are only allowed on methods.  `Call->getArg(0)` will crash if there are zero arguments, but the only kind of non-method operator function allowed to have zero arguments is a user-defined literal, and those do not produce `CXXOperatorCallExpr`s.

The bug could be fixed by changing the first snippet to check for `CXXMethodDecl` like the second one, but this refactor makes things cleaner by only having to check in one place.


Repository:
  rC Clang

https://reviews.llvm.org/D67647

Files:
  lib/Analysis/Consumed.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67647.220421.patch
Type: text/x-patch
Size: 3779 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190917/75bce856/attachment.bin>