EricWF added inline comments.
================
Comment at: lib/Sema/SemaDecl.cpp:10484-10485
@@ -10478,1 +10483,4 @@
+ if (var->hasAttr<RequireConstantInitAttr>() && !Init)
+ Diag(var->getLocation(), diag::err_require_constant_init_failed);
+
----------------
rsmith wrote:
> I think this check is incorrect: we perform constant initialization (to zero)
> for globals with no initializer.
Agreed. Technically not "constant initialization" but every bit as safe.
================
Comment at: lib/Sema/SemaDecl.cpp:10485
@@ +10484,3 @@
+ if (var->hasAttr<RequireConstantInitAttr>() && !Init)
+ Diag(var->getLocation(), diag::err_require_constant_init_failed);
+
----------------
majnemer wrote:
> Any reason not to use the already existing `err_init_element_not_constant`?
> Any reason not to use the already existing err_init_element_not_constant?
I hadn't considered it, but the error text seems misleading, since it may
select a constructor that's not a valid constant expression even when every
element in the initializer is.
================
Comment at: lib/Sema/SemaDecl.cpp:10500
@@ +10499,3 @@
+ if (!*HasConstInit)
+ Diag(var->getLocation(), diag::warn_global_constructor)
+ << Init->getSourceRange();
----------------
rsmith wrote:
> Instead of diagnosing the condition separately (and getting both a warning
> and an error for the same situation), it would seem preferable to change this
> to produce either `diag::warn_global_constructor` or your new error depending
> on whether the attribute is present. This would also remove the duplicate
> error messages if the attribute is specified on an object that is also marked
> `constexpr`.
Already I think I've dealt with the duplicate diagnostics.
================
Comment at: lib/Sema/SemaDecl.cpp:10528
@@ +10527,3 @@
+ var->getLocation())) {
+ // Warn about globals which don't have a constant initializer. Don't
+ // warn about globals with a non-trivial destructor because we already
----------------
I can't figure out where the diagnostic is this comment is coming from.
Hopefully I'm just missing something simple.
https://reviews.llvm.org/D23385
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
