[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116
 
+  const bool Foreign = false; // From CTU.
+
----------------
martong wrote:
> martong wrote:
> > xazax.hun wrote:
> > > I feel that we use different terms for the imported declarations. Sometimes we call them `new`, sometimes `imported`, sometimes `foreign`. In case all of these means the same thing, it would be nice to standardize on a single way of naming. If there is a subtle difference between them, let's document that in a comment. It would be nice if we did not need the comment after the declaration but it would be obvious from the variable name.
> > Yes, I agree that this should deserver some more explanation. Maybe right above this declaration?
> > 
> > So, `new` means that a declaration is **created** newly by the ASTImporter.
> > `imported` means it has been imported, but not necessarily `new`. Think about this case, we import `foo`'s definition.
> > ```
> > // to.cpp
> > void bar() {} // from a.h
> > // from.cpp
> > void bar() {} // from a.h
> > void foo() {
> >   bar();
> > }
> > ```
> > Then `foo` will be `new` and `imported`, `bar` will be `imported` and not `new`.  
> > `foreign` basically means `imported` and `new`.
> I've just added an explanatory comment for this field.
Foreign means new and imported. But is there a way for a declaration to be new and not to be imported? If no, in that case it feels like new and foreign are actually the same and we should standardize on a single name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773