Proposal to make rtti errors more generic.

[Sunil_Srivastava]

Summary

This patch changes the wording of two errors to make them more generic.

An attempt to use dynamic_cast while rtti is disabled, curently emits the error:

cannot use dynamic_cast with -fno-rtti

and a similar one for typeid.

This patch proposes to change that to:

use of dynamic_cast requires enabling RTTI

For targets where the default mode of RTTI is disabled, the current error
message is confusing because the user never used the -fno-rtti option.

This proposal is motivated by the PS4 compiler, whose default is to have RTTI
disabled. However, it is just as clear as the existing diagnostic, and it
may be applicable to other llvm compilers having the have the same default as the
PS4 compiler. It is also more appropriate in cases where the spelling of the
argument (to disable RTTI) is something other than -fno-rtti (for
example, /GR- is the switch to disable RTTI for cl).

[filcab]

include/clang/Basic/DiagnosticSemaKinds.td

6729

I'd prefer to have the way to enable RTTI mentioned in the message. Could we have something like ToolChain::getRTTIMode() and have a "RTTI was on/of" or "RTTI defaulted to on/off"? That way we'd be able to have a message similar to the current one (mentioning "you passed -fno-rtti") on platforms that default to RTTI=on, and have your updated message (possibly with a mention of "use -frtti") on platforms that default to RTTI=off.

(This is a minor usability comment about this patch, I don't consider it a blocker or anything)

[probinson]

include/clang/Basic/DiagnosticSemaKinds.td

6729

If the options are spelled differently for clang-cl and we had a way to retrieve the appropriate spellings, providing the option to use in the diagnostic does seem like a nice touch.

[Sunil_Srivastava]

include/clang/Basic/DiagnosticSemaKinds.td

6729

The idea of suggestion as to how-to-turn-on-rtti is appealing, but it is not trivial.

First, clang-cl does not give this warning at all, so the issue is moot for clang-cl.

For unix-line command-line, if the default RTTI mode in ENABLED (the unknown-linux)
then this warning appears only if the user gives -fno-rtti options, so again we do
not need to say anything more.

The only cases left are compilers a where the default RTTI mode is DISABLED.
Here an addendum like '[-frtti]' may make sense. AFAIK, PS4 is the only compiler
with this default, but there may be other such private compilers.

So should we append '[-frtti]' if Target.getTriple().isPS4() is true?

[wristow]

include/clang/Basic/DiagnosticSemaKinds.td

6729

So should we append '[-frtti]' if Target.getTriple().isPS4() is true?

Personally, I'd be OK with producing a suggestion of how to enable RTTI based on the PS4 triple. But I'd also be OK with not enhancing this diagnostic to suggest how to turn on RTTI (that is, going with the patch as originally proposed here).

If clang-cl produced a warning when a dynamic_cast or typeid construct was encountered in /GR- mode, then I'd feel it's worth complicating the code to provide a target-sensitive way for advising the user how to turn RTTI on. But clang-cl doesn't produce a warning in that case, so the effort to add the framework for producing a target-sensitive warning doesn't seem worth it to me.

Improving clang-cl to produce a diagnostic in this /GR- situation seems like a good idea, but separate from this proposed patch. If that work gets done at some point, then it would be natural to revisit this diagnostic at that time.

[probinson]

include/clang/Basic/DiagnosticSemaKinds.td

6729

If clang-cl is not a consideration, then I think the easiest and clearest way to do this is simply to say requires -frtti without hair-splitting which targets default which way.

[wristow]

include/clang/Basic/DiagnosticSemaKinds.td

6729

Saying requires -frtti makes good sense to me.

[Sunil_Srivastava]

Changed as per Paul's suggestion.

include/clang/Basic/DiagnosticSemaKinds.td

6729

Done

[probinson]

LGTM

include/clang/Basic/DiagnosticSemaKinds.td

 def err_invalid_declarator_in_function : Error<
   "definition or redeclaration of %0 not allowed inside a function">;
 def err_invalid_declarator_in_block : Error<
   "definition or redeclaration of %0 not allowed inside a block">;
 def err_not_tag_in_scope : Error<
   "no %select{struct|interface|union|class|enum}0 named %1 in %2">;
 
 def err_no_typeid_with_fno_rtti : Error<
-  "cannot use typeid with -fno-rtti">;
+  "use of typeid requires enabling RTTI">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
-  "cannot use dynamic_cast with -fno-rtti">;
+  "use of dynamic_cast requires enabling RTTI">;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
 def err_incomplete_object_call : Error<
   "incomplete type in call to object of type %0">;
 
 def warn_condition_is_assignment : Warning<"using the result of an "
   "assignment as a condition without parentheses">,

test/SemaCXX/no-rtti.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -fno-rtti %s
 
 namespace std {
   class type_info;
 }
 
 void f()
 {
-  (void)typeid(int); // expected-error {{cannot use typeid with -fno-rtti}}
+  (void)typeid(int); // expected-error {{use of typeid requires enabling RTTI}}
 }
 
 namespace {
 struct A {
   virtual ~A(){};
 };
 
 struct B : public A {
   B() : A() {}
 };
 }
 
 bool isa_B(A *a) {
-  return dynamic_cast<B *>(a) != 0; // expected-error {{cannot use dynamic_cast with -fno-rtti}}
+  return dynamic_cast<B *>(a) != 0; // expected-error {{use of dynamic_cast requires enabling RTTI}}
 }
 
 void* getMostDerived(A* a) {
   // This cast does not use RTTI.
   return dynamic_cast<void *>(a);
 }