[PATCH] D54604: Automatic variable initialization

JF Bastien via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 13 13:50:21 PST 2018


jfb added a comment.

In D54604#1327893 <https://reviews.llvm.org/D54604#1327893>, @rsmith wrote:

> 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.)


I'm inclined to go with what @kcc wants, because I want the data he'll gather. For my own use, I don't really care. One thing I could do: check `__DATE__` and generate a warning if it's above a certain value and `zero` is used? i.e. check the date at which the compiler was compiled (not when it's used!) and error out if it was compiled after a certain point in time? I'm throwing out options that could makes folks more comfortable with `zero`...



================
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]>;
----------------
rsmith wrote:
> 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.
I definitely think that we want *some* attribute: the usecase is primarily to turn on automatic initialization for entire projects, and then turning it off manually where you measure unacceptable runtime costs (until you can do something better, or until the compiler fixes its perf problem). For example, large stack arrays would be something that you'd want to slap this attribute on.

That being said, I agree that `__uninitialized__` or `[[clang::uninitialized]]` would serve this purpose. I figured that mirroring the command-line options would be more desirable, but I'm not sure anymore. I've updated the patch to have `[[clang::uninitialized]]`.

I don't think we want user code to write it in the source: that's forcing user code to add semantics which their code otherwise doesn't have. It's existing practice, but IMO it's bad.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604





More information about the cfe-commits mailing list