Skip to content

[clang] Add test for CWG2811 "Clarify "use" of main" #96168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Jun 20, 2024

This patch covers CWG2811 "Clarify "use" of main", basically adding a test for -Wmain, focusing on usages of main in unevaluated contexts.

To my understanding, the diagnostic message is based on the wording, so I updated it based on the new wording. I also replaces "ISO C++ requires" with a phrasing that explicitly says "extension".

@Endilll Endilll added test-suite c++ clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Jun 20, 2024
@Endilll Endilll requested review from cor3ntin and AaronBallman June 20, 2024 11:13
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch covers CWG2811 "Clarify "use" of main", basically adding a test for -Wmain, focusing on usages of main in unevaluated contexts.

To my understanding, the diagnostic message is based on the wording, so I updated it based on the new wording. I also replaces "ISO C++ requires" with a phrasing that explicitly says "extension".


Full diff: https://github.com/llvm/llvm-project/pull/96168.diff

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1)
  • (modified) clang/test/CXX/basic/basic.start/basic.start.init/p3.cpp (+2-2)
  • (modified) clang/test/CXX/drs/cwg28xx.cpp (+19)
  • (modified) clang/www/cxx_dr_status.html (+1-1)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 14736784cff5f..b747d2637bd5f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -964,7 +964,7 @@ def err_main_global_variable :
 def warn_main_redefined : Warning<"variable named 'main' with external linkage "
     "has undefined behavior">, InGroup<Main>;
 def ext_main_used : Extension<
-  "ISO C++ does not allow 'main' to be used by a program">, InGroup<Main>;
+  "expressions that refer to 'main' are an extension">, InGroup<Main>;
 
 /// parser diagnostics
 def ext_no_declarators : ExtWarn<"declaration does not declare anything">,
diff --git a/clang/test/CXX/basic/basic.start/basic.start.init/p3.cpp b/clang/test/CXX/basic/basic.start/basic.start.init/p3.cpp
index 506232ebacc4c..8f215172235c2 100644
--- a/clang/test/CXX/basic/basic.start/basic.start.init/p3.cpp
+++ b/clang/test/CXX/basic/basic.start/basic.start.init/p3.cpp
@@ -16,9 +16,9 @@ int main(int argc, char **argv)
   = delete; // expected-error {{'main' is not allowed to be deleted}}
 #else
 {
-  int (*pmain)(int, char**) = &main; // expected-error {{ISO C++ does not allow 'main' to be used by a program}}
+  int (*pmain)(int, char**) = &main; // expected-error {{expressions that refer to 'main' are an extension}}
 
   if (argc)
-    main(0, 0); // expected-error {{ISO C++ does not allow 'main' to be used by a program}}
+    main(0, 0); // expected-error {{expressions that refer to 'main' are an extension}}
 }
 #endif
diff --git a/clang/test/CXX/drs/cwg28xx.cpp b/clang/test/CXX/drs/cwg28xx.cpp
index da81eccc8dc22..049d90a1f7b20 100644
--- a/clang/test/CXX/drs/cwg28xx.cpp
+++ b/clang/test/CXX/drs/cwg28xx.cpp
@@ -6,6 +6,25 @@
 // RUN: %clang_cc1 -std=c++23 -pedantic-errors -verify=expected,since-cxx20,since-cxx23 %s
 // RUN: %clang_cc1 -std=c++2c -pedantic-errors -verify=expected,since-cxx20,since-cxx23,since-cxx26 %s
 
+
+int main() {} // required for cwg2811
+
+namespace cwg2811 { // cwg2811: 3.5
+#if __cplusplus >= 201103L
+void f() {
+  (void)[&] {
+    using T = decltype(main);
+    // expected-error@-1 {{expressions that refer to 'main' are an extension}}
+  };
+  using T2 = decltype(main);
+  // expected-error@-1 {{expressions that refer to 'main' are an extension}}
+}
+
+using T = decltype(main);
+// expected-error@-1 {{expressions that refer to 'main' are an extension}}
+#endif
+} // namespace cwg2811
+
 namespace cwg2819 { // cwg2819: 19 tentatively ready 2023-12-01
 #if __cpp_constexpr >= 202306L
   constexpr void* p = nullptr;
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index dac38cedfcb75..d6b7c32dd844d 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -16675,7 +16675,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2811.html">2811</a></td>
     <td>DR</td>
     <td>Clarify "use" of main</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 3.5</td>
   </tr>
   <tr class="open" id="2812">
     <td><a href="https://cplusplus.github.io/CWG/issues/2812.html">2812</a></td>

@cor3ntin
Copy link
Contributor

I think I prefer the current wording, it's clearer that it's not conforming (at the very least we should say something like "clang extension". and maybe "referring to main is a clang extension".
LGTM otherwise

do we have test that this is fine?

namespace foo {
    int main();
    using a = decltype(&main);
}```

@Endilll
Copy link
Contributor Author

Endilll commented Jun 20, 2024

it's clearer that it's not conforming (at the very least we should say something like "clang extension"

As someone who was a user of Clang not so long ago, I have to say that the only thing "ISO C++ requires" conveyed to me was "you're doing something wrong". For me it never had a clear interpretation "you're asking for an extension, but we're not giving it to you". On the other hand, I believe the word "extension" is clearly understood to describe something outside of ISO C++, somewhat by definition. At least that would match my understanding 3-4 years ago.

CC @AaronBallman I remember we discussed it offline a while ago.

@Endilll
Copy link
Contributor Author

Endilll commented Jun 20, 2024

do we have test that this is fine?

namespace foo {
    int main();
    using a = decltype(&main);
}```

Thank you, I added the test.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if @AaronBallman is happy

("expressions that... are an extensions" still sounds weird to me, cf previous comment)

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Endilll Endilll merged commit 2b5d1fb into llvm:main Jun 21, 2024
7 checks passed
@Endilll Endilll deleted the cwg2811 branch June 21, 2024 09:50
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
This patch covers
[CWG2811](https://cplusplus.github.io/CWG/issues/2811.html) "Clarify
"use" of main", basically adding a test for `-Wmain`, focusing on usages
of `main` in unevaluated contexts.

To my understanding, the diagnostic message is based on the wording, so
I updated it based on the new wording. I also replaces "ISO C++
requires" with a phrasing that explicitly says "extension".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants