> On Feb 3, 2015, at 1:30 PM, David Blaikie <[email protected]> wrote: > > > > On Tue, Feb 3, 2015 at 1:23 PM, Steven Wu <[email protected] > <mailto:[email protected]>> wrote: > Hi David > > Sorry I dropped you from the previous update. Does the updated patch look ok > now? > > Code looks fine (I still like range-for loops more, but probably would need > to have a "filter" function that returns a range to make that convenient > enough to bother using here, rather than having to build a range from the > result) - but I don't know anything about the semantics here so I'll leave it > to someone who knows about frameworks & their flags to sign off on the > semantics of this change. > > - David
Yes, the semantics look right. (The only difference between clang’s -F and -iframework options is whether the directory is treated as containing user vs. system frameworks. The linker does not make a distinction between those, so both kinds of directories should be passed to the linker with -F. The current behavior of ignoring the -iframework option when linking is pretty horribly broken.) This is fine to commit. > > > Thanks > > Steven > >> On Jan 21, 2015, at 12:50 PM, Steven Wu <[email protected] >> <mailto:[email protected]>> wrote: >> >> - Format fix >> >> The extra braces are bad so I get rid of them. The range loop doesn't make >> thing better because of the long variable names. I happily switch to auto >> but keep my original loop structure. >> >> >> http://reviews.llvm.org/D7106 <http://reviews.llvm.org/D7106> >> >> Files: >> lib/Driver/Tools.cpp >> test/Driver/darwin-ld.c >> >> Index: lib/Driver/Tools.cpp >> =================================================================== >> --- lib/Driver/Tools.cpp >> +++ lib/Driver/Tools.cpp >> @@ -5964,6 +5964,12 @@ >> Args.AddAllArgs(CmdArgs, options::OPT_T_Group); >> Args.AddAllArgs(CmdArgs, options::OPT_F); >> >> + // -iframework should be forwarded as -F. >> + for (auto it = Args.filtered_begin(options::OPT_iframework), >> + ie = Args.filtered_end(); it != ie; ++it) >> + CmdArgs.push_back(Args.MakeArgString(std::string("-F") + >> + (*it)->getValue())); >> + >> const char *Exec = >> Args.MakeArgString(getToolChain().GetLinkerPath()); >> std::unique_ptr<Command> Cmd = >> Index: test/Driver/darwin-ld.c >> =================================================================== >> --- test/Driver/darwin-ld.c >> +++ test/Driver/darwin-ld.c >> @@ -204,3 +204,9 @@ >> // RUN: FileCheck -check-prefix=LINK_IOS_SIMULATOR_VERSION_MIN %s < %t.log >> // LINK_IPHONEOS_VERSION_MIN: -iphoneos_version_min >> // LINK_IOS_SIMULATOR_VERSION_MIN: -ios_simulator_version_min >> + >> +// Check -iframework gets forward to ld as -F >> +// RUN: %clang -target x86_64-apple-darwin %s -iframework Bar -framework >> Foo -### 2>&1 | \ >> +// RUN: FileCheck --check-prefix=LINK-IFRAMEWORK %s >> +// LINK-IFRAMEWORK: {{ld(.exe)?"}} >> +// LINK-IFRAMEWORK: "-FBar" >> >> EMAIL PREFERENCES >> http://reviews.llvm.org/settings/panel/emailpreferences/ >> <http://reviews.llvm.org/settings/panel/emailpreferences/> >> <D7106.18548.patch> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
