================
Comment at: include/clang/Sema/Sema.h:395-396
@@ -392,1 +394,4 @@
+ /// \brief All delete-expressions within the translation unit.
+ llvm::SmallPtrSet<const CXXDeleteExpr *, 4> DeleteExprs;
+
----------------
Richard Smith wrote:
> This is not a reasonable way to track the relevant state here.
>
> The interesting cases are `VarDecl`s and `FieldDecl`s. For `VarDecl`s, at the
> point where we see the `delete`, you can immediately check whether you have
> an new-expression as an initializer (and don't worry about the obscure cases
> where an initializer comes after the use). For `FieldDecl`s, perform the
> check where you see the `delete`, and if you see either (a) no constructor
> with a matching `new`, or (b) only constructors with the wrong kind of `new`,
> then add your `delete` to a list to check at the end of the TU.
>
> That way, in almost all cases we can deal with the diagnostic immediately
> rather than at end of TU. (This avoids building up a big set of all delete
> expressions.)
>
> You'll also need to figure out how you're going to handle
> serialization/deserialization here, in the case where you hit one of these
> "not sure" cases in a PCH or similar. To that end, you should probably store
> a list of `{SourceLocation, FieldDecl}` pairs, or -- better -- a map from
> `FieldDecl` to the first source location where we saw a `delete`. (You'll
> need one extra bit to describe whether it was `delete` or `delete[]` I
> suppose...).
I have implemented the first half of the patch, slightly different than your
suggestion. We diagnose this immediately for `VarDecl`, as you said. For
`FieldDecl`s, delete expression is queued, if at least one constructor isn't
defined (yet). Otherwise (if all constructor definitions are visible), warning
is issued in `ActOnCXXDelete` right before function returns. Could you please
explain the rationale for your point (b)? If we can see all constructors while
analyzing a `CXXDeleteExpr`, we can reason about constructors and in-class
initializers immediately. If no constructor initializes the member, we check
in-class initializer (if exists). If all constructors initialize member with
mismatching type of `new`, then we can diagnose them all immediately, instead
of postponing it to the end of translation unit. I can think of only one case
(aside from PCH case) that requires postponing this analysis to the end of
translation unit; that is, when we cannot see at least one constru!
ctor's definition.
I have a few questions regarding PCH/external source. I have no prior
experience with ASTReader/Writer at all. Sorry, if my questions don't make any
sense. Do you mean we should handle following case:
// header; compiled as pch.
#ifndef header
struct X {
int *a;
~X() { delete a; } // not sure
X();
};
#endif
// source file, includes above pch
#include "header"
X::X() : a(new int[1]) { }
We encounter with a delete-expr in a PCH. We can't prove anything, because we
can't see definition of `X::X()` yet. Even if there was an in-class
initializer, that wouldn't help, exactly for the same reason. `X::X()` will be
visible only at the end of translation unit. Therefore, we must write this
information somewhere in PCH so that we can load it at the end of translation
unit.
I have followed `UndefinedButUsed` as a serialization/deserialization example.
As far as I understood, I need to write a record, and read it back. I blindly
followed your suggestion here :) This record contains, as you said,
`FieldDecl`, `SourceLocation` and a bit indicating array form. I put them in an
`std::tuple<FieldDecl *, SourceLocation, bool>`. Can that "bit" be represented
as `APInt(width=1)`? I did, roughly:
RecordData DeleteExprsToAnalyze;
AddDeclRef(get<0>(tuple), DeleteExprsToAnalyze);
AddSource(get<1>(tuple), DeleteExprsToAnalyze);
AddAPInt(APInt(1, get<2>(tuple)), DeleteExprsToAnalyze);
// ...
if (!DeleteExprsToAnalyze.empty())
Stream.EmitRecord(DELETE_EXPRS_TO_ANALYZE, DeleteExprsToAnalyze);
I am trying the PCH part now. I would like to learn the right way, as I might
be doing something completely wrong.
http://reviews.llvm.org/D4661
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
