RedDocMD added a comment.

> But before we go there we should decide whether we want to actually go for 
> inlining (or otherwise default-evaluating) these destructors. If we do, we 
> should probably not spend too much time on improving invalidation in the 
> checker, because default evaluation would do that properly for us anyway 
> (well, it doesn't really dodge any problems such as the absence of the 
> necessary AST so we'll probably have to solve all these problems anyway, just 
> in a different setting). So it's great that we've fixed `evalCall` for 
> destructors, this could definitely land as a separate patch (tested via 
> `debug.AnalysisOrder`), but we really need to think what to do next here. So 
> I recommend gathering some data to see if proper destructor evaluation is 
> actually a real problem.

`MallocChecker` doesn't seem to mind not evaluating destructors properly. With 
the current version of the patch, the following code doesn't emit any warnings:

  class SillyPtr {
        int *ptr;
        bool wasMalloced;
  public:
        SillyPtr(int *ptr, bool wasMalloced = false) : 
                        ptr(ptr), 
                        wasMalloced(wasMalloced) {}
        ~SillyPtr() {
                if (wasMalloced) free(ptr);
                else delete ptr;
        }
  };
  
  void foo() {
        int *ptr = new int(13);
        SillyPtr silly(ptr);
        // No leak here!
  }

I am going to remove the debug dumps and run this patch on the projects in the 
`clang/utils/analyzer/projects` folder. If I don't find any false positives 
being caused due to this lack of modelling, then I think we can defer the 
proper handling of destructors (ie, finish up the invalidation) and move on to 
the other remaining problems (notes on get for an instance).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821

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

Reply via email to