rsmith added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7417-7421
@@ -7416,4 +7416,7 @@
InGroup<Varargs>;
-def warn_va_start_of_reference_type_is_undefined : Warning<
- "'va_start' has undefined behavior with reference types">, InGroup<Varargs>;
+def warn_va_start_type_is_undefined : Warning<
+ "'va_start' has undefined behavior with "
+ "%select{types that undergo default argument promotion|reference types|"
+ "an object declared with the register storage class}0">,
+ InGroup<Varargs>;
def err_first_argument_to_va_arg_not_of_type_va_list : Error<
----------------
aaron.ballman wrote:
> rsmith wrote:
> > I'm not sure that
> >
> > 'va_start' has undefined behavior with an object declared with the
> > register storage class
> >
> > is clear enough about what object it's talking about. Can you explicitly
> > state that the problem is with the type of the parameter name passed to
> > `va_start` here, somehow?
> How about: "'va_start' has undefined behavior with a parameter declared with
> the 'register' keyword"?
>
> Then again, we could reword the entire diagnostic and I wouldn't be sad. I
> notice that it's using the plural "types" already instead of the singular
> form, for instance. How about:
>
> "Passing %select{an object of reference type|an object that undergoes default
> argument promotion|a parameter declared with the 'register' keyword}0 to
> 'va_start' is undefined behavior"
I think either of these is fine. (Though in the latter one, I'd say "has
undefined behavior" or "results in undefined behavior" rather than "is
undefined behavior".)
================
Comment at: lib/Sema/SemaChecking.cpp:2722
@@ -2720,2 +2721,3 @@
ParamLoc = PV->getLocation();
+ IsRegister = PV->getStorageClass() == SC_Register;
}
----------------
aaron.ballman wrote:
> rsmith wrote:
> > Where is this restriction specified? Does this only apply to C?
> 7.16.1.4p3:
>
> If the parameter parmN is declared with the register storage class ... the
> behavior is undefined.
>
> I think this may only apply in C, actually, because [support.runtime]p3 does
> not mention it in its description (even as far back as C++03). Whether that's
> an explicit omission (to make va_start() more powerful than in C) or an
> oversight is kind of moot since register was removed in C++17. Thoughts?
I think it's an intentional difference with C. The intent of disallowing
`register` is to allow `va_start` to be implemented as a macro that takes the
address of the parameter. This would be ill-formed in C for a `register`
parameter, whereas in C++ it is (was) valid to take the address of a variable
declared with the `register` specifier.
http://reviews.llvm.org/D19244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
