[PATCH] D54604: Automatic variable initialization

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

For the record: I'm OK with this direction. (I somewhat prefer removing the `-enable-long-wordy-thing` option and instead automatically disabling the `zero` option for clang release builds, but I'm OK with either approach.)



================
Comment at: include/clang/Basic/Attr.td:3134-3142
+
+def TrivialAutoInit : InheritableAttr {
+  let Spellings = [Clang<"trivial_auto_init", 1>];
+  let Args = [EnumArgument<"TrivialAutoVarInit", "TrivialAutoVarInitKind",
+                           ["uninitialized", "zero", "pattern"],
+                           ["Uninitialized", "Zero", "Pattern"]>];
+  let Subjects = SubjectList<[LocalVar]>;
----------------
I'm unconvinced that we should add this attribute; it seems insufficiently motivated to me, and harmful to portability. I think we should have some way of saying "leave this uninitialized", but zero- and pattern-initialization really seem like things that should simply be written in the source code in the normal way.

I think it would be reasonable to have a `__uninitialized__` keyword or similar that can be used as the initializer of an explicitly-uninitialized variable. If you want an attribute for this (for compatibility with other compilers), `[[clang::uninitialized]]` also seems reasonable. But absent a strong motivation, I do not think we should have an attribute that specifies a particular initial value.


================
Comment at: include/clang/Basic/AttrDocs.td:3710-3714
+The command-line parameter ``-ftrivial-auto-var-init=*`` can be used to
+automatically initialize trivial automatic stack variables. By default, trivial
+automatic stack variables are uninitialized. This attribute is used to override
+the command-line parameter, and accepts a single parameter that must be one of
+the following: ``uninitialized``, ``zero``, or ``pattern``.
----------------
If we keep the attribute, we should not document the `zero` mode. We don't need to make it discoverable.


================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:409-412
+def warn_drv_trivial_auto_var_init_zero_disabled : Warning<
+  "-ftrivial-auto-var-init=zero hasn't been enabled, using pattern initialization instead. "
+  "Enable it at your own peril for benchmarking purpose only with "
+  "-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">;
----------------
I think this should be an error instead of a warning. We shouldn't silently do something different from what was requested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604