wristow added inline comments.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; ---------------- Sunil_Srivastava wrote: > probinson wrote: > > filcab wrote: > > > 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) > > 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. > 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? > 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. Repository: rC Clang https://reviews.llvm.org/D47291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
