bugprone check for delete this

[EugeneZelenko]

Bugzilla Link	38741
Version	unspecified
OS	All
CC	@JonasToth,@m4tx

Extended Description

It'll be great to bugprone check for delete this in class methods.

[EugeneZelenko]

I'm not sure about present situation, but in past Static Analyzer worked only in single translation unit, so not all problematic cases could be detected.

[cd578596-8da0-4f5b-aaae-5d286920841a]

It seems that Clang Static Analyzer already handles this pretty well. For a class like so:

class Foo {
  int x;

public:
  void cleanup() { delete this; }

  void test1() {
    cleanup();
    test2();
  }

  void test2() {}
};

clang-tidy produces:

1 warning generated.
/tmp/tidytest/foo.cpp:13:5: warning: Use of memory after it is freed [clang-analyzer-cplusplus.NewDelete]
    test2();
    ^
[...]
/tmp/tidytest/foo.cpp:9:20: note: Memory is released
  void cleanup() { delete this; }
                   ^

Because of that, I believe we do not need to add another check, especially since without the usage of static analysis, we would need to implement some kind of heuristics to cover common use cases of delete this. Therefore, I think this ticket can be closed.

[JonasToth]

I added the beginner tag as this sounds not too complicated.

If someone tries to implement, don't hesitate to ask (here, IRC or mailing list) if you are stuck!

[EugeneZelenko]

assigned to @m4tx

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1) Assign the issue to you. 2) Fix the issue locally. 3) Run the test suite locally. 3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests. 4) Create a git commit 5) Run git clang-format HEAD~1 to format your changes. 6) Submit the patch to Phabricator. 6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

[lamarrr]

There are some valid use cases for "delete this" I. E. Intrusive ref-counting