Re: [v3 PATCH] Implement N4387 and LWG 2367
On 7 June 2015 at 09:53, Marc Glisse wrote: > Since the paper does not mention looking at _MoveConstructibleTuple or > _ImplicitlyMoveConvertibleTuple here, could you add a comment explaining > why that is needed? Sure. > Does the following code still compile with your patch? > struct A { int a,b; }; > std::tuple a(3,4,{1,2}); No. :/ And we have no test for it.. I'll need to look at that. > IMO the parts with is_default_constructible point to a core issue, we should > not have to duplicate information, especially in such a convoluted way. But > I guess that has lower priority than noexcept(auto), and I haven't yet > looked if concepts will help. Concepts would help a lot, but being able to use them in a library implementation is some ways off. > You use a lot: typename enable_if::type=true > while the current code seems to favor: class=typename enable_if::type. > I don't really care which one is used, but it is easier to read when the > style is consistent through the library. It's not a style issue. That template parameter needs to be a non-type one, otherwise the overloads are ambiguous. > Introducing > typename _XXX = _TC<(sizeof...(_Elements) == sizeof...(_UElements)), > _Elements...> > and then using _XXX::template thing() might give less clutter when you have > to repeat it 4 times. Sounds good, I'll give it a spin.
Re: [v3 PATCH] Implement N4387 and LWG 2367
On 7 June 2015 at 11:12, Ville Voutilainen wrote: >> Does the following code still compile with your patch? >> struct A { int a,b; }; >> std::tuple a(3,4,{1,2}); > > No. :/ And we have no test for it.. I'll need to look at that. Ahem, yes, this is because the constructor that used to take _Elements now takes _UElements. I can change it back to take _Elements, because the technique that the default constructors use allow making the signature dependent enough that it will sfinae correctly. >> You use a lot: typename enable_if::type=true >> while the current code seems to favor: class=typename enable_if::type. >> I don't really care which one is used, but it is easier to read when the >> style is consistent through the library. > It's not a style issue. That template parameter needs to be a non-type one, > otherwise the overloads are ambiguous. ...and I think it doesn't necessarily need to be non-type, I think it can be made to work with a type parameter that is enable_if and enable_if for the mutually-exclusive overloads.
Re: [v3 PATCH] Implement N4387 and LWG 2367
On Sun, 7 Jun 2015, Ville Voutilainen wrote: On 7 June 2015 at 11:12, Ville Voutilainen wrote: Does the following code still compile with your patch? struct A { int a,b; }; std::tuple a(3,4,{1,2}); No. :/ And we have no test for it.. I'll need to look at that. Ahem, yes, this is because the constructor that used to take _Elements now takes _UElements. I can change it back to take _Elements, because the technique that the default constructors use allow making the signature dependent enough that it will sfinae correctly. Yes. You use a lot: typename enable_if::type=true while the current code seems to favor: class=typename enable_if::type. I don't really care which one is used, but it is easier to read when the style is consistent through the library. It's not a style issue. That template parameter needs to be a non-type one, otherwise the overloads are ambiguous. Ah, I had overlooked that. I have seen several work-arounds for this issue, but I don't remember this one, it seems nice. -- Marc Glisse
Re: [v3 PATCH] Implement N4387 and LWG 2367
On 7 June 2015 at 11:33, Ville Voutilainen wrote: >>> You use a lot: typename enable_if::type=true >>> while the current code seems to favor: class=typename enable_if::type. >>> I don't really care which one is used, but it is easier to read when the >>> style is consistent through the library. >> It's not a style issue. That template parameter needs to be a non-type one, >> otherwise the overloads are ambiguous. > ...and I think it doesn't necessarily need to be non-type, I think it can be > made to work with a type parameter that is enable_if > and enable_if for the mutually-exclusive overloads. Except that no, it can't. It really needs to be a non-type parameter. :P
[patch] fix _OBJC_Module defined but not used warning
On 06/07/2015 06:19 AM, Andreas Schwab wrote: Another fallout: FAIL: obj-c++.dg/try-catch-5.mm -fgnu-runtime (test for excess errors) Excess errors: : warning: '_OBJC_Module' defined but not used [-Wunused-variable] check_global_declarations is called for more symbols now. All the defined but not used errors I've seen in development have been legitimate. For tests, the tests should be fixed. For built-ins such as these, does the attached fix the problem? It is up to the objc maintainers, we can either fix this with the attached patch, or setting DECL_IN_SYSTEM_HEADER. /* Nonzero for a given ..._DECL node means that no warnings should be generated just because this node is unused. */ #define DECL_IN_SYSTEM_HEADER(NODE) \ (in_system_header_at (DECL_SOURCE_LOCATION (NODE))) Let me know what you prefer. Aldy commit 25ce72372f7b1309004b87810140573b422e1355 Author: Aldy Hernandez Date: Sun Jun 7 07:32:12 2015 -0400 * objc-runtime-shared-support.c (build_module_descriptor): Set TREE_USED on UOBJC_MODULES_decl. diff --git a/gcc/objc/objc-runtime-shared-support.c b/gcc/objc/objc-runtime-shared-support.c index d9b3c27..1bcb14a 100644 --- a/gcc/objc/objc-runtime-shared-support.c +++ b/gcc/objc/objc-runtime-shared-support.c @@ -519,6 +519,9 @@ build_module_descriptor (long vers, tree attr) is referenced by the runtime and, therefore, needed. */ DECL_PRESERVE_P (UOBJC_MODULES_decl) = 1; + /* Squash `defined but not used' warning. */ + TREE_USED (UOBJC_MODULES_decl) = 1; + /* Allow the runtime to mark meta-data such that it can be assigned to target specific sections by the back-end. */ if (attr)
Re: [patch] fix _OBJC_Module defined but not used warning
Aldy Hernandez writes: > check_global_declarations is called for more symbols now. All the defined > but not used errors I've seen in development have been legitimate. For > tests, the tests should be fixed. For built-ins such as these, does the > attached fix the problem? Yes, it does. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
New Swedish PO file for 'gcc' (version 5.1.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Swedish team of translators. The file is available at: http://translationproject.org/latest/gcc/sv.po (This file, 'gcc-5.1.0.sv.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
[PATCH] Refactor -Wmisleading-indentation API and extend coverage
This patch refactors the entry point of -Wmisleading-indentation from: void warn_for_misleading_indentation (location_t guard_loc, location_t body_loc, location_t next_stmt_loc, enum cpp_ttype next_tok_type, const char *guard_kind); to struct common_token_info { location_t location; cpp_ttype type; rid keyword; }; void warn_for_misleading_indentation (const common_token_info &guard_tinfo, const common_token_info &body_tinfo, const common_token_info &next_tinfo); The purpose of this refactoring is to expose more information to the -Wmisleading-indentation implementation to allow for more advanced heuristics and for better coverage. This change of API of course required changes to the C and C++ frontend. Amidst these minor changes I also made sure in both frontends that warn_for_misleading_indentation() gets called even when the body of the guard in question is a semicolon. This allows us to implement warnings about dubious semicolons. Also I moved the keyword != RID_ELSE checks guarding the call to warn_for_misleading_indentation() to the function itself itself. At the same time this patch extends the coverage of the -Wmisleading-indentation implementation to catch misleading indentation involving the semicolon. (These changes are all contained in c-indentation.c.) For example, now we warn on the following code samples: if (flag); foo (); while (flag); { ... } if (flag); { ... } if (flag) ; /* blah */ { ... } if (flag); foo (); while avoiding to warn on code that is poorly indented but not misleadingly so; while (flag); foo (); while (flag) ; foo (); if (flag1) ; if (flag) ; else ... Finally this patch reverts the fix to PR 66620 because it regresses the following test cases: if (foo) bar (); else if (baz) bar (); bar (); // No warning, because guard_vis_column > body_vis_column The better fix for this PR IMO is to bail out early when the the body is a braced compound statement. This is what the patch does. In general, when the body of the guard in question is braced then control flow is visually explicit despite any potentially sloppy indentation. This restriction could always be relaxed however. Bootstrap and regtest on x86_64-linux in progress, OK to commit if succeeds? gcc/c-family/ChangeLog: * c-common.h (struct common_token_info): Define. (extract_token_info): Define. (warn_for_misleading_information): Change declaration to take three common_token_infos. * c-identation.c (should_warn_for_misleading_indentation): Likewise. Bail out early if the body is a compound statement or if the next token is the else keyword. Float out the definition of guard_exploc. Don't warn on code containing a semicolon body on its own line. Remove fix for PR 66220. Add heuristics for when a semicolon body is on the same line as the guard. (guard_tinfo_to_string): Define. (warn_for_misleading_indentation): Change declaration to take three common_token_infos. Adjust accordingly. gcc/c/ChangeLog: * c-parser.c (c_parser_if_body): Call warn_for_misleading_indentation even when the body is a semicolon. Extract common_token_infos corresponding to the guard, body and next tokens. Adjust call to warn_for_misleading_indentation accordingly. (c_parser_else_body): Likewise. (c_parser_if_statement): Likewise. (c_parser_while_statement): Likewise. (c_parser_for_statement): Likewise. gcc/cp/ChangeLog: * parser.c (cp_parser_selection_statement): Move handling of semicolon body to ... (cp_parser_implicitly_scoped_statement): .. here. Call warn_for_misleading_indentation even when the body is a semicolon. Extract common_token_infos corresponding to the guard, body and next tokens. Adjust call to warn_for_misleading_indentation accordingly. (cp_parser_already_scoped_statement): Likewise. (cp_parser_selection_statement, cp_parser_iteration_statement): Extract a common_token_info corresponding to the guard token. gcc/testsuite/ChangeLog: * c-c++-common/Wmisleading-indentation.c: Augment test. --- gcc/c-family/c-common.h| 34 - gcc/c-family/c-indentation.c | 144 - gcc/c/c-parser.c | 82 ++-- gcc/cp/parser.c| 99 ++ .../c-c++-common/Wmisleading-indentation.c | 99 ++ 5 files changed, 325 insertions(+), 133 deletions(-) diff --git a/gcc/c
[gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives
Hi Cesar! This patch fixes checks of OpenMP and OpenACC continuations in case if someone mixes them (i.e. continues OpenMP directive with !$ACC sentinel or vice versa). OK for gomp branch? -- Ilmir. From 5492bf5bc991b6924f5e3b35c11eeaed745df073 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Sun, 7 Jun 2015 23:55:22 +0300 Subject: [PATCH] Fix mix of OpenACC and OpenMP sentinels in continuation --- gcc/fortran/ChangeLog | 5 + gcc/fortran/scanner.c | 28 gcc/testsuite/ChangeLog | 5 + gcc/testsuite/gfortran.dg/goacc/omp.f95 | 8 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index 67f9e09..f61e0e9 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,8 @@ +2015-06-07 Ilmir Usmanov + + * scanner.c (gfc_next_char_literal): Fix mix of OpenACC and OpenMP + sentinels in continuation. + 2015-05-05 David Malcolm * expr.c (check_inquiry): Fix indentation so that it reflects the diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c index f0e6404..5af4eea 100644 --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -1331,7 +1331,7 @@ restart: continue_line = gfc_linebuf_linenum (gfc_current_locus.lb); if (flag_openmp) - if (prev_openmp_flag != openmp_flag) + if (prev_openmp_flag != openmp_flag && !openacc_flag) { gfc_current_locus = old_loc; openmp_flag = prev_openmp_flag; @@ -1340,7 +1340,7 @@ restart: } if (flag_openacc) - if (prev_openacc_flag != openacc_flag) + if (prev_openacc_flag != openacc_flag && !openmp_flag) { gfc_current_locus = old_loc; openacc_flag = prev_openacc_flag; @@ -1359,7 +1359,7 @@ restart: while (gfc_is_whitespace (c)) c = next_char (); - if (openmp_flag) + if (openmp_flag && !openacc_flag) { for (i = 0; i < 5; i++, c = next_char ()) { @@ -1370,7 +1370,7 @@ restart: while (gfc_is_whitespace (c)) c = next_char (); } - if (openacc_flag) + if (openacc_flag && !openmp_flag) { for (i = 0; i < 5; i++, c = next_char ()) { @@ -1382,6 +1382,26 @@ restart: c = next_char (); } + /* In case we have an OpenMP directive continued by OpenACC + sentinel, or vice versa, we get both openmp_flag and + openacc_flag on. */ + + if (openacc_flag && openmp_flag) + { + int is_openmp = 0; + for (i = 0; i < 5; i++, c = next_char ()) + { + if (gfc_wide_tolower (c) != (unsigned char) "!$acc"[i]) + is_openmp = 1; + if (i == 4) + old_loc = gfc_current_locus; + } + gfc_error ("Wrong %s continuation at %C: expected %s, got %s", + is_openmp ? "OpenACC" : "OpenMP", + is_openmp ? "!$ACC" : "!$OMP", + is_openmp ? "!$OMP" : "!$ACC"); + } + if (c != '&') { if (in_string) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 7c4781c..05a9a52 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2015-06-07 Ilmir Usmanov + + * gfortran.dg/goacc/omp.f95: Add mix of OpenACC and OpenMP + sentinels in continuation to test. + 2015-05-06 Yvan Roux PR target/64208 diff --git a/gcc/testsuite/gfortran.dg/goacc/omp.f95 b/gcc/testsuite/gfortran.dg/goacc/omp.f95 index 24f639f..a7333eb 100644 --- a/gcc/testsuite/gfortran.dg/goacc/omp.f95 +++ b/gcc/testsuite/gfortran.dg/goacc/omp.f95 @@ -63,4 +63,12 @@ contains !$omp end parallel !$acc end data end subroutine roku + + subroutine nana + !$acc parallel & + !$omp do ! { dg-error "Wrong OpenACC continuation" } + + !$omp parallel & + !$acc loop ! { dg-error "Wrong OpenMP continuation" } + end subroutine nana end module test \ No newline at end of file -- 1.9.1
Re: [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives
Fixed fortran mail-list address. Sorry for inconvenience. 08.06.2015, 00:01, "Ilmir Usmanov" : > Hi Cesar! > > This patch fixes checks of OpenMP and OpenACC continuations in case if > someone mixes them (i.e. continues OpenMP directive with !$ACC sentinel or > vice versa). > > OK for gomp branch? > > -- > Ilmir. -- Ilmir. From 5492bf5bc991b6924f5e3b35c11eeaed745df073 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Sun, 7 Jun 2015 23:55:22 +0300 Subject: [PATCH] Fix mix of OpenACC and OpenMP sentinels in continuation --- gcc/fortran/ChangeLog | 5 + gcc/fortran/scanner.c | 28 gcc/testsuite/ChangeLog | 5 + gcc/testsuite/gfortran.dg/goacc/omp.f95 | 8 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index 67f9e09..f61e0e9 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,8 @@ +2015-06-07 Ilmir Usmanov + + * scanner.c (gfc_next_char_literal): Fix mix of OpenACC and OpenMP + sentinels in continuation. + 2015-05-05 David Malcolm * expr.c (check_inquiry): Fix indentation so that it reflects the diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c index f0e6404..5af4eea 100644 --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -1331,7 +1331,7 @@ restart: continue_line = gfc_linebuf_linenum (gfc_current_locus.lb); if (flag_openmp) - if (prev_openmp_flag != openmp_flag) + if (prev_openmp_flag != openmp_flag && !openacc_flag) { gfc_current_locus = old_loc; openmp_flag = prev_openmp_flag; @@ -1340,7 +1340,7 @@ restart: } if (flag_openacc) - if (prev_openacc_flag != openacc_flag) + if (prev_openacc_flag != openacc_flag && !openmp_flag) { gfc_current_locus = old_loc; openacc_flag = prev_openacc_flag; @@ -1359,7 +1359,7 @@ restart: while (gfc_is_whitespace (c)) c = next_char (); - if (openmp_flag) + if (openmp_flag && !openacc_flag) { for (i = 0; i < 5; i++, c = next_char ()) { @@ -1370,7 +1370,7 @@ restart: while (gfc_is_whitespace (c)) c = next_char (); } - if (openacc_flag) + if (openacc_flag && !openmp_flag) { for (i = 0; i < 5; i++, c = next_char ()) { @@ -1382,6 +1382,26 @@ restart: c = next_char (); } + /* In case we have an OpenMP directive continued by OpenACC + sentinel, or vice versa, we get both openmp_flag and + openacc_flag on. */ + + if (openacc_flag && openmp_flag) + { + int is_openmp = 0; + for (i = 0; i < 5; i++, c = next_char ()) + { + if (gfc_wide_tolower (c) != (unsigned char) "!$acc"[i]) + is_openmp = 1; + if (i == 4) + old_loc = gfc_current_locus; + } + gfc_error ("Wrong %s continuation at %C: expected %s, got %s", + is_openmp ? "OpenACC" : "OpenMP", + is_openmp ? "!$ACC" : "!$OMP", + is_openmp ? "!$OMP" : "!$ACC"); + } + if (c != '&') { if (in_string) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 7c4781c..05a9a52 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2015-06-07 Ilmir Usmanov + + * gfortran.dg/goacc/omp.f95: Add mix of OpenACC and OpenMP + sentinels in continuation to test. + 2015-05-06 Yvan Roux PR target/64208 diff --git a/gcc/testsuite/gfortran.dg/goacc/omp.f95 b/gcc/testsuite/gfortran.dg/goacc/omp.f95 index 24f639f..a7333eb 100644 --- a/gcc/testsuite/gfortran.dg/goacc/omp.f95 +++ b/gcc/testsuite/gfortran.dg/goacc/omp.f95 @@ -63,4 +63,12 @@ contains !$omp end parallel !$acc end data end subroutine roku + + subroutine nana + !$acc parallel & + !$omp do ! { dg-error "Wrong OpenACC continuation" } + + !$omp parallel & + !$acc loop ! { dg-error "Wrong OpenMP continuation" } + end subroutine nana end module test \ No newline at end of file -- 1.9.1
Re: Get LTO correct for Fortran C_PTR type
Hi, this is variant of patch I ended up comitting. Tobias: I would still welcome a final word about validity of my fortran testcase. The original version run into checking failures with nested functions tranpolines because we now all pointers have the same TYPE_CANONICAL that makes useless_type_conversion_p to return true when converting function pointer to non-function. This eventually triggers verify_stmt ICE because we insist on gimple_call to have function pointer as parameter. To fix this, I simply reordered the existing checks so we do not return early. I will be able to revert this once the followup patch to improve canonical types on pointers gets in. In general however useless_type_conversion_p IMO should not be tied to canonical type machinery. useless_type_conversion_p is about two types being compatible in a sense that all operations on inner type have same semantics as operations on outer type. TBAA notion of canonical types for LTO is weaker than that due to the need to produce equivalence classes. I suppose the TYPE_CANONICAL check is an useful compile time optimization for cases where canonical types have same strength, but we ought to add sanity checks for that. I will send patch for that. Finally I noticed that I ended up comitting wrong version of the c-compatible-types testcase and replaced it by the final one. I did not found a way to test for short-enums in LTO test, but I simply added enum value of INT_MAX that makes it sure the enum will be large enough with -fshort-enum build for testcase to pass. Bootstrapped/regtested ppc64le-linux after working around the miscompare, comitted. Honza Index: ChangeLog === --- ChangeLog (revision 224200) +++ ChangeLog (working copy) @@ -1,3 +1,12 @@ +2015-06-06 Jan Hubicka + + * alias.c (get_alias_set): Be ready for TYPE_CANONICAL + of ptr_type_node to not be ptr_to_node. + * tree.c (gimple_types_compatible_p): Do not match TREE_CODE of + TREE_TYPE of pointers. + * gimple-expr.c (useless_type_conversion): Reorder the check for + function pointers and TYPE_CANONICAL. + 2015-06-06 John David Anglin PR bootstrap/66319 Index: alias.c === --- alias.c (revision 224200) +++ alias.c (working copy) @@ -1072,8 +1072,9 @@ } /* In LTO the rules above needs to be part of canonical type machinery. For now just punt. */ - else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p) -set = get_alias_set (ptr_type_node); + else if (POINTER_TYPE_P (t) + && t != TYPE_CANONICAL (ptr_type_node) && in_lto_p) +set = get_alias_set (TYPE_CANONICAL (ptr_type_node)); /* Otherwise make a new alias set for this type. */ else Index: gimple-expr.c === --- gimple-expr.c (revision 224200) +++ gimple-expr.c (working copy) @@ -84,6 +84,12 @@ if (TYPE_ADDR_SPACE (TREE_TYPE (outer_type)) != TYPE_ADDR_SPACE (TREE_TYPE (inner_type))) return false; + /* Do not lose casts to function pointer types. */ + if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE + || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) + && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE + || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) + return false; } /* From now on qualifiers on value types do not matter. */ @@ -142,13 +148,6 @@ else if (POINTER_TYPE_P (inner_type) && POINTER_TYPE_P (outer_type)) { - /* Do not lose casts to function pointer types. */ - if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE - || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) - && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE - || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) - return false; - /* We do not care for const qualification of the pointed-to types as const qualification has no semantic value to the middle-end. */ Index: lto/ChangeLog === --- lto/ChangeLog (revision 224200) +++ lto/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2015-06-06 Jan Hubicka + + * lto.c (hash_canonical_type): Do not hash TREE_CODE of TREE_TYPE of + pointers. + 2015-06-05 Aldy Hernandez * lto-lang.c (lto_write_globals): Remove. Index: lto/lto.c === --- lto/lto.c (revision 224200) +++ lto/lto.c (working copy) @@ -337,12 +337,12 @@ if (TREE_CODE (type) == COMPLEX_TYPE) hstate.add_int (TYPE_UNSIGNED (type)); - /* For pointer and reference types, fold in information about the type - pointed to but do not recurse to the pointed-to type. */ + /*
Re: [PATCH] Refactor -Wmisleading-indentation API and extend coverage
The new heuristic turns out to trigger a common false positive (see test case below) in many programs. I have applied this diff on top of the original patch to make the heuristic more strict. diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index d3e842b..5532ff4 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -390,7 +390,10 @@ should_warn_for_misleading_indentation (const common_token_info &guard_tinfo, unsigned int guard_vis_column; if (!get_visual_column (guard_exploc, &guard_vis_column)) return false; - if (next_stmt_vis_column != guard_vis_column + if (next_stmt_vis_column > guard_vis_column + || (guard_tinfo.keyword == RID_IF + && next_stmt_vis_column < guard_vis_column + && !is_first_nonwhitespace_on_line (guard_exploc)) || (next_tok_type == CPP_OPEN_BRACE && next_stmt_vis_column == guard_vis_column)) return true; diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index e8e04e3..fd25102 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -787,6 +787,13 @@ fn_37 (void) foo (3); } + if (i) +while (i++ < 1); + foo (5); + + if (i) while (i++ < 1); + foo (5); + #undef EMPTY #undef FOR_EACH }
Fix more of C/fortran canonical type issues
Hi, this patchs makes fortran's C_SIGNED_CHAR and C_SIZE_T interoperable with signed char and size_t as standard require. There are two issues here. First Fortran integers are always signed, but the standard insist on the signed integer to match C size_t that is unsigned (if it was ptrdiff_t, we would be better of) and similarly the standard seems to explicitly state that C_SIGNED_CHAR is interoperable with both signed char and unsigned char. I thus globbed all integer types of precision compatible either with char or size_t to be the same regardless the signedness. These types are special and apparently magic. I wonder if C FE can be adjusted to make these unsigned instead, but I have no idea if that could possibly work. I also had to update useless_type_conversion_p here and move INTEGER_CST handling ahead so it does not ignore sign. The patch got handled in ugly way by diff, it only moves the whole block on INTEGER_CST up. The second problem is that Fortran FE does not set STRING_FLAG for C_SIGNED_CHAR while C frontend do. I suppose we can teach Fortran to set the flag, but again I do not know what consequences that would have. I thus dropped STRING_FLAG from the canonical type computation. It is already ignored by gimple_canonical_types_compatible_p. My understanding is that STRING_FLAG on integer types is only kind of support to define the array types anyway. The wrong code can be reproduced by uncommenging the char part of bind_c-4 testcase. Finally I added testcases for all types of 15.3.2 of the fortran 2008 draft. I found that C_CHAR is not really compatible with char because it is declared as array of chars. Lets handle that incrementally. Bootstrapped/regtested ppc64le-linux, seems sane? * gimple-expr.c (useless_type_conversion_p): Move INTEGER_TYPE handling ahead. * tree.c (gimple_canonical_types_compatible_p): Do not compare TYPE_UNSIGNED for size_t and char compatible types; do not hash STRING_FLAG on integer types. * lto.c (hash_canonical_type): Do not hash TYPE_UNSIGNED for size_t and char compatible types; do not hash STRING_FLAG on integer types. * gfortran.dg/lto/bind_c-2_0.f90: New testcase. * gfortran.dg/lto/bind_c-2_1.c: New testcase. * gfortran.dg/lto/bind_c-3_0.f90: New testcase. * gfortran.dg/lto/bind_c-3_1.c: New testcase. * gfortran.dg/lto/bind_c-4_0.f90: New testcase. * gfortran.dg/lto/bind_c-4_1.c: New testcase. Index: gimple-expr.c === --- gimple-expr.c (revision 224201) +++ gimple-expr.c (working copy) @@ -91,30 +91,14 @@ || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) return false; } - - /* From now on qualifiers on value types do not matter. */ - inner_type = TYPE_MAIN_VARIANT (inner_type); - outer_type = TYPE_MAIN_VARIANT (outer_type); - - if (inner_type == outer_type) -return true; - - /* If we know the canonical types, compare them. */ - if (TYPE_CANONICAL (inner_type) - && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) -return true; - - /* Changes in machine mode are never useless conversions unless we - deal with aggregate types in which case we defer to later checks. */ - if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) - && !AGGREGATE_TYPE_P (inner_type)) -return false; - /* If both the inner and outer types are integral types, then the conversion is not necessary if they have the same mode and - signedness and precision, and both or neither are boolean. */ - if (INTEGRAL_TYPE_P (inner_type) - && INTEGRAL_TYPE_P (outer_type)) + signedness and precision, and both or neither are boolean. + + Do not rely on TYPE_CANONICAL here because LTO puts same canonical + type for signed char and unsigned char. */ + else if (INTEGRAL_TYPE_P (inner_type) + && INTEGRAL_TYPE_P (outer_type)) { /* Preserve changes in signedness or precision. */ if (TYPE_UNSIGNED (inner_type) != TYPE_UNSIGNED (outer_type) @@ -134,6 +118,25 @@ return true; } + + /* From now on qualifiers on value types do not matter. */ + inner_type = TYPE_MAIN_VARIANT (inner_type); + outer_type = TYPE_MAIN_VARIANT (outer_type); + + if (inner_type == outer_type) +return true; + + /* If we know the canonical types, compare them. */ + if (TYPE_CANONICAL (inner_type) + && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) +return true; + + /* Changes in machine mode are never useless conversions unless we + deal with aggregate types in which case we defer to later checks. */ + if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) + && !AGGREGATE_TYPE_P (inner_type)) +return false; + /* Scalar floating point types with the same mode are compatible. */ else if (SCALAR_FLOAT_TYPE_P (inner_type) && SCALAR_FLOAT
Re: debug-early branch merged into mainline
On 06/07/2015 02:33 PM, Richard Biener wrote: On June 7, 2015 6:00:05 PM GMT+02:00, Aldy Hernandez wrote: On 06/07/2015 11:25 AM, Richard Biener wrote: On June 7, 2015 5:03:30 PM GMT+02:00, Aldy Hernandez wrote: On 06/06/2015 05:49 AM, Andreas Schwab wrote: Bootstrap fails on aarch64: Comparing stages 2 and 3 warning: gcc/cc1objplus-checksum.o differs warning: gcc/cc1obj-checksum.o differs warning: gcc/cc1plus-checksum.o differs warning: gcc/cc1-checksum.o differs Bootstrap comparison failure! gcc/ira-costs.o differs gcc/tree-sra.o differs gcc/tree-parloops.o differs gcc/tree-vect-data-refs.o differs gcc/java/jcf-io.o differs gcc/ipa-inline-analysis.o differs The bootstrap comparison failure on ppc64le, aarch64, and possibly others is due to the order of some sections being in a different order with and without debugging. Stage2 is being compiled with no debugging due to -gtoggle, and stage3 is being compiled with debugging. For ira-costs.o on ppc64le we have: -Disassembly of section .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8: +Disassembly of section .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8: ... -Disassembly of section .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8: +Disassembly of section .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8: There is no semantic difference between the objects, just the ordering. I assume it's the same problem for the rest of the objects and architectures. I will look into this, unless someone beats me to it, or has an idea right off the bat. Check whether the symbol table walkers are walking hash tables. I assume the above are emitted via the symbol removal handling for debug stuff? Ughh, indeed. These sections are being outputted from output_object_blocks which traverses a hash table: void output_object_blocks (void) { object_block_htab->traverse (NULL); } Perhaps we should sort them by some deterministic field and then call output_object_block() on each member of the resulting list? Yes, that would be the usual fix. Maybe sth has an UID already, is the 'object' a decl by chance? The attached patch fixes the bootstrap failure on ppc64le, and theoretically the aarch64 problem as well, but I haven't checked. Tested on ppc64le linux by bootstrapping, and regtesting C/C++ against pre debug-early merge sources. Also tested by a full bootstrap and regtest on x86-64 Linux. OK for mainline? Aldy * varasm.c (output_object_block_htab): Push each object_block into a vector instead of calling output_object_block. (output_object_block_compare): New. (output_object_blocks): Sort object_blocks and then output them. diff --git a/gcc/varasm.c b/gcc/varasm.c index 18f3eac..008360e 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -7420,22 +7420,57 @@ output_object_block (struct object_block *block) } } -/* A htab_traverse callback used to call output_object_block for - each member of object_block_htab. */ +/* An htab_traverse callback used to copy object_blocks into a vector. */ int -output_object_block_htab (object_block **slot, void *) +output_object_block_htab (object_block **slot, void *data) { - output_object_block (*slot); + vec *v = (vec *) data; + v->safe_push (*slot); return 1; } +/* A callback for qsort to compare object_blocks. We only care about + named sections. */ + +static int +output_object_block_compare (const void *x, const void *y) +{ + object_block *p1 = *(object_block * const*)x; + object_block *p2 = *(object_block * const*)y; + + if (p1->sect->common.flags & SECTION_NAMED + && !(p2->sect->common.flags & SECTION_NAMED)) +return 1; + + if (!(p1->sect->common.flags & SECTION_NAMED) + && p2->sect->common.flags & SECTION_NAMED) +return -1; + + if (p1->sect->common.flags & SECTION_NAMED + && p2->sect->common.flags & SECTION_NAMED) +return strcmp (p1->sect->named.name, + p2->sect->named.name); + + return 0; +} + /* Output the definitions of all object_blocks. */ void output_object_blocks (void) { - object_block_htab->traverse (NULL); + vec v = vNULL; + object_block_htab->traverse (&v); + + /* Sort them in order to output them in a deterministic manner, + otherwise we may get .rodata sections in different orders with + and without -g. */ + v.qsort (output_object_block_compare); + unsigned i; + object_block *obj; + FOR_EACH_VEC_ELT (v, i, obj) +output_object_block (obj); } /* This function provides a possible implementation of the
Fix LTO streaming of BUILTINS_LOCATION
Hi, currently we stream BUILTINS_LOCATION by expanding it and streaming resulting filename/line/col tripplet. That is a nonsense and breaks some logic that special case it. This patch fixes it by special casing it same way as we do UNKNOWN_LOCATION (we have precisely 2 special location codes, so doing compound bitpack is not needed) Bootstrapped/regtested ppc64le-linux, OK? Honza * lto-streamer-out.c (lto_output_location): Correctly stream BUILTINS_LOCATION * lto-streamer-in (lto_input_location): Likewise. Index: lto-streamer-out.c === --- lto-streamer-out.c (revision 224201) +++ lto-streamer-out.c (working copy) @@ -205,6 +205,9 @@ bp_pack_value (bp, loc == UNKNOWN_LOCATION, 1); if (loc == UNKNOWN_LOCATION) return; + bp_pack_value (bp, loc == BUILTINS_LOCATION, 1); + if (loc == BUILTINS_LOCATION) +return; xloc = expand_location (loc); Index: lto-streamer-in.c === --- lto-streamer-in.c (revision 224201) +++ lto-streamer-in.c (working copy) @@ -283,6 +283,11 @@ *loc = UNKNOWN_LOCATION; return; } + if (bp_unpack_value (bp, 1)) +{ + *loc = BUILTINS_LOCATION; + return; +} *loc = BUILTINS_LOCATION + 1; file_change = bp_unpack_value (bp, 1);
Fix ICE in warn_types_mismatch
Hi, while preparing the fortran testcases I noticed that warn_types_mismatch generally ICEs on non-C++ types because these have IDENTIFIER_NODE in TYPE_NAME while C++ FE uses TYPE_DECL. warn_types_mismatch was not exactly written with non-C++ types in mind. This patch should fix the ICEs and it also does bit of cleanup of the warnings output to make them more compact. I added location parameters so we can output the warnings attached to the actual symbol we warn about if we do not warn about type itself. The code will need a bit more TLC. In particular we want to merge the logic of warn_type_compatibility_p with type_mismatch_p, but I decided to do this incrementally. For the fortran testcase we now output: ../bind_c-4_1.c:27:18: warning: type of 'myVar' does not match original declaration [-Wlto-type-mismatch] extern myctype_t myVar; ^ ../bind_c-4_0.f90:20:0: note: type 'struct myftype_1' should match type 'struct myctype_t' type(myftype_1), bind(c, name="myVar") :: myVar ^ ../bind_c-4_1.c:14:3: note: the incompatible type is defined here } myctype_t; ^ ../bind_c-4_0.f90:20:0: note: 'myvar' was previously declared here type(myftype_1), bind(c, name="myVar") :: myVar ^ this is still not quite right, because we probably should output "struct myftype_1" in fortran syntax, but I think it is quite understandable. One option is to output only the type names (skipping "struct") and use human readable descriptions of type where needed instead of C syntax. Or we can just declare the C syntax to be OK for LTO messages :) Honza PR lto/65378 * lto-symtab.c (warn_type_compatibility_p): Fix call of odr_or_derived_type_p. (lto_symtab_merge_decls_2): Update call of warn_types_mismatch. * ipa-utils.h (warn_types_mismatch): Update prototype. * ipa-devirt.c (odr_types_equivalent_p): Add loc1/loc2 parameters. (type_mismatch_p): New function. (warn_types_mismatch): Reorg to work better on non-C++ types. (odr_types_equivalent_p): Add loc1/loc2 parameters. (add_type_duplicate): Update. Index: lto/lto-symtab.c === --- lto/lto-symtab.c(revision 224201) +++ lto/lto-symtab.c(working copy) @@ -212,7 +212,7 @@ int lev = 0; /* C++ provide a robust way to check for type compatibility via the ODR rule. */ - if (odr_or_derived_type_p (prevailing_type) && odr_type_p (type) + if (odr_or_derived_type_p (prevailing_type) && odr_or_derived_type_p (type) && !odr_types_equivalent_p (prevailing_type, type)) lev = 2; @@ -542,7 +542,9 @@ "declaration", decl); if (diag) warn_types_mismatch (TREE_TYPE (prevailing->decl), -TREE_TYPE (decl)); +TREE_TYPE (decl), +DECL_SOURCE_LOCATION (prevailing->decl), +DECL_SOURCE_LOCATION (decl)); diagnosed_p |= diag; } else if ((DECL_USER_ALIGN (prevailing->decl) Index: ipa-utils.h === --- ipa-utils.h (revision 224201) +++ ipa-utils.h (working copy) @@ -86,7 +86,8 @@ bool types_odr_comparable (tree, tree, bool strict = false); cgraph_node *try_speculative_devirtualization (tree, HOST_WIDE_INT, ipa_polymorphic_call_context); -void warn_types_mismatch (tree t1, tree t2); +void warn_types_mismatch (tree t1, tree t2, location_t loc1 = UNKNOWN_LOCATION, + location_t loc2 = UNKNOWN_LOCATION); bool odr_or_derived_type_p (const_tree t); bool odr_types_equivalent_p (tree type1, tree type2); Index: ipa-devirt.c === --- ipa-devirt.c(revision 224201) +++ ipa-devirt.c(working copy) @@ -201,7 +201,8 @@ }; static bool odr_types_equivalent_p (tree, tree, bool, bool *, - hash_set *); + hash_set *, + location_t, location_t); static bool odr_violation_reported = false; @@ -777,7 +778,8 @@ static bool odr_subtypes_equivalent_p (tree t1, tree t2, - hash_set *visited) + hash_set *visited, + location_t loc1, location_t loc2) { /* This can happen in incomplete types that should be handled earlier. */ @@ -822,7 +824,7 @@ } if (visited->add (pair)) return true; - return odr_types_equivalent_p (t1, t2, false, NULL, visited); + return odr_types_equivalent_p (t1, t2, false, NULL, visited, loc1, loc2); } /* Compare two virtual tables, PREVAILING and VTABLE and output ODR @@ -1079,7 +1081,7 @@ lto_location_cache::current_cache->apply_location_cache (); if (!warning_at (DECL_SOURCE_LOCATI
Re: Fix more of C/fortran canonical type issues
> Hi, > this patchs makes fortran's C_SIGNED_CHAR and C_SIZE_T interoperable with > signed char and size_t as standard require. There are two issues here. > > First Fortran integers are always signed, but the standard insist on the > signed integer to match C size_t that is unsigned (if it was ptrdiff_t, we > would > be better of) and similarly the standard seems to explicitly state that > C_SIGNED_CHAR is interoperable with both signed char and unsigned char. > I thus globbed all integer types of precision compatible either with char > or size_t to be the same regardless the signedness. Hmm, actually there is a note: NOTE 15.8 ISO/IEC 9899:1999 specifies that the representations for nonnegative signed integers are the same as the corresponding values of unsigned integers. Because Fortran does not provide direct support for unsigned kinds of integers, the ISO C BINDING module does not make accessible named constant s for their kind type parameter values. A user can use the signed kinds of integers to interoperate with the unsigned types and all their qualified versions as well. This has the potentially surprising side ect that the C type unsigned char is interoperable with the type integer with a kind type parameter of C SIGNED CHAR This seems to imply that other integer types also should be interoperable regardless of the signedness. It is true that representation is same for C, but alias sets are not. Perhaps all of the C BINDING types shall just be dropped to alias set 0? That would also solve the inter-operability of char versus char[1]. I would say that the note is non-normative, so perhaps it can just be ignored, too :) Honza
Re: Fix more of C/fortran canonical type issues
Hi, this is a variant of patch that globs also the rest of integer types. Note that we will still get false warnings out of lto-symtab when the values are not wrapped up in structures. This is because lto-symtab uses types_compatible_p that in turn uses useless_type_conversion and that one needs to honor signedness. I suppose we need a way to test representation compatibility and TBAA compatiblity. I will give it a more tought how to reorganize the code. Basically we need - way to decide if two types have compatible memory representation (to test in lto-symtab and for some cases in ipa-icf (contructors/pure moves)) operands_equal_p/compare_constant/ipa-icf::sem_variable all implements bit of this. copmare_constant seems to be most complete. - way to decide if two types match by TBAA oracle (to test in lto-symtab merging and for ipa-icf memory operations) - way to decide if one type is semantically compatible to other (useless_type_conversion_p) - way to decide if two types are same for canonical type computation (gimple_type_compatible_p). This may be sensitive to the set of languages we are merging and enable/disable various globbing as required. It may make sense to refactor the type walkers and get this more organized. But before playing with this I think we want to get something conservatively correct according to language standards and get a reasonable body of testcases. This is a variant of patch that removes TYPE_UNSIGNED testing completely. I am fine with both variants. Bootstrapped/regtested ppc64le-linux. Honza * gimple-expr.c (useless_type_conversion_p): Move INTEGER_TYPE handling ahead. * tree.c (gimple_canonical_types_compatible_p): Do not compare TYPE_UNSIGNED for size_t and char compatible types; do not hash STRING_FLAG on integer types. * lto.c (hash_canonical_type): Do not hash TYPE_UNSIGNED for size_t and char compatible types; do not hash STRING_FLAG on integer types. * gfortran.dg/lto/bind_c-2_0.f90: New testcase. * gfortran.dg/lto/bind_c-2_1.c: New testcase. * gfortran.dg/lto/bind_c-3_0.f90: New testcase. * gfortran.dg/lto/bind_c-3_1.c: New testcase. * gfortran.dg/lto/bind_c-4_0.f90: New testcase. * gfortran.dg/lto/bind_c-4_1.c: New testcase. Index: gimple-expr.c === --- gimple-expr.c (revision 224201) +++ gimple-expr.c (working copy) @@ -91,30 +91,14 @@ || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) return false; } - - /* From now on qualifiers on value types do not matter. */ - inner_type = TYPE_MAIN_VARIANT (inner_type); - outer_type = TYPE_MAIN_VARIANT (outer_type); - - if (inner_type == outer_type) -return true; - - /* If we know the canonical types, compare them. */ - if (TYPE_CANONICAL (inner_type) - && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) -return true; - - /* Changes in machine mode are never useless conversions unless we - deal with aggregate types in which case we defer to later checks. */ - if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) - && !AGGREGATE_TYPE_P (inner_type)) -return false; - /* If both the inner and outer types are integral types, then the conversion is not necessary if they have the same mode and - signedness and precision, and both or neither are boolean. */ - if (INTEGRAL_TYPE_P (inner_type) - && INTEGRAL_TYPE_P (outer_type)) + signedness and precision, and both or neither are boolean. + + Do not rely on TYPE_CANONICAL here because LTO puts same canonical + type for signed char and unsigned char. */ + else if (INTEGRAL_TYPE_P (inner_type) + && INTEGRAL_TYPE_P (outer_type)) { /* Preserve changes in signedness or precision. */ if (TYPE_UNSIGNED (inner_type) != TYPE_UNSIGNED (outer_type) @@ -134,6 +118,25 @@ return true; } + + /* From now on qualifiers on value types do not matter. */ + inner_type = TYPE_MAIN_VARIANT (inner_type); + outer_type = TYPE_MAIN_VARIANT (outer_type); + + if (inner_type == outer_type) +return true; + + /* If we know the canonical types, compare them. */ + if (TYPE_CANONICAL (inner_type) + && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) +return true; + + /* Changes in machine mode are never useless conversions unless we + deal with aggregate types in which case we defer to later checks. */ + if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) + && !AGGREGATE_TYPE_P (inner_type)) +return false; + /* Scalar floating point types with the same mode are compatible. */ else if (SCALAR_FLOAT_TYPE_P (inner_type) && SCALAR_FLOAT_TYPE_P (outer_type)) Index: lto/lto.c === --- lto/lto.c (revision 224201) +++