================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2095
@@ -2094,1 +2094,3 @@
+def err_attribute_dllimport_function_definition : Error<
+  "dllimport cannot by applied to non-inline function definition">;
 def err_attribute_dllimport_data_definition : Error<
----------------
Yunzhong Gao wrote:
> Typo. "cannot by applied" => "cannot be applied"
Thanks for catching! Fixed.

================
Comment at: lib/AST/ASTContext.cpp:7802
@@ +7801,3 @@
+static GVALinkage fixGVALinkageForDLLAttribute(GVALinkage L, const Decl *D) {
+  if (D->hasAttr<DLLImportAttr>()) {
+    if (L == GVA_DiscardableODR || L == GVA_StrongODR)
----------------
Yunzhong Gao wrote:
> Reid Kleckner wrote:
> > This should have a comment about the semantics of dllimport and dllexport, 
> > and link to the MSDN doc on dllexport with inline functions:
> > http://msdn.microsoft.com/en-us/library/xa0d9ste.aspx
> If the first argument to fixGVALinkageForDLLAttribute() is always the return 
> result of
> basicGVALinkageForFunction(), I wonder maybe it is more readable to just call 
> the
> basic linkage function from the fix linkage function.
> 
> Do we still check somewhere that we do not accept the combination of 
> C99/GNU-inline with dllexport/dllimport?
No, with this patch we allow dllimport/export on all inline functions.

Reid pointed out that we would get the linkage wrong for exported functions in 
C99 mode, and that the reasonable thing to do would be to use MS semantics 
dllimport/export inline functions. I'll update the patch to do this.

================
Comment at: lib/Sema/SemaDecl.cpp:9774
@@ +9773,3 @@
+
+    if (!DA->isInherited() && !FD->isInlined()) {
+      Diag(FD->getLocation(), 
diag::err_attribute_dllimport_function_definition);
----------------
Yunzhong Gao wrote:
> Do you have a test case for the isInherited() case where the dllimport 
> definition should be allowed?
The original code had it, so I figured it was important for some reason.

The check goes all the way back to r61437, and it's not entirely clear what is 
was for even then. Back then it was possible to have both dllimport and 
dllexport attributes on the same function, and I suspect the isInherited check 
has to do with that.

I don't think that check is correct today, so I'm going to remove it and 
simplify the code here a bit.

http://reviews.llvm.org/D3772



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to