[cfe-commits] [PATCH] PR13824 warn if reinterpret_cast used for up/downcast

[Dmitri Gribenko]

On Wed, Jan 23, 2013 at 11:33 PM, Alexander Zinenko <ftynse at gmail.com> wrote:
> Thanks for feedback! Here is an updated version. It warns by default in case
> of virtual base or in case of nonzero offset. Other up/downcasts lead to a
> warning with -Wreinterpret-updown-extra, ignored by default.

I think it is sensible to leave the warning on by default.  The
purpose of a separate flag is to turn off the (possibly) noisy case
separately.

I am not a fan of the name '-Wreinterpret-updown-extra'.
-Wreinterpret-updown-zero-adjustment or -Wreinterpret-updown-safe
might be better.

+  // subobject with offset

Comments should start with a capital letter and end with a period.

+/// ReinterpretUpDownCast - Check that a reinterpret_cast\<DestType\>(SrcExpr)
+/// is not used as upcast or downcast between respective pointers or
+/// references.

Please don't duplicate function name in comments when adding new code.

+static void ReinterpretUpDownCast(Sema &Self, ExprResult &SrcExpr,

A better name: DiagnoseReinterpretUpDownCast?

+  CharUnits offset = CharUnits::Zero();

s/offset/Offset/

+def warn_reinterpret_updowncast : Warning<
+  "'reinterpret_cast' is used as an %select{upcast|downcast}0 from type %1 "
+  "to its %select{base|derived}0 type %2">,

(1) Trailing whitespace.

(2) It would be better to do "%select{an upcast|a downcast}" or we
would print "an downcast".

+def warn_reinterpret_wrong_subobject : Warning<
+  "'reinterpret_cast' might result in pointing to wrong subobject">,

... might return a pointer... ?

+def note_reinterpret_updowncast_use_static_cast : Note<
+  "use of 'static_cast' adjusts the pointer correctly while "
+  "%select{upcasting|downcasting}0">;

"use 'static_cast' to adjust the pointer correctly ..." ?

+def note_reinterpret_virtual_base : Note<
+  "casting %select{to|from}0 virtual base type %1 %select{from|to}0 "
+  "its derived type %2">, InGroup<ReinterpretUpDownCast>;

Notes should not be put in groups.

+  InGroup<ReinterpretUpDownCast>, DefaultWarn;

DefaultWarn can be dropped here.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/