[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D91047#2407091 , @psionic12 wrote:

> @aaron.ballman That would be nice if your could help, and `Yafei Liu 
> ` is okay.

Thank you for the new example, I've commit on your behalf in 
2ce6352e46344d97fed6a3ca6de7228345956191 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added a comment.

@aaron.ballman That would be nice if your could help, and `Yafei Liu 
` is okay.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for the new plugin example! Do you need me to commit on your 
behalf? If so, are you okay with `Yafei Liu ` for 
attribution?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 306283.
psionic12 added a comment.

Simplify the test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify=callsuper %s
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -DBAD_CALLSUPER -fsyntax-only -Xclang -verify=badcallsuper %s
+// REQUIRES: plugins, examples
+
+// callsuper-no-diagnostics
+struct Base1 {
+  [[clang::call_super]] virtual void Test() {}
+};
+struct Base2 {
+  [[clang::call_super]] virtual void Test() {}
+};
+struct Derive : public Base1, public Base2 {
+#ifndef BAD_CALLSUPER
+  void Test() override;
+#else
+  [[clang::call_super]] virtual void Test() override final;
+  // badcallsuper-warning@16 {{'call_super' attribute marked on a final method}}
+#endif
+};
+void Derive::Test() {
+  Base1::Test();
+#ifndef BAD_CALLSUPER
+  Base2::Test();
+#else
+  // badcallsuper-warning@20 {{virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version}}
+  // badcallsuper-note@10 {{function marked 'call_super' here}}
+#endif
+}
+struct Derive2 : public Base1, public Base2 {
+  void Test() override {
+Base1::Test();
+Base2::Test();
+  }
+};
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,190 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Attribute plugin to mark a virtual method as ``call_super``, subclasses must
+// call it in the overridden method.
+//
+// This example shows that attribute plugins combined with ``PluginASTAction``
+// in Clang can do some of the same things which Java Annotations do.
+//
+// Unlike the other attribute plugin examples, this one does not attach an
+// attribute AST node to the declaration AST node. Instead, it keeps a separate
+// list of attributed declarations, which may be faster than using
+// ``Decl::getAttr()`` in some cases. The disadvantage of this approach is
+// that the attribute is not part of the AST, which means that dumping the AST
+// will lose the attribute information, pretty printing the AST won't write the
+// attribute back out to source, and AST matchers will not be able to match
+// against the attribute on the declaration.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet 
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:116
 
+Defining CallSuperAttr
+==

psionic12 wrote:
> After a whole day's research of `Sphinx`, I figured out that  
> `ClangPlugins.rst` is the "proto-type" of 
> https://clang.llvm.org/docs/ClangPlugins.html, which is the document on how 
> to use Clang plugin features.
> 
> This leads that my change in `ClangPlugins.rst` are not very appropriate, 
> sort of off-topic.
> 
> Sorry I don't notice the rest of the file, and misunderstood it as a 
> documents for examples.
> 
> I decide to move the illustration to the source code.
You're correct that the .rst file is the prototype for the public HTML 
documentation, sorry for not making that more clear sooner. I think it's fine 
to have the documentation either in the header file (as you've switched to) or 
within this file (since it is showing a slightly different way to define a 
plugin-based attribute).



Comment at: clang/test/Frontend/plugin-call-super.cpp:5
+
+#ifndef BAD_CALLSUPER
+// expected-no-diagnostics

Rather than duplicating the test logic like this, you can pass a custom prefix 
to `-verify` that gets used when checking the expected results. e.g.,
```
// RUN: %clang ... -verify=callsuper ...
// RUN: %clang ... -verify=nocallsuper -DBAD_CALLSUPER ...
// callsuper-no-diagnostics

...
void Derive::Test() { // nocallsuper-expected-warning {{virtual function ... }}
  Base1::Test();
#ifndef BAD_CALLSUPER
  Base2::Test();
#endif
}
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 marked an inline comment as done.
psionic12 added inline comments.



Comment at: clang/test/Frontend/plugin-call-super.cpp:18-19
+struct Derive2 : public Base1, public Base2 { void Test() override {  
Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: virtual function 'Base2::Test' is marked as 
'call_super' but this overriding method does not call the base version
+// BADCALLSUPER: note: function marked 'call_super' here
+#endif

aaron.ballman wrote:
> These warnings and notes (and the warning a few lines up) are ones I would 
> have expected to catch using `// expected-warning {{virtual function 
> 'Base2::Test' is marked as 'call_super' but this overriding method does not 
> call the base version}}` style checks instead of needing to use FileCheck.
> 
> Do plugin-based diagnostics not get caught by `-verify`? I expect this test 
> file to fail as currently written because of the `expected-no-diagnostics`, 
> but I've not done a whole lot of testing of plugins before.
`-verify` works well with plugins, I just tested, thanks for pointing out this 
elegant test way for syntax only features.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:116
 
+Defining CallSuperAttr
+==

After a whole day's research of `Sphinx`, I figured out that  
`ClangPlugins.rst` is the "proto-type" of 
https://clang.llvm.org/docs/ClangPlugins.html, which is the document on how to 
use Clang plugin features.

This leads that my change in `ClangPlugins.rst` are not very appropriate, sort 
of off-topic.

Sorry I don't notice the rest of the file, and misunderstood it as a documents 
for examples.

I decide to move the illustration to the source code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 305725.
psionic12 added a comment.

use `VerifyDiagnosticConsumer` (-verify) instead of `FileCheck` for syntax only 
feature test.
remove illustration in ClangPlugins.rst (which is not very appropriate)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify %s
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -DBAD_CALLSUPER -fsyntax-only -Xclang -verify %s
+// REQUIRES: plugins, examples
+
+#ifndef BAD_CALLSUPER
+// expected-no-diagnostics
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+#else
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[clang::call_super]] virtual void Test() override final; };
+// expected-warning@15 {{'call_super' attribute marked on a final method}}
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+// expected-warning@17 {{virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version}}
+// expected-note@14 {{function marked 'call_super' here}}
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,190 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Attribute plugin to mark a virtual method as ``call_super``, subclasses must
+// call it in the overridden method.
+//
+// This example shows that attribute plugins combined with ``PluginASTAction``
+// in Clang can do some of the same things which Java Annotations do.
+//
+// Unlike the other attribute plugin examples, this one does not attach an
+// attribute AST node to the declaration AST node. Instead, it keeps a separate
+// list of attributed declarations, which may be faster than using
+// ``Decl::getAttr()`` in some cases. The disadvantage of this approach is
+// that the attribute is not part of the AST, which means that dumping the AST
+// will lose the attribute information, pretty printing the AST won't write the
+// attribute back out to source, and AST matchers will not be able to match
+// against the attribute on the declaration.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == 

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

psionic12 wrote:
> aaron.ballman wrote:
> > psionic12 wrote:
> > > aaron.ballman wrote:
> > > > aaron.ballman wrote:
> > > > > The number of underlines here looks off -- can you verify it's 
> > > > > correct?
> > > > This still appears to be incorrect and will cause build errors for the 
> > > > documentation.
> > > Do you mean that there's a command to build the documentation and 
> > > currently my patch will cause a failure on it?
> > > 
> > > I thought this ClangPlugins.rst is only an documentation with markdown, 
> > > but seems that it's not what I thought?
> > > 
> > > Currently I will make sure there's no build error on the plugin itself 
> > > and the regression test case, and make sure the
> > > regression test will pass. Seems that's not enough, right?
> > > Do you mean that there's a command to build the documentation and 
> > > currently my patch will cause a failure on it?
> > 
> > Yes, we have a bot that builds docs: http://lab.llvm.org:8011/#/builders/92
> > 
> > > I thought this ClangPlugins.rst is only an documentation with markdown, 
> > > but seems that it's not what I thought?
> > 
> > It is a documentation file with markdown, but the markdown bots will 
> > complain if a markdown file cannot be built (they treat markdown warnings 
> > as errors).
> > 
> > > Currently I will make sure there's no build error on the plugin itself 
> > > and the regression test case, and make sure the regression test will 
> > > pass. Seems that's not enough, right?
> > 
> > Most of the time that's enough because the markdown usage is pretty trivial 
> > and can be inspected by sight for obvious issues (so people don't typically 
> > have to set up their build environments to generate documentation and test 
> > it with every patch).
> > 
> > In this case, the issue is with the underlines under the title. The number 
> > of underlines needs to match the number of characters in the title, but in 
> > this case there are 20 `=` characters but 23 title characters.
> It seems that only a committee member can trigger this bot, any way I can 
> test on my own environment? So that I can make sure the doc will compile 
> successfully before uploading patches?
> 
> As I mentioned before, using `make doxygen-clang` will succeed even the `=` 
> characters are not match with title characters, so it seems that the bot 
> doesn't use this way.
You can test in your own environment (if a build bot can do it, so can anyone 
else). I don't typically build docs locally so my instructions are untested, 
but I think you need to set the following cmake variables:

```
LLVM_BUILD_DOCS=YES ' enables building docs
LLVM_ENABLE_SPHINX=YES ' enable building sphinx docs (.rst files)
```
You also have to have Sphinx installed (you can do this using `pip install -U 
Sphinx`) and on the PATH before you configure cmake for Clang.

Doxygen is a different documentation format than Sphinx. Doxygen handles 
building the docs from comments in the source files. Sphinx is what turns these 
.rst files into HTML. If you enable Sphinx builds from cmake there should be a 
target added that lets you build those docs.



Comment at: clang/test/Frontend/plugin-call-super.cpp:5
+
+// expected-no-diagnostics
+struct Base1 { [[clang::call_super]] virtual void Test() {} };

Hmm, this seems wrong to me -- we do expect diagnostics from this file.



Comment at: clang/test/Frontend/plugin-call-super.cpp:18-19
+struct Derive2 : public Base1, public Base2 { void Test() override {  
Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: virtual function 'Base2::Test' is marked as 
'call_super' but this overriding method does not call the base version
+// BADCALLSUPER: note: function marked 'call_super' here
+#endif

These warnings and notes (and the warning a few lines up) are ones I would have 
expected to catch using `// expected-warning {{virtual function 'Base2::Test' 
is marked as 'call_super' but this overriding method does not call the base 
version}}` style checks instead of needing to use FileCheck.

Do plugin-based diagnostics not get caught by `-verify`? I expect this test 
file to fail as currently written because of the `expected-no-diagnostics`, but 
I've not done a whole lot of testing of plugins before.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 marked 6 inline comments as done.
psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

aaron.ballman wrote:
> psionic12 wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > The number of underlines here looks off -- can you verify it's correct?
> > > This still appears to be incorrect and will cause build errors for the 
> > > documentation.
> > Do you mean that there's a command to build the documentation and currently 
> > my patch will cause a failure on it?
> > 
> > I thought this ClangPlugins.rst is only an documentation with markdown, but 
> > seems that it's not what I thought?
> > 
> > Currently I will make sure there's no build error on the plugin itself and 
> > the regression test case, and make sure the
> > regression test will pass. Seems that's not enough, right?
> > Do you mean that there's a command to build the documentation and currently 
> > my patch will cause a failure on it?
> 
> Yes, we have a bot that builds docs: http://lab.llvm.org:8011/#/builders/92
> 
> > I thought this ClangPlugins.rst is only an documentation with markdown, but 
> > seems that it's not what I thought?
> 
> It is a documentation file with markdown, but the markdown bots will complain 
> if a markdown file cannot be built (they treat markdown warnings as errors).
> 
> > Currently I will make sure there's no build error on the plugin itself and 
> > the regression test case, and make sure the regression test will pass. 
> > Seems that's not enough, right?
> 
> Most of the time that's enough because the markdown usage is pretty trivial 
> and can be inspected by sight for obvious issues (so people don't typically 
> have to set up their build environments to generate documentation and test it 
> with every patch).
> 
> In this case, the issue is with the underlines under the title. The number of 
> underlines needs to match the number of characters in the title, but in this 
> case there are 20 `=` characters but 23 title characters.
It seems that only a committee member can trigger this bot, any way I can test 
on my own environment? So that I can make sure the doc will compile 
successfully before uploading patches?

As I mentioned before, using `make doxygen-clang` will succeed even the `=` 
characters are not match with title characters, so it seems that the bot 
doesn't use this way.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:104
+for (const auto *Method : MarkedMethods) {
+  lateDiagAppertainsToDecl(Diags, Method);
+}

Move the `lateDiagAppertainsToDecl()` here and only check marked functions.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:69
+  bool VisitCXXMethodDecl(CXXMethodDecl *MethodDecl) {
+lateDiagAppertainsToDecl(MethodDecl);
+

Call `lateDiagAppertainsToDecl` in every `VisitCXXMethodDecl` functions is not 
very efficiency, since marked methods are already saved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 305405.
psionic12 added a comment.

Fix the grammar
Simplify the code logic
Simplify the test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify %s
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+#ifdef BAD_CALLSUPER
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[clang::call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: 'call_super' attribute marked on a final method
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version
+// BADCALLSUPER: note: function marked 'call_super' here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,178 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This Clang plugin checks that overrides of the marked virtual function
+// directly call the marked function.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet 
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine ) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"virtual function %q0 is marked as 'call_super' but this overriding "
+"method does not call the base version");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(
+DiagnosticsEngine::Note, "function marked 'call_super' here");
+  }
+  

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

psionic12 wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > The number of underlines here looks off -- can you verify it's correct?
> > This still appears to be incorrect and will cause build errors for the 
> > documentation.
> Do you mean that there's a command to build the documentation and currently 
> my patch will cause a failure on it?
> 
> I thought this ClangPlugins.rst is only an documentation with markdown, but 
> seems that it's not what I thought?
> 
> Currently I will make sure there's no build error on the plugin itself and 
> the regression test case, and make sure the
> regression test will pass. Seems that's not enough, right?
> Do you mean that there's a command to build the documentation and currently 
> my patch will cause a failure on it?

Yes, we have a bot that builds docs: http://lab.llvm.org:8011/#/builders/92

> I thought this ClangPlugins.rst is only an documentation with markdown, but 
> seems that it's not what I thought?

It is a documentation file with markdown, but the markdown bots will complain 
if a markdown file cannot be built (they treat markdown warnings as errors).

> Currently I will make sure there's no build error on the plugin itself and 
> the regression test case, and make sure the regression test will pass. Seems 
> that's not enough, right?

Most of the time that's enough because the markdown usage is pretty trivial and 
can be inspected by sight for obvious issues (so people don't typically have to 
set up their build environments to generate documentation and test it with 
every patch).

In this case, the issue is with the underlines under the title. The number of 
underlines needs to match the number of characters in the title, but in this 
case there are 20 `=` characters but 23 title characters.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:9-10
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//

How about: `This Clang plugin checks that overrides of the marked virtual 
function directly call the marked function.`



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:80
+
+  // Now find if supper is used in `MethodDecl`.
+  MethodUsageVisitor Visitor(OverriddenMarkedMethods);

supper is used -> the superclass method is called



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:84
+  // After traversing, all methods left in `OverriddenMarkedMethods`
+  // are not get called, warning about these.
+  for (const auto  : OverriddenMarkedMethods) {

are not get called, warning -> are not called, warn



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:152
+  S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
+  << Attr << "methods";
+  return false;

Instead of `methods`, how about `virtual functions`? Then the logic can be: `if 
(!TheMethod || !TheMethod->isVirtual())` and you can remove the diagnostic 
below.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:166-168
+// No need to add an attr object (usually an annotation attr is added)
+// save the address of the Decl in sets, it maybe faster than compare to
+// strings.

attr is added) save the address of the Decl in sets -> .attr is added). Save 
the address of the Decl in a set



Comment at: clang/test/Frontend/plugin-call-super.cpp:1-2
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s 
-o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm 
-DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples

The way we would typically test something like this is to pass `-fsyntax-only` 
and `-verify` to verify the diagnostics rather than using FileCheck, because 
the plugin doesn't have any impact on code generation (just diagnostics). This 
makes the test faster to check (and is a bit easier than using FileCheck).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added a comment.

I just enabled `LLVM_ENABLE_DOXYGEN` `LLVM_BUILD_DOCS`, and use `make 
doxygen-clang`, it seems no error with my newest patch, is this

> This still appears to be incorrect and will cause build errors for the 
> documentation.

use the same way?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 marked 5 inline comments as done.
psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

aaron.ballman wrote:
> aaron.ballman wrote:
> > The number of underlines here looks off -- can you verify it's correct?
> This still appears to be incorrect and will cause build errors for the 
> documentation.
Do you mean that there's a command to build the documentation and currently my 
patch will cause a failure on it?

I thought this ClangPlugins.rst is only an documentation with markdown, but 
seems that it's not what I thought?

Currently I will make sure there's no build error on the plugin itself and the 
regression test case, and make sure the
regression test will pass. Seems that's not enough, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 305011.
psionic12 marked an inline comment as done.
psionic12 added a comment.

fix grammer errors in warning messages and documents


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[clang::call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: 'call_super' attribute marked on a final method
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// CALLSUPER: [[STR1_VAR:@.+]] = dso_local constant [8 x i8] c"6Derive\00", align 1
+#ifdef BAD_CALLSUPER
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version
+// BADCALLSUPER: note: function marked 'call_super' here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,178 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet 
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine ) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"virtual function %q0 is marked as 'call_super' but this overriding "
+"method does not call the base version");
+

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I have more review to do on this but have to run for a while and wanted to get 
you this feedback sooner rather than later.




Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as `call_super`, subclasses must 
call it
+in the overridden method.

Sorry, I was unclear, it should be *double* backticks around syntax elements 
like `call_super`, not single backticks.



Comment at: clang/docs/ClangPlugins.rst:122-123
+
+This example shows the possibility that attribute plugins combine with 
PluginASTAction
+in Clang can do same things which Java Annotations do.
+

Slight tweaking for grammar:

`This example shows that attribute plugins combined with PluginASTAction in 
Clang can do some of the same things which Java Annotations do.`

(with double backticks around `PluginASTAction`)



Comment at: clang/docs/ClangPlugins.rst:125
+
+Not like other attribute plugin examples, this one do not attached any 
attribute nodes
+to a declaration node, instead, saving the declaration addresses directly, 
which may

Slight tweaking for grammar:

```
Unlike the other attribute plugin examples, this one does not attach an 
attribute AST node to the declaration AST node. Instead, it keeps a separate 
list of attributed declarations, which may be faster than using 
Decl::getAttr() in some cases. The disadvantage of this approach is that the 
attribute is not part of the AST, which means that dumping the AST will lose 
the attribute information, pretty printing the AST won't write the attribute 
back out to source, and AST matchers will not be able to match against the 
attribute on the declaration.
```
(with double backticks around `Decl::getAttr()`)



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

aaron.ballman wrote:
> The number of underlines here looks off -- can you verify it's correct?
This still appears to be incorrect and will cause build errors for the 
documentation.



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

psionic12 wrote:
> psionic12 wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > Should add backticks around `call_super` since it's syntax. Also, 
> > > > `sub-classes` should be `subclasses`.
> > > > 
> > > > "call it in overridden the method" -> "call it in the overridden method"
> > > There should be more explanation here about what concepts the example 
> > > demonstrates. For example, one interesting bit to me is that it shows how 
> > > you can add a custom attribute that's useful even if it does not generate 
> > > an AST node for the attribute.
> > "how you can add a custom attribute that's useful even if it does not 
> > generate an AST node for the attribute", do you mean I should add an 
> > Annotation Attribute object even I don't use it? So that when someone dumps 
> > the AST, the `call_super` attribute will show?
> > 
> > Or do you mean to explain the inner implementation of how could the 
> > RecursiveASTVisitor knows the declaration is marked as `call_super`, even 
> > if I didn't add any extra attribute nodes to this declaration node?
> I got you point, please ignore the comment above.
> 
> Since this is an example, I should mention more about this example itself 
> rather than how to use this plugin, right?
> Since this is an example, I should mention more about this example itself 
> rather than how to use this plugin, right?

Exactly!



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:63
+DiagnosticsEngine::Warning,
+"%0 is marked as call_super but override method %1 does not call it");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(

psionic12 wrote:
> aaron.ballman wrote:
> > Should put single quotes around `call_super` as it's a syntactic element.
> > 
> > I'm not certain that %0 and %1 are necessary because the overridden methods 
> > will have the same names. I think you can drop the %1 and rely on the note 
> > to point out where the overridden method lives.
> > 
> About the '%0' and '%1' problem, same as the comment about "fullName()" 
> function, since the overridden methods have the same name, I want use 
> "class_name::method_name" to distinguish between these methods to make users 
> clear.
> 
> It would very nice if you could help to come up with a decent warning message 
> which is better than this, I tried some but none of them give a 
> compiler-warning-style feeling... 
> It would very nice if you could help to come up with a decent warning message 
> which is better than this, I tried some but none of them give a 
> compiler-warning-style feeling...

How about:

`virtual function %q0 is 

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:133
+  PluginASTAction::ActionType getActionType() override {
+return AddBeforeMainAction;
+  }

`AddAfterMainAction` will get run after IR codes were generated, I found this 
when writing test cases, so it would be better to use `AddBeforeMainAction`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 304129.
psionic12 added a comment.

Add more detail in ClangPlugins.rst.
Re-design overridden usage checking.
Fix some code style issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[clang::call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: 'call_super' attribute marked on a final method
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// CALLSUPER: [[STR1_VAR:@.+]] = dso_local constant [8 x i8] c"6Derive\00", align 1
+#ifdef BAD_CALLSUPER
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: 'Test' is marked as 'call_super' but override method does not call it
+// BADCALLSUPER: note: overridden method marked 'call_super' here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,177 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet 
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine ) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"%0 is marked as 'call_super' but override method does not call it");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(
+DiagnosticsEngine::Note, "overridden method 

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

psionic12 wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > Should add backticks around `call_super` since it's syntax. Also, 
> > > `sub-classes` should be `subclasses`.
> > > 
> > > "call it in overridden the method" -> "call it in the overridden method"
> > There should be more explanation here about what concepts the example 
> > demonstrates. For example, one interesting bit to me is that it shows how 
> > you can add a custom attribute that's useful even if it does not generate 
> > an AST node for the attribute.
> "how you can add a custom attribute that's useful even if it does not 
> generate an AST node for the attribute", do you mean I should add an 
> Annotation Attribute object even I don't use it? So that when someone dumps 
> the AST, the `call_super` attribute will show?
> 
> Or do you mean to explain the inner implementation of how could the 
> RecursiveASTVisitor knows the declaration is marked as `call_super`, even if 
> I didn't add any extra attribute nodes to this declaration node?
I got you point, please ignore the comment above.

Since this is an example, I should mention more about this example itself 
rather than how to use this plugin, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

aaron.ballman wrote:
> aaron.ballman wrote:
> > Should add backticks around `call_super` since it's syntax. Also, 
> > `sub-classes` should be `subclasses`.
> > 
> > "call it in overridden the method" -> "call it in the overridden method"
> There should be more explanation here about what concepts the example 
> demonstrates. For example, one interesting bit to me is that it shows how you 
> can add a custom attribute that's useful even if it does not generate an AST 
> node for the attribute.
"how you can add a custom attribute that's useful even if it does not generate 
an AST node for the attribute", do you mean I should add an Annotation 
Attribute object even I don't use it? So that when someone dumps the AST, the 
`call_super` attribute will show?

Or do you mean to explain the inner implementation of how could the 
RecursiveASTVisitor knows the declaration is marked as `call_super`, even if I 
didn't add any extra attribute nodes to this declaration node?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:32
+
+std::string fullName(const CXXMethodDecl *D) {
+  std::string FullName;

aaron.ballman wrote:
> Is the functionality for getting names out of a `NamedDecl` insufficient?
`fullName()` here is used to get a string which format is 
"class_name::method_name", I think NamedDecl::getName or 
NamedDecl::getNameAsString only gets the method name, without the class name.

Or is there an easy way to accomplish this?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:63
+DiagnosticsEngine::Warning,
+"%0 is marked as call_super but override method %1 does not call it");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(

aaron.ballman wrote:
> Should put single quotes around `call_super` as it's a syntactic element.
> 
> I'm not certain that %0 and %1 are necessary because the overridden methods 
> will have the same names. I think you can drop the %1 and rely on the note to 
> point out where the overridden method lives.
> 
About the '%0' and '%1' problem, same as the comment about "fullName()" 
function, since the overridden methods have the same name, I want use 
"class_name::method_name" to distinguish between these methods to make users 
clear.

It would very nice if you could help to come up with a decent warning message 
which is better than this, I tried some but none of them give a 
compiler-warning-style feeling... 



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:90
+Diags.Report(MethodDecl->getLocation(), WarningSuperNotCalled)
+<< fullName(Overridden) << fullName(MethodDecl);
+Diags.Report(Overridden->getLocation(),

aaron.ballman wrote:
> FWIW, you can pass `Overridden` and `MethodDecl` in directly rather than 
> trying to scrape the name out yourself -- the diagnostics engine knows how to 
> properly turn a `NamedDecl` into a string for diagnostics.
Same problem, a decent warning message is welcome, I don't like this 
scrape-name-thing either. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

The number of underlines here looks off -- can you verify it's correct?



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

Should add backticks around `call_super` since it's syntax. Also, `sub-classes` 
should be `subclasses`.

"call it in overridden the method" -> "call it in the overridden method"



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

aaron.ballman wrote:
> Should add backticks around `call_super` since it's syntax. Also, 
> `sub-classes` should be `subclasses`.
> 
> "call it in overridden the method" -> "call it in the overridden method"
There should be more explanation here about what concepts the example 
demonstrates. For example, one interesting bit to me is that it shows how you 
can add a custom attribute that's useful even if it does not generate an AST 
node for the attribute.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:25
+namespace {
+// cached methods which are marked as call_super
+llvm::SmallPtrSet ForcingCalledMethods;

Comments should be grammatical including starting with a capital letter and 
trailing punctuation. (Should be corrected elsewhere in the patch as well.)



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:32
+
+std::string fullName(const CXXMethodDecl *D) {
+  std::string FullName;

Is the functionality for getting names out of a `NamedDecl` insufficient?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:43
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}

ctor should probably be `explicit`



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:43
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}

aaron.ballman wrote:
> ctor should probably be `explicit`
Can drop `clang::` prefixed everywhere since you have a `using` declaration at 
the top of the file.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:63
+DiagnosticsEngine::Warning,
+"%0 is marked as call_super but override method %1 does not call it");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(

Should put single quotes around `call_super` as it's a syntactic element.

I'm not certain that %0 and %1 are necessary because the overridden methods 
will have the same names. I think you can drop the %1 and rely on the note to 
point out where the overridden method lives.




Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:66
+DiagnosticsEngine::Note,
+"overridden method which is marked as call_super here");
+  }

Single quotes around `call_super` here as well; we usually phrase this sort of 
thing as `"overridden method marked 'call_super' here`



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:73
+  DiagnosticsEngine::Warning,
+  "call_super attribute attached on a final method");
+  Diags.Report(MethodDecl->getLocation(), ID);

Single quotes around `call_super` and we should use consistent terminology for 
"attached" vs "marked".



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:76
+}
+for (const auto *Overridden : MethodDecl->overridden_methods()) {
+  if (isMarkedAsCallSuper(Overridden)) {

Won't this visit every method declaration multiple times? Once for the 
declaration itself (as we encounter it) and once for each time it's listed as 
an overridden method?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:80
+// skip checking
+if (!MethodDecl->isThisDeclarationADefinition()) {
+  continue;

We typically elide the braces for single-line substatements.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:90
+Diags.Report(MethodDecl->getLocation(), WarningSuperNotCalled)
+<< fullName(Overridden) << fullName(MethodDecl);
+Diags.Report(Overridden->getLocation(),

FWIW, you can pass `Overridden` and `MethodDecl` in directly rather than trying 
to scrape the name out yourself -- the diagnostics engine knows how to properly 
turn a `NamedDecl` into a 

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.



Comment at: clang/test/Frontend/plugin-call-super.cpp:7
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[call_super]] virtual void 
Test() override final; };
+// CALLSUPER: warning: call_super attribute attached on a final method

If I remove the keyword "virtual" here, the isVirtual() will return false.

To my knowledge "final" means this method cannot be overridden, but it is still 
a virtual method, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 303772.
psionic12 added a comment.

add final attribute checking


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91047/new/

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: call_super attribute attached on a final method
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// CALLSUPER: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
+// CALLSUPER: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
+// CALLSUPER: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
+
+#ifdef BAD_CALLSUPER
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: Base2::Test is marked as call_super but override method Derive::Test does not call it
+// BADCALLSUPER: note: overridden method which is marked as call_super here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,170 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// cached methods which are marked as call_super
+llvm::SmallPtrSet ForcingCalledMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // use this way to avoid add an annotation attr to the AST.
+  return ForcingCalledMethods.contains(D);
+}
+
+std::string fullName(const CXXMethodDecl *D) {
+  std::string FullName;
+  FullName += D->getParent()->getName();
+  FullName += "::";
+  FullName += D->getName();
+  return FullName;
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}
+  bool VisitCallExpr(clang::CallExpr *CallExpr) {
+if (CallExpr->getCalleeDecl() == MethodDecl) {
+  // super is called
+  IsOverriddenUsed = true;
+  return false;
+}
+return true;
+  }
+
+private:
+  const clang::CXXMethodDecl *MethodDecl;
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine ) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"%0 is 

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 created this revision.
psionic12 added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
psionic12 requested review of this revision.

If a virtual method is marked as call_super, the
override method must call it, simpler feature like @CallSuper
in Android Java.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// CALLSUPER: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
+// CALLSUPER: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
+// CALLSUPER: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
+
+#ifdef BAD_CALLSUPER
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: Base2::Test is marked as call_super but override method Derive::Test does not call it
+// BADCALLSUPER: note: overridden method which is marked as call_super here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,153 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+
+llvm::SmallPtrSet ForcingCalledMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  return ForcingCalledMethods.contains(D);
+}
+
+std::string FullName(const CXXMethodDecl *D) {
+  std::string FullName;
+  FullName += D->getParent()->getName();
+  FullName += "::";
+  FullName += D->getName();
+  return FullName;
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOvrriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}
+  bool VisitCallExpr(clang::CallExpr *CallExpr) {
+if (CallExpr->getCalleeDecl() == MethodDecl) {
+  IsOvrriddenUsed = true;
+  return false;
+}
+return true;
+  }
+
+private:
+  const clang::CXXMethodDecl *MethodDecl;
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine ) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"%0 is marked as call_super but override method %1 does not call it");
+