Thanks for the reviews! Committed in r182647. I'll watch the build bots for the autolink test.
~Aaron On Fri, May 24, 2013 at 10:41 AM, Reid Kleckner <[email protected]> wrote: > 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
