Message ID | 20170420150920.GO3412@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Apr 20, 2017 at 04:09:20PM +0100, Jonathan Wakely wrote: > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -3128,11 +3128,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, > { > warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " > "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type)); > - inform (input_location, "uses %qD, which does not have an alignment " > - "parameter", alloc_fn); > - if (!aligned_new_threshold) > - inform (input_location, "use %<-faligned-new%> to enable C++17 " > - "over-aligned new support"); > + if (diagnostic_report_warnings_p (global_dc, input_location)) > + { > + inform (input_location, "uses %qD, which does not have an alignment " > + "parameter", alloc_fn); > + if (!aligned_new_threshold) > + inform (input_location, "use %<-faligned-new%> to enable C++17 " > + "over-aligned new support"); > + } This looks weird. I'd expect instead: if (warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type))) { inform (input_location, "uses %qD, which does not have an alignment " "parameter", alloc_fn); if (!aligned_new_threshold) inform (input_location, "use %<-faligned-new%> to enable C++17 " "over-aligned new support"); } That is a standard idiom used if some inform or later warning/error depends on whether earlier warning/error has been diagnosed. If that works, this is ok for trunk and 7.1 (we don't have a rc1 yet, it is ok now). Jakub
On 20/04/17 17:22 +0200, Jakub Jelinek wrote: >On Thu, Apr 20, 2017 at 04:09:20PM +0100, Jonathan Wakely wrote: >> --- a/gcc/cp/init.c >> +++ b/gcc/cp/init.c >> @@ -3128,11 +3128,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, >> { >> warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " >> "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type)); >> - inform (input_location, "uses %qD, which does not have an alignment " >> - "parameter", alloc_fn); >> - if (!aligned_new_threshold) >> - inform (input_location, "use %<-faligned-new%> to enable C++17 " >> - "over-aligned new support"); >> + if (diagnostic_report_warnings_p (global_dc, input_location)) >> + { >> + inform (input_location, "uses %qD, which does not have an alignment " >> + "parameter", alloc_fn); >> + if (!aligned_new_threshold) >> + inform (input_location, "use %<-faligned-new%> to enable C++17 " >> + "over-aligned new support"); >> + } > >This looks weird. I'd expect instead: > if (warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " > "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type))) > { > inform (input_location, "uses %qD, which does not have an alignment " > "parameter", alloc_fn); > if (!aligned_new_threshold) > inform (input_location, "use %<-faligned-new%> to enable C++17 " > "over-aligned new support"); > } >That is a standard idiom used if some inform or later warning/error depends >on whether earlier warning/error has been diagnosed. Aha, thanks. >If that works, this is ok for trunk and 7.1 (we don't have a rc1 yet, it is >ok now). OK, I'll test it now.
On Thu, Apr 20, 2017 at 05:22:00PM +0200, Jakub Jelinek wrote: > On Thu, Apr 20, 2017 at 04:09:20PM +0100, Jonathan Wakely wrote: > > --- a/gcc/cp/init.c > > +++ b/gcc/cp/init.c > > @@ -3128,11 +3128,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, > > { > > warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " > > "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type)); > > - inform (input_location, "uses %qD, which does not have an alignment " > > - "parameter", alloc_fn); > > - if (!aligned_new_threshold) > > - inform (input_location, "use %<-faligned-new%> to enable C++17 " > > - "over-aligned new support"); > > + if (diagnostic_report_warnings_p (global_dc, input_location)) > > + { > > + inform (input_location, "uses %qD, which does not have an alignment " > > + "parameter", alloc_fn); > > + if (!aligned_new_threshold) > > + inform (input_location, "use %<-faligned-new%> to enable C++17 " > > + "over-aligned new support"); > > + } > > This looks weird. I'd expect instead: > if (warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " > "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type))) > { > inform (input_location, "uses %qD, which does not have an alignment " > "parameter", alloc_fn); > if (!aligned_new_threshold) > inform (input_location, "use %<-faligned-new%> to enable C++17 " > "over-aligned new support"); > } > That is a standard idiom used if some inform or later warning/error depends > on whether earlier warning/error has been diagnosed. Yes. > If that works, this is ok for trunk and 7.1 (we don't have a rc1 yet, it is > ok now). One more thing, the test passes even without the patch. Did you mean to add // { dg-options "-Wall -w" } ? Marek
On 20/04/17 17:33 +0200, Marek Polacek wrote: >On Thu, Apr 20, 2017 at 05:22:00PM +0200, Jakub Jelinek wrote: >> On Thu, Apr 20, 2017 at 04:09:20PM +0100, Jonathan Wakely wrote: >> > --- a/gcc/cp/init.c >> > +++ b/gcc/cp/init.c >> > @@ -3128,11 +3128,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, >> > { >> > warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " >> > "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type)); >> > - inform (input_location, "uses %qD, which does not have an alignment " >> > - "parameter", alloc_fn); >> > - if (!aligned_new_threshold) >> > - inform (input_location, "use %<-faligned-new%> to enable C++17 " >> > - "over-aligned new support"); >> > + if (diagnostic_report_warnings_p (global_dc, input_location)) >> > + { >> > + inform (input_location, "uses %qD, which does not have an alignment " >> > + "parameter", alloc_fn); >> > + if (!aligned_new_threshold) >> > + inform (input_location, "use %<-faligned-new%> to enable C++17 " >> > + "over-aligned new support"); >> > + } >> >> This looks weird. I'd expect instead: >> if (warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " >> "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type))) >> { >> inform (input_location, "uses %qD, which does not have an alignment " >> "parameter", alloc_fn); >> if (!aligned_new_threshold) >> inform (input_location, "use %<-faligned-new%> to enable C++17 " >> "over-aligned new support"); >> } >> That is a standard idiom used if some inform or later warning/error depends >> on whether earlier warning/error has been diagnosed. > >Yes. > >> If that works, this is ok for trunk and 7.1 (we don't have a rc1 yet, it is >> ok now). > >One more thing, the test passes even without the patch. Did you mean to add >// { dg-options "-Wall -w" } >? Good catch. My original testcase used <type_traits> but I changed it to not depend on the library, and didn't fix the dg-options.
commit 66697a6551dd13715c2156ac6b2d5969010c12db Author: Jonathan Wakely <jwakely@redhat.com> Date: Thu Apr 20 15:24:09 2017 +0100 PR c++/80473 allow suppressing notes about over-aligned new gcc/cp: PR c++/80473 * init.c (build_new_1): Suppress notes about over-aligned new when the warning is suppressed. gcc/testsuite: PR c++/80473 * g++.dg/diagnostic/pr80473.C: New test. diff --git a/gcc/cp/init.c b/gcc/cp/init.c index bfa9020..2e72451 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3128,11 +3128,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts, { warning (OPT_Waligned_new_, "%<new%> of type %qT with extended " "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type)); - inform (input_location, "uses %qD, which does not have an alignment " - "parameter", alloc_fn); - if (!aligned_new_threshold) - inform (input_location, "use %<-faligned-new%> to enable C++17 " - "over-aligned new support"); + if (diagnostic_report_warnings_p (global_dc, input_location)) + { + inform (input_location, "uses %qD, which does not have an alignment " + "parameter", alloc_fn); + if (!aligned_new_threshold) + inform (input_location, "use %<-faligned-new%> to enable C++17 " + "over-aligned new support"); + } } /* If we found a simple case of PLACEMENT_EXPR above, then copy it diff --git a/gcc/testsuite/g++.dg/diagnostic/pr80473.C b/gcc/testsuite/g++.dg/diagnostic/pr80473.C new file mode 100644 index 0000000..6a739f5 --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/pr80473.C @@ -0,0 +1,14 @@ +// { dg-do compile { target c++11 } } +// { dg-bogus "over-aligned new" "PR c++/80473" { target *-*-* } 0 } +template<typename T> T&& declval(); + +template<typename T, typename U, typename = void> +struct is_constructible { enum { value = 0 }; }; + +template<typename T, typename U> +struct is_constructible<T, U, decltype(::new T(declval<U>()), void())> +{ enum { value = 1 }; }; + +struct alignas(64) A { int i; }; + +constexpr bool b = is_constructible<A, A>::value;