https://github.com/isuckatcs requested changes to this pull request.
So up until this point we just kept extending the checker with features without pushing any of these features to completion. At this point it looks like it got out of hand a bit, so the **focus** should be **on testing and merging** what we have, and **stop adding features** for now. `LifetimeAnnotations.cpp` currently does 3 different things, so we should probably rethink the architecture. 1. I'd like to see `DebugLifetimeAnnotations` moved to a separate source file called `DebugLifetimeAnnotations.cpp`. 2. I also think the logic inside `checkLocation` belongs to a diferent checker, probably `DereferenceChecker`, so I'd like to see that moved as well, because it has nothing to do with lifetime annotations. The 3 different parts (LifetimeAnnotations, DebugLifetimeAnnotations, `checkLocation()` logic) should probably all be inside separate reviews, which explains why this one got so big. Don't separate them now, but we should keep this in mind for the future. Also, the missing dependency issue is still not resolved, and the analyzer cannot be built with dynamic linking. https://github.com/llvm/llvm-project/pull/200145#discussion_r3412911596 https://github.com/llvm/llvm-project/pull/200145 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
