[cfe-dev] extern "C" support

[Mike Stump]

On Dec 22, 2007, at 5:28 PM, Chris Lattner wrote:
> Please attach patches as attachments, not inline.  If you're using  
> 'mail', just name the file "whatever.patch" and this will happen.   
> Until this happens, I can't properly review the patch as it's all  
> wrapped and nastified :(

Odd, you must be using an old mail viewer...  In modern viewers, it  
should come out just fine.  I pretested cutting and pasting and  
ensured there were no problems with it.  Oh well...  What mail viewer  
do you use?

> +/// LinkageSpecDecl - This represents a linkage specification.

> +class LinkageSpecDecl : public Decl {
>
> Please give an example of what a linkage spec is in the header.  At  
> some point, we probably want to make a DeclCXX.h file, but deferring  
> this for now is fine.
>
> +private:
> +  /// Language - The language for this linkage specification.
> +  Language language;
> +  bool inside_braces;
> +  Decl *D;
>
> inside_braces is good for "remembering" the original source,

I did that merely because we're calling them ASTs and because we form  
ParenExprs.  If we didn't want ASTs, we could get rid of ParenExprs?

> but unless there is a client,

-ast-print uses that information to print the ast. Absent that  
information we can't faithfully reproduce the syntax tree.  Do we care  
that the printed ast is wrong?  Do we care if the ast isn't actually  
an ast?

> I'd suggest not including it.  How does your Decl structure handle  
> stuff like:
>
> extern "C" {
>  int x;  int y;
> }

work in progress... but roughly, an AST should have one linkage spec  
with two decls under it.

> Is there just one LinkageSpecDecl?

Yes.  This choice was mainly driven by keeping the AST an ast.

> +++ ./Parse/Parser.cpp	2007-12-21 13:58:52.000000000 -0800
> @@ -378,6 +378,12 @@ Parser::DeclTy *Parser::ParseDeclaration
>       return ParseObjCAtInterfaceDeclaration(AtLoc,
> DS.getAttributes());
>   }
>
> +  if (Tok.is(tok::string_literal)
> +      && DS.getStorageClassSpec() == DeclSpec::SCS_extern)
> +    {
> +      return ParseLinkage(Declarator::FileContext);
>
> I'm not sure if this is sufficient:

It wasn't, but that was a known issue.

> does this allow 'extern int "C"'  or 'extern inline "C"'

I'm sure...

> The DeclSpec::getParsedSpecifiers() method can be used to make sure  
> there is just a storage class specified.

Ah, perfect, I've fixed it and tested extern inline "C"; that is now  
rejected.

> +Sema::DeclTy* Sema::ActOnLinkageSpec(SourceLocation Loc,
> +				     std::string Lang,
> +
>
> Please don't pass std::strings by-value, pass by const reference, or  
> better yet, pass in the string as a const char* or something like  
> that.

Ok, fixed.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: d.patch
Type: application/octet-stream
Size: 12466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080102/e2244349/attachment.obj>
-------------- next part --------------