On Wed, May 29, 2013 at 4:15 PM, Reid Kleckner <[email protected]> wrote: > On Wed, May 29, 2013 at 3:55 PM, Aaron Ballman <[email protected]> > wrote: >> >> On Wed, May 29, 2013 at 3:31 PM, Reid Kleckner <[email protected]> wrote: >> > --- include/clang/AST/ASTConsumer.h (revision 182601) >> > +++ include/clang/AST/ASTConsumer.h (working copy) >> > @@ -92,6 +92,12 @@ >> > /// only exists to support Microsoft's #pragma comment(linker, >> > "/foo"). >> > virtual void HandleLinkerOptionPragma(llvm::StringRef Opts) {} >> > >> > + /// \brief Handle a pragma that emits a mismatch identifier and value >> > to >> > the >> > + /// object file for the linker to work with. Currently, this only >> > exists >> > to >> > + /// support Microsoft's #pragma detect_mismatch. >> > + virtual void HandleDetectMismatch(llvm::StringRef Name, >> > + llvm::StringRef Value) {} >> > + >> > >> > Should we avoid broadening the ASTConsumer interface? If we lowered the >> > option in Sema, then we could reuse HandleLinkerOptionPragma() without >> > too >> > much fuss. But then we're doing codegen-like work in Sema. I'd like a >> > second opinion on this. If we can do this in Sema, we can eliminate a >> > lot >> > of the plumbing for this and for #pragma comment(lib). >> >> I had originally done it that way, but wasn't overly keen on it. We >> could reuse the TargetInfo behavior for getting the proper linker >> options, but it seemed like a layering violation. > > > Yeah, we'd have to use some Target interface outside of CodeGen, but I don't > see anything obvious. > > Perhaps we should continue full steam ahead on this route and refactor at a > later date.
I don't think the current approach is particularly heinous, just verbose. I'd be fine sticking with this route for now. But if someone has a better idea as to how to structure this so we could get more reusable machinery out of it, I'd be happy to attempt it as well. ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
