With all that fixed, LGTM, feel free to see if it sticks. :)
On Fri, May 24, 2013 at 10:41 AM, Reid Kleckner <[email protected]> wrote: > It'd be good to factor out the bodies of getDependentLibraryOption() into > a static helper getWindowsLib() or something. When it was just += > "/DEFAULTLIB" it didn't seem reasonable. > > nit: trailing whitespace at "|| \n" > > This test will probably fail on non-Windows platforms: > > -// CHECK: ![[AUTOLINK]] = metadata !{metadata > !"{{(-l|/DEFAULTLIB:)}}autolink"} > +// CHECK: ![[AUTOLINK]] = metadata !{metadata > !"{{(-l|/DEFAULTLIB:)}}autolink.lib"} > > You'll have to relax it. I think {{(\.lib)?}} might work, but watch the > bots afterwards if you can't easily test elsewhere before committing. > > The .lib here on Windows is desirable, although no one is using modules > with autolink on Windows yet. > > > On Fri, May 24, 2013 at 10:01 AM, Aaron Ballman <[email protected]>wrote: > >> This patch moves the functionality to TargetInfo. Interesting thing >> to note is that this changes the autolink behavior, whereas the >> previous patch did not; is that acceptable (I don't know much about >> autolink). >> >> Thanks! >> >> ~Aaron >> >> On Fri, May 24, 2013 at 7:55 AM, Reid Kleckner <[email protected]> wrote: >> > It's probably better to move this to TargetInfo. Specifically >> > WinX86_32/64TargetCodeGenInfo know about /defaultlib already. >> > >> > >> > On Thu, May 23, 2013 at 4:44 PM, Aaron Ballman <[email protected]> >> > wrote: >> >> >> >> One of the things comment(lib) does in MSVC is automatically suffix >> >> the argument with ".lib" if needs be. This patch implements the same >> >> logic, which allows us to properly link libraries such as ones >> >> provided by the MSDN examples >> >> (http://msdn.microsoft.com/en-us/library/7f0aews7(v=vs.80).aspx). >> >> >> >> ~Aaron >> > >> > >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
