On Mon, Sep 10, 2012 at 8:40 AM, Alexander Kornienko <[email protected]> wrote: > I could definitely try to find a number of other solutions, but > unfortunately it would be not fast, as I don't have a configuration that > allows to reproduce this issue, and I would have to ask the original > reporter to verify each version.
Ah, I see. I didn't realize you didn't have a local repro. (maybe we could get some older buildbots up at some point - or at least decide what we support & what we don't support, version-wise) > And, after all, is this solution that bad? I'd say most of us just object on principle - it's not the "right thing". > What problems can it lead to? ODR violations (as you say, if the name is sufficiently unique, that's unlikely). The only other thing I can think of is possible minor build perf (the function, since it's exported (assuming it's not inlined at all the call sites anyway & then dropped from the object file entirely), would end up in some linker data structure to unique any duplicates that will never arise - not knowing much about the linker I don't know whether that would increase the size of some common data structure of all weak symbols & in any way effect the performance) No worries, - David > > > On Mon, Sep 10, 2012 at 5:23 PM, David Blaikie <[email protected]> wrote: >> >> On Mon, Sep 10, 2012 at 7:54 AM, Alexander Kornienko <[email protected]> >> wrote: >> > Author: alexfh >> > Date: Mon Sep 10 09:54:38 2012 >> > New Revision: 163513 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=163513&view=rev >> > Log: >> > Workaround for MacOSX build failure with gcc <= 4.4 >> > >> > Summary: >> > A better solution to http://llvm.org/bugs/show_bug.cgi?id=13777 >> > Named namespace + more unique name to make ODR violations unlikely. >> > >> > Reviewers: chandlerc, doug.gregor, klimek >> > >> > Reviewed By: doug.gregor >> > >> > CC: cfe-commits >> > >> > Differential Revision: http://llvm-reviews.chandlerc.com/D38 >> > >> > Modified: >> > cfe/trunk/tools/clang-check/ClangCheck.cpp >> > >> > Modified: cfe/trunk/tools/clang-check/ClangCheck.cpp >> > URL: >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-check/ClangCheck.cpp?rev=163513&r1=163512&r2=163513&view=diff >> > >> > ============================================================================== >> > --- cfe/trunk/tools/clang-check/ClangCheck.cpp (original) >> > +++ cfe/trunk/tools/clang-check/ClangCheck.cpp Mon Sep 10 09:54:38 2012 >> > @@ -58,10 +58,10 @@ >> > "ast-dump-filter", >> > >> > cl::desc(Options->getOptionHelpText(options::OPT_ast_dump_filter))); >> > >> > -// Anonymous namespace here causes problems with gcc <= 4.4 on MacOS: >> > -// http://llvm.org/bugs/show_bug.cgi?id=13777 >> > -// namespace { >> > -class ActionFactory { >> > +// Anonymous namespace here causes problems with gcc <= 4.4 on MacOS >> > 10.6. >> > +// "Non-global symbol: ... can't be a weak_definition" >> >> Judging by that diagnostic - is it possible that making the >> newASTConsumer member function non-inline would have been an >> alternative fix to the code? >> >> > +namespace clang_check { >> > +class ClangCheckActionFactory { >> > public: >> > clang::ASTConsumer *newASTConsumer() { >> > if (ASTList) >> > @@ -73,10 +73,10 @@ >> > return new clang::ASTConsumer(); >> > } >> > }; >> > -// } >> > +} >> > >> > int main(int argc, const char **argv) { >> > - ActionFactory Factory; >> > + clang_check::ClangCheckActionFactory Factory; >> > CommonOptionsParser OptionsParser(argc, argv); >> > ClangTool Tool(OptionsParser.GetCompilations(), >> > OptionsParser.GetSourcePathList()); >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > > -- > Alexander Kornienko | Software Engineer | [email protected] | +49 151 221 77 > 957 > Google Germany GmbH | Dienerstr. 12 | 80331 München > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
