[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-18 Thread Benoit Belley via Phabricator via cfe-commits
belleyb added a comment.

@chh I had a chance to try out your proposed changes. It's not causing us any 
trouble. In fact, __gcov_flush() is not even used at all (at least in LLVM 
5.0.1).. I can recompile llvm, compiler_rt and clang and re-run all the tests 
with __gcov_flush commented out! No problem.

I would suggest adding a bit more documentation to __gcov_flush(), thus 
describing what those "special cases" are...


https://reviews.llvm.org/D45454



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


[PATCH] D41081: Fix clang Lexer Windows line-ending bug

2017-12-11 Thread Benoit Belley via Phabricator via cfe-commits
belleyb added inline comments.



Comment at: test/lit.cfg.py:52-57
+if platform.system() in ['Windows']:
+config.substitutions.append(('dos2unix', 'sed -b "s/\r$//"'))
+config.substitutions.append(('unix2dos', 'sed -b "s/\r*$/\r/"'))
+else:
+config.substitutions.append(('dos2unix', "sed $'s/\r$//'"))
+config.substitutions.append(('unix2dos', "sed $'s/\r*$/\r/'"))

caoz wrote:
> zturner wrote:
> > Since the user has `sed` already, why wouldn't they have the actual tool 
> > `dos2unix` and `unix2dos`?
> Thanks Zachary. dos2unix and unix2dos aren't installed by default on 
> Linux/Mac. However, if the user can be assumed to have them, I can remove 
> these substitutions.
@zturner According to https://llvm.org/docs/GettingStarted.html#software, we 
shouldn't be relying on `dos2unix` nor `unix2dos` to be present on a build 
machine. 

Note that the `$'string'` syntax (ANSI-C quotes) is a `bash` extension. 
According to the page mentioned above, the LLVM builds should only be relying 
on having a Bourne shell (`sh`). But, are there still any *nix system out there 
where `/bin/sh` isn't a link for `\bin\bash` ? I.e. Is relying on `bash+sed` 
fine here ?

A fully portable solution would be to write python scripts for `dos2unix.py` 
and `unix2dos.py`. That way, one would not be relying on build tools that LLVM 
isn't already using.

Any advice on how to proceed ?




https://reviews.llvm.org/D41081



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


[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

2017-11-02 Thread Benoit Belley via Phabricator via cfe-commits
belleyb added a comment.

Autodesk would also like to see this fix merged into LLVM 5.0.1 and we also 
agree about the importance of having a regression tests.

@sylvestre.ledru : Is someone on the Firefox team working on writing a unit 
test for this ? Shall we jump-in and try to help ?


https://reviews.llvm.org/D38124



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


[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

2017-11-08 Thread Benoit Belley via Phabricator via cfe-commits
belleyb added a comment.

@sylvestre.ledru Thanks for the test!


https://reviews.llvm.org/D38124



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