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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
