[PATCH] D74361: [Clang] Undef attribute for global variables

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 6 07:09:48 PST 2020


aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:2718
+      // attribute then there are two different definitions.
+      // In C++, prefer the standard diagnostics
+      if (!S.getLangOpts().CPlusPlus) {
----------------
Missing a full stop at the end of the sentence. The comment can probably be re-flowed with the preceding sentence too.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11949
 
+  // The LoaderUninitialized attribute acts as a definition (of undef)
+  if (VDecl->hasAttr<LoaderUninitializedAttr>()) {
----------------
Missing full stop.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:12377
+      }
+      if (Var->getStorageClass() == SC_Extern) {
+        Diag(Var->getLocation(), diag::err_loader_uninitialized_extern);
----------------
Should this either be calling `VarDecl::hasExternalStorage()` or looking at the linkage of the variable, rather than at the storage class written in the source?


================
Comment at: clang/test/CodeGen/attr-loader-uninitialized.c:4
+// CHECK: @tentative_attr_first = weak global i32 undef, align 4
+int tentative_attr_first __attribute__((loader_uninitialized));
+int tentative_attr_first;
----------------
I thought `err_loader_uninitialized_extern` says that the variable cannot have external linkage?


================
Comment at: clang/test/CodeGenCXX/attr-loader-uninitialized.cpp:4
+// CHECK: @defn = global i32 undef
+int defn  [[clang::loader_uninitialized]];
+
----------------
I thought `err_loader_uninitialized_extern` says that the variable cannot have external linkage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361





More information about the cfe-commits mailing list