erichkeane 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;
----------------
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.


https://reviews.llvm.org/D39210



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to