[PATCH] D74361: [Clang] Uninitialize attribute on global variables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm not sure if it's a good idea to be re-using the existing attribute like this.  It's not that unreasonable, I guess, but I'd like to hear JF's thoughts.

I think you do need to update the documentation to mention the impact on globals.  For normal targets (i.e. ones with loaders), I assume this guarantees a zero pattern.  Except maybe not for thread-locals?  Do we have any targets where we can actually optimize thread-locals by suppressing initialization?

This will need to impact static analysis and the AST, I think.  Local variables can't be redeclared, but global variables can, so disallowing initializers syntactically when the attribute is present doesn't necessarily stop other declarations from defining the variable.  Also, you need to make the attribute mark something as a definition, the same way that e.g. the alias attribute does.  Also, this probably ought to disable the default-initialization of non-trivial types in C++, in case that's not already done.



================
Comment at: clang/test/Sema/attr-uninitialized.c:9
   int im_bad __attribute((uninitialized("zero")));  // expected-error {{'uninitialized' attribute takes no arguments}}
-  static int im_baaad __attribute((uninitialized)); // expected-warning {{'uninitialized' attribute only applies to local variables}}
 }
 
----------------
In general, you shouldn't remove the existing test lines; just modify them to not report a diagnostic.


================
Comment at: clang/test/Sema/attr-uninitialized.c:12
+void answer_right_now() __attribute((uninitialized)) {}                        // expected-warning {{'uninitialized' attribute only applies to variables}}
+void just_to_tell_you_once_again(__attribute((uninitialized)) int whos_bad) {}
 
----------------
This should still be diagnosed; it doesn't make any sense to apply this to parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361