[PATCH] D128649: [clang-cl] Accept a pragma alloc_text corner case accepted by MSVC

[Stephen Long via Phabricator via cfe-commits]

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

- Use `getCanonicalDecl()`. This does make us reject (MSVC rejects it already 
because of conflicting declarations):

  static void f();
  extern "C" { static void f(); }
  static void f() {}



- Error if not function decl

I think we have to use `isInExternCContext()` to accept the following (MSVC 
accepts this):

  extern "C" { static void f(); }
  static void f() {}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128649

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaAttr.cpp
  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
@@ -40,3 +40,20 @@
 #pragma alloc_text(c, foo6) // no-warning
 void foo6() {}
 }
+
+extern "C" {
+static void foo7();
+}
+static void foo7();
+#pragma alloc_text(c, foo7) // no-warning
+void foo7() {}
+
+static void foo8();
+extern "C" {
+static void foo8();
+}
+#pragma alloc_text(c, foo8) // expected-error {{'#pragma alloc_text' is 
applicable only to functions with C linkage}}
+void foo7() {}
+
+enum foo9 { A, B, C };
+#pragma alloc_text(c, foo9) // expected-error {{'#pragma alloc_text' is 
applicable only to functions}}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -810,10 +810,18 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
-  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
-  return;
+if (getLangOpts().CPlusPlus) {
+  Decl *D = cast(ND)->getCanonicalDecl();
+
+  if (auto *FD = dyn_cast(D)) {
+if (!FD->isInExternCContext()) {
+  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+  return;
+}
+  } else {
+Diag(Loc, diag::err_pragma_alloc_text_not_function);
+return;
+  }
 }
 
 FunctionToSectionMap[II->getName()] = std::make_tuple(Section, Loc);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -992,6 +992,8 @@
   "'#pragma %0' can only appear at file scope">;
 def err_pragma_alloc_text_c_linkage: Error<
   "'#pragma alloc_text' is applicable only to functions with C linkage">;
+def err_pragma_alloc_text_not_function: Error<
+  "'#pragma alloc_text' is applicable only to functions">;
 
 def warn_pragma_unused_undeclared_var : Warning<
   "undeclared variable %0 used as an argument for '#pragma unused'">,


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
@@ -40,3 +40,20 @@
 #pragma alloc_text(c, foo6) // no-warning
 void foo6() {}
 }
+
+extern "C" {
+static void foo7();
+}
+static void foo7();
+#pragma alloc_text(c, foo7) // no-warning
+void foo7() {}
+
+static void foo8();
+extern "C" {
+static void foo8();
+}
+#pragma alloc_text(c, foo8) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
+void foo7() {}
+
+enum foo9 { A, B, C };
+#pragma alloc_text(c, foo9) // expected-error {{'#pragma alloc_text' is applicable only to functions}}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -810,10 +810,18 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
-  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
-  return;
+if (getLangOpts().CPlusPlus) {
+  Decl *D = cast(ND)->getCanonicalDecl();
+
+  if (auto *FD = dyn_cast(D)) {
+if (!FD->isInExternCContext()) {
+  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+  return;
+}
+  } else {
+Diag(Loc, diag::err_pragma_alloc_text_not_function);
+return;
+  }
 }
 
 FunctionToSectionMap[II->getName()] = std::make_tuple(Section, Loc);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -992,6 +992,8 @@
   "'#pragma %0' can only appear at file scope">;
 def err_pragma_alloc_text_c_linkage: Error<
   "'#pragma alloc_text' is applicable only to functions with C linkage">;
+def err_pragma_alloc_text_not_function: Error<
+  "'#pragma 

[PATCH] D128649: [clang-cl] Accept a pragma alloc_text corner case accepted by MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:811
   return;
 }
 

Since the pragma only applies to functions, maybe we should error here if the 
decl is not a FunctionDecl?



Comment at: clang/lib/Sema/SemaAttr.cpp:816
+  Decl *D = ND;
+  while (D != nullptr) {
+if (auto *FD = dyn_cast(D)) {

Instead of walking the decls, would checking isExternC() on the 
getCanonicalDecl() be enough?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128649

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

[PATCH] D128649: [clang-cl] Accept a pragma alloc_text corner case accepted by MSVC

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

This does make us accept, but MSVC doesn't:

  static void f();
  extern "C" {
static void f();
  }
  #pragma alloc_text(c, f);
  static void f() {}

MSVC and GCC fail with an error for conflicting declaration.
GCC: https://godbolt.org/z/Tavvx88E4
MSVC: https://godbolt.org/z/vj41TWdYh


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128649

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

[PATCH] D128649: [clang-cl] Accept a pragma alloc_text corner case accepted by MSVC

[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.

MSVC's pragma alloc_text accepts a function that was redeclared in
a non extern-C context if the previous declaration was in an extern-C
context. i.e.

  extern "C" { static void f(); }
  static void f();


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128649

Files:
  clang/lib/Sema/SemaAttr.cpp
  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
@@ -40,3 +40,10 @@
 #pragma alloc_text(c, foo6) // no-warning
 void foo6() {}
 }
+
+extern "C" {
+static void foo7();
+}
+static void foo7();
+#pragma alloc_text(c, foo7) // no-warning
+void foo7() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -810,10 +810,23 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
-  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
-  return;
+if (getLangOpts().CPlusPlus) {
+  bool IsInExternCContext = false;
+  Decl *D = ND;
+  while (D != nullptr) {
+if (auto *FD = dyn_cast(D)) {
+  if (FD->isInExternCContext()) {
+IsInExternCContext = true;
+break;
+  }
+}
+D = D->getPreviousDecl();
+  }
+
+  if (!IsInExternCContext) {
+Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+return;
+  }
 }
 
 FunctionToSectionMap[II->getName()] = std::make_tuple(Section, Loc);


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
@@ -40,3 +40,10 @@
 #pragma alloc_text(c, foo6) // no-warning
 void foo6() {}
 }
+
+extern "C" {
+static void foo7();
+}
+static void foo7();
+#pragma alloc_text(c, foo7) // no-warning
+void foo7() {}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -810,10 +810,23 @@
   return;
 }
 
-DeclContext *DC = ND->getDeclContext();
-if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
-  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
-  return;
+if (getLangOpts().CPlusPlus) {
+  bool IsInExternCContext = false;
+  Decl *D = ND;
+  while (D != nullptr) {
+if (auto *FD = dyn_cast(D)) {
+  if (FD->isInExternCContext()) {
+IsInExternCContext = true;
+break;
+  }
+}
+D = D->getPreviousDecl();
+  }
+
+  if (!IsInExternCContext) {
+Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+return;
+  }
 }
 
 FunctionToSectionMap[II->getName()] = std::make_tuple(Section, Loc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits