donat.nagy added inline comments.

================
Comment at: clang/test/Analysis/ArrayDelete.cpp:85-88
+    Derived *d3 = new DoubleDerived[10];
+    Base *b3 = d3; // expected-note{{Conversion from derived to base happened 
here}}
+    delete[] b3; // expected-warning{{Deleting an array of polymorphic objects 
is undefined}}
+    // expected-note@-1{{Deleting an array of polymorphic objects is 
undefined}}
----------------
steakhal wrote:
> donat.nagy wrote:
> > steakhal wrote:
> > > Hmm, the static type of `d3` doesn't tell us much about how it refers to 
> > > an object of type `DoubleDerived`.
> > > To me, it would make sense to have multiple `Conversion from derived to 
> > > base happened here`, even telling us what static type it converted to 
> > > what other static type in the message.
> > > And it should add a new visitor of the same kind tracking the castee.
> > > 
> > > ```
> > > Derived *d3 = new DoubleDerived[10]; // note: `DoubleDerived` -> 
> > > `Derived` here
> > > Base *b3 = d3; // note: `Derived` -> `Base` here
> > > delete[] b3; // warn: Deleting `Derived` objects as `Base` objects.
> > > ```
> > > WDYT @donat.nagy ?
> > I agree that it would be good to be good to mention the class names in the 
> > message.
> Do you also agree that we should have all steps where such a conversion 
> happened?
> Notice the 2 `note:` markers in my example. @donat.nagy 
It would be a nice addition if it wouldn't seriously complicate the 
implementation.

If we want to report multiple/all conversions, then we would need to create 
messages for back-and-forth conversions (e.g. allocating Derived, converting it 
to Base, back to Derived, back to Base, then deleting it illegally).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158156/new/

https://reviews.llvm.org/D158156

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to