using __attribute__((override)) in C++

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

using __attribute__((override)) in C++

Evan Martin
Hello,

MSVC, at least since 2005, supports an "override" keyword on virtual
methods.  It produces a compiler error when you tag a function with
the keyword but fail to override a superclass method.  It would've
been handy to catch some troublesome typos in the past.  (The worst is
when you change the type in a common superclass and then miss
mirroring the change in one of a set of subclasses!)
  http://msdn.microsoft.com/en-us/library/41w3sh1c(v=VS.80).aspx

It turns out that clang already parses __attribute__((override)), in
the same position as MSVC's "override", so it was pretty easy to make
a tiny clang plugin that produces a similar error.  I'll inline the
code below.

dgregor on IRC suggested I should be adding this to Sema.  I would be
happy to contribute this if anyone would give me a pointer on where it
should plug in.  Today is the first time I've really looked at the
clang code and I don't really know what I'm doing (as the following
code likely demonstrates).  It looks like "coppro" added the original
support, so perhaps they could advise if they're on this mailing list.

PS: I don't really follow C++0x but my vague impression is that it
might have a different syntax or semantics for this keyword.  If
that's will be an issue with this please let me know.

PPS: This also prints out a warning to help me find places in my code
to add the keyword.  I used "OVERRIDE" in my message because we
#define OVERRIDE in our tree to the appropriate compiler-specific
value.


$ cat Overrides.cpp
#include "clang/Frontend/FrontendPluginRegistry.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/AST.h"
#include "clang/Frontend/CompilerInstance.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;

namespace {

class OverrideConsumer : public ASTConsumer {
public:
  virtual void HandleTagDeclDefinition(TagDecl* tag) {
    if (CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(tag)) {
      for (CXXRecordDecl::method_iterator method = record->method_begin();
           method != record->method_end(); ++method) {
        if (method->isVirtual()) {
          bool has_override_attr = method->getAttr<OverrideAttr>();
          bool is_override = method->begin_overridden_methods() !=
method->end_overridden_methods();

          if (is_override && !has_override_attr) {
            Diagnostic& D = method->getASTContext().getDiagnostics();
            unsigned id = D.getCustomDiagID(Diagnostic::Warning,
                "method overrides parent class; consider adding OVERRIDE");
            D.Report(method->getASTContext().getFullLoc(method->getLocation()),
id);
          } else if (has_override_attr && !is_override) {
            Diagnostic& D = method->getASTContext().getDiagnostics();
            unsigned id = D.getCustomDiagID(Diagnostic::Error,
                "method claims OVERRIDE but fails to override any
superclass method");
            D.Report(method->getASTContext().getFullLoc(method->getLocation()),
id);
          }
        }
      }
    }
  }
};

class CheckOverrideAction : public PluginASTAction {
protected:
  ASTConsumer *CreateASTConsumer(CompilerInstance &CI, llvm::StringRef) {
    return new OverrideConsumer();
  }

  bool ParseArgs(const CompilerInstance &CI,
                 const std::vector<std::string>& args) {
    return true;
  }
};

}

static FrontendPluginRegistry::Add<CheckOverrideAction>
X("check-overrides", "check usage of __attribute__((override))");
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: using __attribute__((override)) in C++

Eli Friedman
On Fri, Oct 8, 2010 at 3:50 PM, Evan Martin <[hidden email]> wrote:

> Hello,
>
> MSVC, at least since 2005, supports an "override" keyword on virtual
> methods.  It produces a compiler error when you tag a function with
> the keyword but fail to override a superclass method.  It would've
> been handy to catch some troublesome typos in the past.  (The worst is
> when you change the type in a common superclass and then miss
> mirroring the change in one of a set of subclasses!)
>  http://msdn.microsoft.com/en-us/library/41w3sh1c(v=VS.80).aspx
>
> It turns out that clang already parses __attribute__((override)), in
> the same position as MSVC's "override", so it was pretty easy to make
> a tiny clang plugin that produces a similar error.  I'll inline the
> code below.
>
> dgregor on IRC suggested I should be adding this to Sema.  I would be
> happy to contribute this if anyone would give me a pointer on where it
> should plug in.  Today is the first time I've really looked at the
> clang code and I don't really know what I'm doing (as the following
> code likely demonstrates).

Try Sema::CheckFunctionDeclaration in SemaDecl.cpp.

> PS: I don't really follow C++0x but my vague impression is that it
> might have a different syntax or semantics for this keyword.  If
> that's will be an issue with this please let me know.

"override" isn't a keyword in C++0x; it's the name of an attribute.
And it does in fact have the same semantics as the MSVC keyword.

> PPS: This also prints out a warning to help me find places in my code
> to add the keyword.  I used "OVERRIDE" in my message because we
> #define OVERRIDE in our tree to the appropriate compiler-specific
> value.

A similar warning would be okay to put into Sema, assuming it's opt-in.

-Eli

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev