On Tue, Apr 19, 2005 at 07:01:40AM +0200, Markus F.X.J. Oberhumer wrote:> While trying to hunt down a codegen bug (not yet found) ...Have you considered using bugpoint for your codegen debugging needs? http://llvm.cs.uiuc.edu/docs/Bugpoint.html#codegendebug> I've collected some small patches you might find useful.Sweet!> Please review and apply as you see fit.I've applied your gccld patch. For the isExecutable patch, you check to see if it's a file first before calling access(), but that precludes that function being used on a directory (which is a valid sys::Path object and could be queried for being executable), so I think it's unnecessary. I also applied your patch to ignore dangling symlinks. Finally, I applied your patch to ArchiveWriter with minor modifications -- please see llvm-commits or the web archive.> P.S. are there any plans to remove trailing whitespace from the source > files? - these cause noisy diffs and sometimes require much manual > work.Sure. Patches are accepted, but please separate formatting changes from functionality changes into different patches. Thanks again! -- Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu
MIsha, You asked for my feedback on Markus' recent patches. Here it is. On Tue, 2005-04-19 at 23:01 -0500, Misha Brukman wrote:> On Tue, Apr 19, 2005 at 07:01:40AM +0200, Markus F.X.J. Oberhumer wrote: > > While trying to hunt down a codegen bug (not yet found) ... > > Have you considered using bugpoint for your codegen debugging needs? > http://llvm.cs.uiuc.edu/docs/Bugpoint.html#codegendebug > > > I've collected some small patches you might find useful. > > Sweet! > > > Please review and apply as you see fit. > > I've applied your gccld patch.I concur.> > For the isExecutable patch, you check to see if it's a file first before > calling access(), but that precludes that function being used on a > directory (which is a valid sys::Path object and could be queried for > being executable), so I think it's unnecessary.The *original* intent of this method was to check for executable programs, not whether a directory is searchable. If we want that capability we should add a method named "searchable". Some operating systems don't have that concept and we should make the interface functions on Path address only one concept at a time. So, given that the "executable" method is verifying that the path points to a program that could be executed by the calling process, Markus patch is reasonable (needed, even).> I also applied your > patch to ignore dangling symlinks.That part of the Path.h patch is similar to a previous one we committed and I concur.> > Finally, I applied your patch to ArchiveWriter with minor modifications > -- please see llvm-commits or the web archive.I concur. Reid. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050419/814b6a21/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050419/814b6a21/attachment.sig>
On Apr 20, 2005, at 0:01, Misha Brukman wrote:> Sure. Patches are accepted, but please separate formatting changes > from > functionality changes into different patches.Speaking of random formatting changes, I've noticed in a few pages that the doxygen documentation comments incorrectly only have two '//' characters on the line. Hence, no doxygen documentation gets generated. Should I be creating a patch for this, or should I just report it? Evan
On Wed, 20 Apr 2005, Evan Jones wrote:> On Apr 20, 2005, at 0:01, Misha Brukman wrote: >> Sure. Patches are accepted, but please separate formatting changes from >> functionality changes into different patches. > > Speaking of random formatting changes, I've noticed in a few pages that the > doxygen documentation comments incorrectly only have two '//' characters on > the line. Hence, no doxygen documentation gets generated. Should I be > creating a patch for this, or should I just report it?Please send patches. :) -Chris -- http://nondot.org/sabre/ http://llvm.cs.uiuc.edu/