[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

I tried the following:

  FunctionDecl *FD = ND->getAsFunction();
  DeclContext *DC = ND->getDeclContext();
  DEBUG_WITH_TYPE("foo", llvm::dbgs() << "[FOO] DC->isExternCContext() 
: " << DC->isExternCContext() << "\n");
  DEBUG_WITH_TYPE("foo", llvm::dbgs() << "[FOO] FD->isExternC()
: " << FD->isExternC()<< "\n");
  DEBUG_WITH_TYPE("foo", llvm::dbgs() << "[FOO] FD->isInExternCContext()   
: " << FD->isInExternCContext()   << "\n");
  llvm::dbgs() << "[FOO] "; ND->dump();
  if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
return;
  }

and I'm getting the following output:

  [FOO] DC->isExternCContext() : 0
  [FOO] FD->isExternC(): 0
  [FOO] FD->isInExternCContext()   : 0
  [FOO] FunctionDecl 0x10ec0250 prev 0x10ec0148  col:13 
foo 'void ()' static

The C file is:

  1 extern "C" {
  2 static void foo();
  3 }
  4 static void foo();
  5 #pragma alloc_text("s", foo)
  6 static void foo() {}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

In D126559#3598950 , @hans wrote:

>> Oh I see, that makes sense. We aren't accepting 
>> https://godbolt.org/z/9Yej9vhYd. Do you know of a way to get the `NamedDecl` 
>> with `extern "C"` instead of the second declaration?
>
> I'm not sure I understand the question, but it seems the current code is 
> checking `isExternCContext()`. I think instead you want to check 
> `FunctionDecl::isExternC()`.

Oh, that answers my question. I'll give that a try. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> Oh I see, that makes sense. We aren't accepting 
> https://godbolt.org/z/9Yej9vhYd. Do you know of a way to get the `NamedDecl` 
> with `extern "C"` instead of the second declaration?

I'm not sure I understand the question, but it seems the current code is 
checking `isExternCContext()`. I think instead you want to check 
`FunctionDecl::isExternC()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

In D126559#3598885 , @hans wrote:

>> Ignoring the `pragma alloc_text`, it looks like GCC compiles the following 
>> `foo` with C linkage vs LLVM which compiles with C++ linkage (foo's name is 
>> mangled):
>
> The mangled name shouldn't matter since it has internal linkage. I tried 
> dropping the `static`, and then foo doesn't seem to get mangled: 
> https://godbolt.org/z/arzW5TbYz

Oh I see, that makes sense. We aren't accepting 
https://godbolt.org/z/9Yej9vhYd. Do you know of a way to get the `NamedDecl` 
with `extern "C"` instead of the second declaration?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.



> Ignoring the `pragma alloc_text`, it looks like GCC compiles the following 
> `foo` with C linkage vs LLVM which compiles with C++ linkage (foo's name is 
> mangled):

The mangled name shouldn't matter since it has internal linkage. I tried 
dropping the `static`, and then foo doesn't seem to get mangled: 
https://godbolt.org/z/arzW5TbYz


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

Hmm, it looks like MSVC is accepting:

  extern "C" {
  static void foo();
  }
  
  static int foo();
  #pragma alloc_text("s", foo)
  static void foo() {}

Ignoring the `pragma alloc_text`, it looks like GCC compiles the following 
`foo` with C linkage vs LLVM which compiles with C++ linkage (foo's name is 
mangled):

  extern "C" {
  static int foo();
  }
  
  static int foo();
  static int foo() {
return 3;
  }
  
  int bar() {
return foo();
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5b056fe49a9: [MSVC] Fix pragma alloc_text failing for C 
files (authored by steplong).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/pragma-ms-alloc-text.c
  clang/test/Sema/pragma-ms-alloc-text.cpp


Index: clang/test/Sema/pragma-ms-alloc-text.cpp
===
--- clang/test/Sema/pragma-ms-alloc-text.cpp
+++ clang/test/Sema/pragma-ms-alloc-text.cpp
@@ -34,3 +34,9 @@
 void foo5();// expected-note {{previous declaration is here}}
 #pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is 
applicable only to functions with C linkage}}
 extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a 
different language linkage}}
+
+extern "C" {
+static void foo6();
+#pragma alloc_text(c, foo6) // no-warning
+void foo6() {}
+}
Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
+
+static void foo1();
+#pragma alloc_text("hello", foo1) // expected-no-diagnostics
+void foo1() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -763,7 +763,7 @@
 }
 
 DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }


Index: clang/test/Sema/pragma-ms-alloc-text.cpp
===
--- clang/test/Sema/pragma-ms-alloc-text.cpp
+++ clang/test/Sema/pragma-ms-alloc-text.cpp
@@ -34,3 +34,9 @@
 void foo5();// expected-note {{previous declaration is here}}
 #pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
 extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a different language linkage}}
+
+extern "C" {
+static void foo6();
+#pragma alloc_text(c, foo6) // no-warning
+void foo6() {}
+}
Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
+
+static void foo1();
+#pragma alloc_text("hello", foo1) // expected-no-diagnostics
+void foo1() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -763,7 +763,7 @@
 }
 
 DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 433194.
steplong added a comment.

- Handle static C/C++ functions and C functions in general


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/pragma-ms-alloc-text.c
  clang/test/Sema/pragma-ms-alloc-text.cpp


Index: clang/test/Sema/pragma-ms-alloc-text.cpp
===
--- clang/test/Sema/pragma-ms-alloc-text.cpp
+++ clang/test/Sema/pragma-ms-alloc-text.cpp
@@ -34,3 +34,9 @@
 void foo5();// expected-note {{previous declaration is here}}
 #pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is 
applicable only to functions with C linkage}}
 extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a 
different language linkage}}
+
+extern "C" {
+static void foo6();
+#pragma alloc_text(c, foo6) // no-warning
+void foo6() {}
+}
Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
+
+static void foo1();
+#pragma alloc_text("hello", foo1) // expected-no-diagnostics
+void foo1() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -763,7 +763,7 @@
 }
 
 DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }


Index: clang/test/Sema/pragma-ms-alloc-text.cpp
===
--- clang/test/Sema/pragma-ms-alloc-text.cpp
+++ clang/test/Sema/pragma-ms-alloc-text.cpp
@@ -34,3 +34,9 @@
 void foo5();// expected-note {{previous declaration is here}}
 #pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
 extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a different language linkage}}
+
+extern "C" {
+static void foo6();
+#pragma alloc_text(c, foo6) // no-warning
+void foo6() {}
+}
Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
+
+static void foo1();
+#pragma alloc_text("hello", foo1) // expected-no-diagnostics
+void foo1() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -763,7 +763,7 @@
 }
 
 DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D126559#3547921 , @steplong wrote:

> It looks like MSVC also accepts
>
>   // foo.c
>   static void foo();
>   #pragma alloc_text("hello", foo)
>   void foo() {}
>
> and
>
>   // foo.cpp
>   extern "C" {
>   static void foo();
>   #pragma alloc_text("hello", foo)
>   void foo() {}
>   }
>
> Do you know of a way I can check whether a function is coming from c or c++? 
> `isExternC()` returns false for the static case

You can look at `!LangOpts.CPlusPlus` to know that you're in C mode (or C++ 
mode)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

It looks like MSVC also accepts

  // foo.c
  static void foo();
  #pragma alloc_text("hello", foo)
  void foo() {}

and

  // foo.cpp
  extern "C" {
  static void foo();
  #pragma alloc_text("hello", foo)
  void foo() {}
  }

Do you know of a way I can check whether a function is coming from c or c++? 
`isExternC()` returns false for the static case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126559/new/

https://reviews.llvm.org/D126559

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

[PATCH] D126559: [MSVC] Fix pragma alloc_text failing for C files

[Stephen Long via Phabricator via cfe-commits]

steplong created this revision.
Herald added a project: All.
steplong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`isExternCContext()` is returning false for functions in C files

Change-Id: I51cccb476aa19dc0a0a78e951d7f482befdb78ca


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126559

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/pragma-ms-alloc-text.c


Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -762,8 +762,8 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+FunctionDecl *FD = ND->getAsFunction();
+if (!FD->isExternC()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }


Index: clang/test/Sema/pragma-ms-alloc-text.c
===
--- /dev/null
+++ clang/test/Sema/pragma-ms-alloc-text.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+
+void foo();
+#pragma alloc_text("hello", foo) // expected-no-diagnostics
+void foo() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -762,8 +762,8 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (!DC->isExternCContext()) {
+FunctionDecl *FD = ND->getAsFunction();
+if (!FD->isExternC()) {
   Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits