On 24/02/2014 12:36, Rafael Espíndola wrote:
On 24 February 2014 01:46, Alp Toker <[email protected]> wrote:
Hi Rafael,
This makes a lot of sense. Supporting the Windows text mode flag only ever
caused bugs that were difficult to detect where F_Binary was mistakenly
omitted.
Furthermore if we do ever want to write out CRLFs, we'll want to do it
explicitly by writing out '\r\n' instead of relying on the Windows feature
given that it introduces byte offsets that break seek/fseek.
Your patch includes context for the cases where F_Binary was removed, but
what'd be more interesting is to see the raw_fd_ostream uses that _were_
previously operating in text mode whose functionality will change following
your patch. Do you have an idea which (if any) uses are still in text mode?
Yes. Clang and llc had explicit logic for setting F_Binary only when
not printing assembly. Things that knew they were always printing text
(like a graphviz file), would also print in text mode. This is
probably an historical thing. We had text output only first and then
added binary.
If you want, I can make a first patch that just removes the default
argument. That should make the F_None explicit where a text output is
being used.
Yep, either that or making F_Binary the default and providing an F_Text
seems reasonable before pulling the rug completely from under so we have
an idea where the functional changes are, including other modules where
Windows test coverage might not be great.
Alp.
Cheers,
Rafael
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
