* The isMainFile() check seems to be missing.
* The call to isDependentContext() basically causes all template classes to
be ignored, even really safe ones. For example, the following test fails:
struct A {
virtual void foo();
};
template <typename T>
struct B : public A {
virtual void foo();
// CHECK: struct B : public A {
// CHECK: virtual void foo() override;
};
B<int> b;
If this was your intent, I'd add this test to the non_risky_... XFAIL test.
* Could you log enhancements in bugzilla (the public C++11 Migrator JIRA
instance isn't quite ready) about fixing the two XFAIL tests and then mention
the PR numbers in the test comments?
================
Comment at: cpp11-migrate/AddOverride/AddOverrideActions.cpp:37
@@ +36,3 @@
+ if (M->hasAttr<OverrideAttr>())
+ return ;
+
----------------
No spaces between return and ;. I recommend using the clang-format tool on your
changes and it'll handle all the style stuff for you.
================
Comment at: cpp11-migrate/AddOverride/AddOverrideActions.cpp:57
@@ +56,3 @@
+ const FunctionDecl *Fn;
+ if (M->hasBody(Fn) && !Fn->isOutOfLine()) {
+ // Start at the beginning of the body and rewind back to the last
----------------
I'd recommend still using hasInlineBody() as it's clearer as to what this code
is for. What you have here is identical to the contents of that other function
except for the getTemplateInstantiationPattern() which should return NULL now
since you've updated what M is.
================
Comment at: cpp11-migrate/AddOverride/AddOverrideActions.cpp:52
@@ +51,3 @@
+ if (M->isDependentContext()) {
+ return ;
+ }
----------------
Style says no braces for single-line if control statements.
================
Comment at: cpp11-migrate/AddOverride/AddOverrideActions.cpp:46
@@ -56,1 +45,3 @@
+ if (const CXXMethodDecl *TemplateMethod = dyn_cast<CXXMethodDecl>(F)) {
+ M = TemplateMethod;
}
----------------
I think you can drop both sets of braces here without impacting readability.
================
Comment at: cpp11-migrate/AddOverride/AddOverrideActions.cpp:45
@@ +44,3 @@
+ if (const FunctionDecl *F = M->getTemplateInstantiationPattern()) {
+ if (const CXXMethodDecl *TemplateMethod = dyn_cast<CXXMethodDecl>(F)) {
+ M = TemplateMethod;
----------------
I think maybe I'd use cast<> here instead to cause an assertion failure if the
type is not CXXMethodDecl as I can't think of a situation where this should not
be the case.
http://llvm-reviews.chandlerc.com/D769
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
