Fly-by thoughts
(one other minor thought, though we have no consistent strategy on this: what
about suppressing the warning in other test files rather than testing it? (I
know there's certainly a lot of incidental testing we do in other test files
like this))
================
Comment at: lib/Sema/SemaDeclCXX.cpp:4248
@@ +4247,3 @@
+ // Also avoid issuing this diagnostic if not in C++11 mode.
+ if (getLangOpts().CPlusPlus11 && Record->hasOverrideAnnotation())
+ CheckMissingOverrideSpecifier(*this, *M);
----------------
Presumably we want to issue this in C++14, etc. So perhaps "not C++98"?
& also - if you only need to test "hasOverrideAnnotation" here, why not just
make it a free function, rather than having to spend a bit in the
CXXRecordDecl's data? (I haven't looked in detail, but I assume that's what
you're doing)
================
Comment at: test/Misc/diag-override.cpp:9
@@ +8,3 @@
+ virtual void annotated_override() override;
+ // expected-error@+1 {{does not override any member functions}}
+ virtual void not_overriding_anything() override;
----------------
Presumably we already have test coverage for this, don't we? (override on a
non-overriding method) It's best not to duplicate test coverage as it just adds
maintenance burden when we make code changes (if we change the diagnostic text,
for example)
================
Comment at: test/FixIt/fixit-cxx0x.cpp:36
@@ -35,3 +35,3 @@
F f1,
- f2 final,
+ f2 final, // expected-warning {{but is not marked 'override'}}
f3 override, // expected-error {{expected ';' at end of declaration}}
----------------
Here are some thoughts (possibly wrong):
1) should we let 'final' imply 'override' (I know it's not necessarily true,
'final' could be placed at the start of an inheritance hierarchy)
2) should we use the presence of 'final' to imply that the user wanted to use
explicit 'override' (maybe not, I suppose - someone could be using one feature
without the other consistently)
http://llvm-reviews.chandlerc.com/D1275
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
