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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
