eandrews added inline comments.
================ Comment at: lib/Sema/SemaType.cpp:3269-3273 + bool IsMain = false; + if (D.getIdentifier() && D.getIdentifier()->isStr("main") && + S.CurContext->getRedeclContext()->isTranslationUnit() && + !S.getLangOpts().Freestanding) + IsMain = true; ---------------- erichkeane wrote: > rnk wrote: > > erichkeane wrote: > > > rnk wrote: > > > > I highly doubt this is correct. I have a feeling there are all kinds of > > > > ways you can get this to fire on things that aren't a function > > > > declaration. It's also inefficient to check the identifier string every > > > > time we make a function type. Please find somewhere else to add this. > > > > I'd suggest adjusting the function type in CheckMain, or some time > > > > before then, whereever we make main implicitly extern "C". > > > I believe the logic here was pulled from FunctionDecl's "isMain" > > > function: > > > https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html#aa2b31caf653741632b16cce1ae2061cc > > > > > > As this is so early in the process (at Declarator), I didn't see a good > > > place to recommend extracting this, besides pulling it into the > > > Declarator. > > > > > > CheckMain is also run at the Decl stage, isn't it? Is your suggestion be > > > to simply let this go through with the wrong calling-convention here, > > > then revert it it in "CheckMain"? > > Yep. You can look at how we use FunctionTypeUnwrapper and > > ASTContext::adjustFunctionType in various places to fix up function types > > that were built with the wrong calling convention without losing type > > source info. > Perfect, thanks for the examples! I am sure @eandrews can use those to track > down the right place to fix this up. Thanks for the review! I've updated the patch. Please take a look. https://reviews.llvm.org/D39210 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits