[cfe-commits] support for Borland extensions and __pascal

[Douglas Gregor]

On Aug 31, 2010, at 3:30 PM, dawn at burble.org wrote:

> 
> This patch adds support for Borland extensions under the option
> -fborland-extensions, as well as semantic support for the "__pascal" calling
> convention (not yet added to llvm).

Typically, we prefer separate patches for separate changes. In this case, -fborland-extensions in one patch and __pascal as a follow-on.

> At this time, it's not possible to disctinguish between GNU attributes
> and those of and other vendors, so the calling convention is supported
> the same as MS calling conventions like "__thiscall".
> "__attribute__((pascal))" is also supported with this patch for
> consistency - I could remove this support, but since clang seems
> to want to print "__attribute__((pascal))" for "__pascal", it made more
> sense to just allow it.  

I agree that it makes sense to allow it. It's really the _pascal that we want to enable with -fborland-extensions.

> Note from the test case that there's an issue with pointer to member
> functions which I have yet to diagnose.


I see that the same problem occurs with other calling conventions, because we're not properly handling calling-convention attributes for member function pointers. That's now fixed in r112715, thanks!

The patch is looking really good. There are a few small things I'd like addressed before committing it:

The patch is missing modifications to various TableGen files, e.g., Attr.td, Options.td, and CC1Options.td. Without those, it won't build.

Index: include/clang/Basic/TokenKinds.def
===================================================================
--- include/clang/Basic/TokenKinds.def	(revision 112654)
+++ include/clang/Basic/TokenKinds.def	(working copy)
@@ -335,6 +335,9 @@
 KEYWORD(__thiscall                  , KEYALL)
 KEYWORD(__forceinline               , KEYALL)
 
+// Borland Extension.
+KEYWORD(__pascal                    , KEYALL)
+
 // Altivec Extension.
 KEYWORD(__vector                    , KEYALTIVEC)
 KEYWORD(__pixel                     , KEYALTIVEC)
@@ -371,6 +374,9 @@
 ALIAS("_stdcall"     , __stdcall  , KEYMS)
 ALIAS("_thiscall"    , __thiscall , KEYMS)
 
+// Borland Extensions.
+ALIAS("_pascal"      , __pascal   , KEYMS)
+

Why re-use KEYMS rather than introducing a new KEYBORLAND that is tied to -fborland-extensions?

Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp	(revision 112654)
+++ lib/CodeGen/CGCall.cpp	(working copy)
@@ -36,6 +36,7 @@
   case CC_X86StdCall: return llvm::CallingConv::X86_StdCall;
   case CC_X86FastCall: return llvm::CallingConv::X86_FastCall;
   case CC_X86ThisCall: return llvm::CallingConv::X86_ThisCall;
+  // TODO: add support for CC_X86Pascal to llvm
   }
 }

Typically, we'd want to emit an error here rather than silently using the wrong calling convention. But, in this case, getting a Diagnostic object all the way down to this routine is a pain. So, in this case, a diagnostic further up in CodeGen, e.g., when we see a DeclRefExpr that refers to a function with PascalAttr, would suffice.

Everything else looks great, thanks!

	- Doug