[Sema] main can't be declared as global variable

[davide]

Summary

I modeled it in this way: every time we act on a variable declaration, we check if it's main and it's global, and in case it's true we emit a diagnostic. Actually, to completely fulfill the standard requirements we should also consider ill-formed anything that declares the name "main" with C linkage, but I'd like to tackle this separately. In particular I'm not sure if "name" refers only to variables or also to e.g. function names etc..
For reference, this was reported in PR24309.

[rsmith]

lib/Sema/SemaDecl.cpp

6110

This check should not apply in -ffreestanding mode.

6110

getAsString is not the best thing to use here; it produces a human-readable string (for diagnostics etc). Instead, check Name.isIdentifier() && Name.getAsIdentifierInfo()->isStr("main").

6110

In C, an external-linkage variable named main results in undefined behavior (because the behavior is undefined if there is no external-linkage function named main, and also if there is both a function and a variable named main), so we can diagnose that case too.

test/CXX/basic/basic.start/basic.start.main/p3.cpp

6–9

Maybe also test:

• an internal-linkage global variable named main (ill-formed in C++, OK in C)
• a function parameter named main (OK)
• a static data member named main (OK, even if defined in TU scope)
• a variable template named main (it looks like this is ill-formed, no diagnostic required, if the variable template has external linkage, and otherwise OK)
• a variable named main within a namespace (OK)
• a function-scope extern variable named main (ill-formed)
• a function-scope static variable named main (OK)

[davide]

Take two. The check looks a little bit ugly, maybe there's a shorter way to handle it. I also enhanced test coverage, and added diagnostic for the undefined behaviour in C as you suggested.

[davide]

Hi Richard, do you have any comments on the new patch?

[rsmith]

Maybe you could refactor the check to something like:

if (Name.isIdentifier() && Name.getAsIdentifierInfo()->isStr("main")
    NewVD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
    !getLangOpts().Freestanding && !NewVD->getDescribedVarTemplate()) {
  if (getLangOpts().CPlusPlus)
    Diag1;
  else if (NewVD->hasExternalFormalLinkage())
    Diag2;
}

include/clang/Basic/DiagnosticSemaKinds.td

514

Hyphenating 'external linkage' is unusual; how about 'variable named 'main' with external linkage has undefined behavior'?

lib/Sema/SemaDecl.cpp

6113

The freestanding check should apply in C too.

6114–6116

The only checks you need here are: 1) DeclContext's redecl context is the translation unit, and 2) NewVD is not a variable template. All the other checks are implied by the DC check. I'd suggest factoring these out into the main check.

6116

You need to check NewVD->getDeclContext()->getRedeclContext(), in case there are LinkageSpecDecls surrounding the variable (extern "C++" int main;).

6121

It's weird to ask isExternC from C. Use hasExternalFormalLinkage instead.

[davide]

• Refactored check
• Added test for -ffreestanding in C
• Changed the diagnostic emitted

Also, thanks for your time and guidance.

[rsmith]

Looks good other than the diagnostic wording.

include/clang/Basic/DiagnosticSemaKinds.td

513

It's much more common to use "cannot" rather than the contraction "can't" in our diagnostic wording.

514

I would still like this reworded.

[davide]

Oops, I uploaded the wrong diff. Anyway, I changed can't to cannot and committed this as r245051.
While looking at DiagnosticSemaKind.td I noticed there are still 7 diagnostics that use can't instead of cannot. Are you OK if I change those? (in a subsequent commit, of course)

include/clang/Basic/DiagnosticSemaKinds.td

 def note_main_change_return_type : Note<"change return type to 'int'">;
 def err_main_surplus_args : Error<"too many parameters (%0) for 'main': "
     "must be 0, 2, or 3">;
 def warn_main_one_arg : Warning<"only one parameter on 'main' declaration">,
     InGroup<Main>;
 def err_main_arg_wrong : Error<"%select{first|second|third|fourth}0 "
     "parameter of 'main' (%select{argument count|argument array|environment|"
     "platform-specific data}0) must be of type %1">;
+def err_main_global_variable : Error<"main can't be declared as global variable">;
+def warn_main_redefined : Warning<"external-linkage variable named 'main' has "
+    "undefined behavior">, InGroup<Main>;
 def ext_main_used : Extension<
   "ISO C++ does not allow 'main' to be used by a program">, InGroup<Main>;
 
 /// parser diagnostics
 def ext_no_declarators : ExtWarn<"declaration does not declare anything">,
   InGroup<MissingDeclarations>;
 def ext_typedef_without_a_name : ExtWarn<"typedef requires a name">,
   InGroup<MissingDeclarations>;

lib/Sema/SemaDecl.cpp

 
   if (getLangOpts().CPlusPlus && NewVD->isStaticLocal()) {
     Decl *ManglingContextDecl;
     if (MangleNumberingContext *MCtx = getCurrentMangleNumberContext(
             NewVD->getDeclContext(), ManglingContextDecl)) {
       Context.setManglingNumber(
           NewVD, MCtx->getManglingNumber(
                      NewVD, getMSManglingNumber(getLangOpts(), S)));
       Context.setStaticLocalNumber(NewVD, MCtx->getStaticLocalNumber(NewVD));
     }
   }
 
+  // Special handling of variable named 'main'.
+  if (Name.isIdentifier() && Name.getAsIdentifierInfo()->isStr("main") &&
+      NewVD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+      !getLangOpts().Freestanding && !NewVD->getDescribedVarTemplate()) {
+
+    // C++ [basic.start.main]p3
+    // A program that declares a variable main at global scope is ill-formed.
+    if (getLangOpts().CPlusPlus)
+      Diag(D.getLocStart(), diag::err_main_global_variable);
+
+    // In C, and external-linkage variable named main results in undefined
+    // behavior.
+    else if (NewVD->hasExternalFormalLinkage())
+      Diag(D.getLocStart(), diag::warn_main_redefined);
+  }
+
   if (D.isRedeclaration() && !Previous.empty()) {
     checkDLLAttributeRedeclaration(
         *this, dyn_cast<NamedDecl>(Previous.getRepresentativeDecl()), NewVD,
         IsExplicitSpecialization);
   }
 
   if (NewTemplate) {
     if (NewVD->isInvalidDecl())

test/CXX/basic/basic.start/basic.start.main/p3.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST1
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST2
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST3
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST4
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 -DTEST5
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 -DTEST6
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST7
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST8
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST9
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST10 -ffreestanding
+
+#if TEST1
+int main; // expected-error{{main can't be declared as global variable}}
+
+#elif TEST2
+// expected-no-diagnostics
+int f () {
+  int main;
+  return main;
+}
+
+#elif TEST3
+// expected-no-diagnostics
+void x(int main) {};
+int y(int main);
+
+#elif TEST4
+// expected-no-diagnostics
+class A {
+  static int main;
+};
+
+#elif TEST5
+// expected-no-diagnostics
+template<class T> constexpr T main;
+
+#elif TEST6
+extern template<class T> constexpr T main; //expected-error{{expected unqualified-id}}
+
+#elif TEST7
+// expected-no-diagnostics
+namespace foo {
+  int main;
+}
+
+#elif TEST8
+void z(void)
+{
+  extern int main;  // expected-error{{main can't be declared as global variable}}
+}
+
+#elif TEST9
+// expected-no-diagnostics
+int q(void)
+{
+  static int main;
+  return main;
+}
+
+#elif TEST10
+// expected-no-diagnostics
+int main;
+
+#else
+#error Unknown Test
+#endif

test/Sema/warn-extern-main.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST1
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST2
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST3
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST4
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST5
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST6
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST7
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST8
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DTEST9 -ffreestanding
+
+#if TEST1
+int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+
+#elif TEST2
+extern int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+
+#elif TEST3
+// expected-no-diagnostics
+void x() {
+  static int main;
+}
+
+#elif TEST4
+void x() {
+  extern int main; // expected-warning{{external-linkage variable named 'main' has undefined behavior}}
+}
+
+#elif TEST5
+// expected-no-diagnostics
+void x() {
+  int main;
+}
+
+#elif TEST6
+// expected-no-diagnostics
+static int main;
+
+#elif TEST7
+// expected-no-diagnostics
+void x() {
+  auto int main;
+}
+
+#elif TEST8
+// expected-no-diagnostics
+void x() {
+  register int main;
+}
+
+#elif TEST9
+// expected-no-diagnostics
+int main;
+
+#else
+#error Unknown Test
+#endif