[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki accepted this revision.
danielmarjamaki added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/scan-build-py/libscanbuild/analyze.py:165
+            with open(filename, 'r') as in_file:
+                for line in in_file:
+                    yield line
----------------
whisperity wrote:
> danielmarjamaki wrote:
> > I believe you can write:
> > 
> >     for line in open(filename, 'r'):
> Do we want to rely on the interpreter implementation on when the file is closed.
> 
> If 
> 
> ```
>   for line in open(filename, 'r'):
>      something()
> ```
> 
> is used, the file handle will be closed based on garbage collection rules. Having this handle disposed after the iteration is true for the stock CPython implementation, but it is still nontheless an implementation specific approach.
> 
> Whereas using `with` will explicitly close the file handle on the spot, no matter what.
ok I did not know that. feel free to ignore my comment.


================
Comment at: tools/scan-build-py/libscanbuild/analyze.py:172
+        extern_fns_map_file = os.path.join(ctudir, CTU_FUNCTION_MAP_FILENAME)
+        with open(extern_fns_map_file, 'w') as out_file:
+            for mangled_name, ast_file in mangled_ast_pairs:
----------------
whisperity wrote:
> danielmarjamaki wrote:
> > this 'with' seems redundant. I suggest an assignment and then less indentation will be needed below
> I don't seem to understand what do you want to assign to what.
I did not consider the garbage collection. I assumed that out_file would Always be closed when it Went out of scope and then this would require less indentation:

    out_file = open(extern_fns_map_file, 'w')
    for mangled_name, ast_file in mangled_ast_pairs:
        out_file.write('%s %s\n' % (mangled_name, ast_file))



================
Comment at: tools/scan-build-py/libscanbuild/analyze.py:223
+    ctu_config = get_ctu_config(args)
+    if ctu_config.collect:
+        shutil.rmtree(ctu_config.dir, ignore_errors=True)
----------------
danielmarjamaki wrote:
> not a big deal but I would use early exits in this function
with "not a big deal" I mean; feel free to ignore my comment if you want to have it this way.


https://reviews.llvm.org/D30691