Re: commits in Bugzilla attributed to others?

2020-03-13 Thread Marek Polacek via Gcc
On Fri, Mar 13, 2020 at 09:46:40AM -0600, Martin Sebor via Gcc wrote:
> It looks as though commits with bug fixes appear in Bugzilla comments
> made by others(*).  Fox instance, commit r10-7151 for PR 92071 shows
> in comment #16 on the bug under Martin Liška's name.
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92071#c16
> 
> Similarly, commits r9-8372, r10-7152, and r8-10121 show up in PR 94119
> in comments with Martin's name on them:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94119
> 
> and likewise here:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91913
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94163
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94154
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94063
> 
> Is this a known problem?
> 
> [*] I haven't seen this for others so maybe Martin's account got messed
> up in the migration somehow?

The authors of those commits are correct, it's just that the commit-to-Bugzilla
sync is broken at the moment and Martin L. was doing it manually.

Marek



Re: commits in Bugzilla attributed to others?

2020-03-13 Thread Marek Polacek via Gcc
On Fri, Mar 13, 2020 at 09:56:58AM -0600, Martin Sebor wrote:
> On 3/13/20 9:50 AM, Marek Polacek wrote:
> > On Fri, Mar 13, 2020 at 09:46:40AM -0600, Martin Sebor via Gcc wrote:
> > > It looks as though commits with bug fixes appear in Bugzilla comments
> > > made by others(*).  Fox instance, commit r10-7151 for PR 92071 shows
> > > in comment #16 on the bug under Martin Liška's name.
> > > 
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92071#c16
> > > 
> > > Similarly, commits r9-8372, r10-7152, and r8-10121 show up in PR 94119
> > > in comments with Martin's name on them:
> > > 
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94119
> > > 
> > > and likewise here:
> > > 
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91913
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94163
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94154
> > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94063
> > > 
> > > Is this a known problem?
> > > 
> > > [*] I haven't seen this for others so maybe Martin's account got messed
> > > up in the migration somehow?
> > 
> > The authors of those commits are correct, it's just that the 
> > commit-to-Bugzilla
> > sync is broken at the moment and Martin L. was doing it manually.
> 
> I see.  Thanks for the explanation (and to Martin for all the work
> to patch up the problem while it's being solved)!

And I should've said "has been doing it manually" because I don't think it's
been fixed yet :).

And yes, thanks Martin!

Marek



Re: New mklog script

2020-05-15 Thread Marek Polacek via Gcc
On Fri, May 15, 2020 at 10:59:56AM +0200, Martin Liška wrote:
> Hi.
> 
> Since we moved to git world and we're in the preparation for ChangeLog 
> messages
> being in git commit messages, I think it's the right time to also simplify 
> mklog
> script.
> 
> I'm sending a new version (which should eventually replace contrib/mklog and 
> contrib/mklog.pl).
> Changes made in the version:
> 
> - the script uses unifdiff - it rapidly simplifies parsing of the '+-!' lines 
> that is done
>   in contrib/mklog

Nice!

> - no author nor date stamp is used - that all can be get from git

This is good.

> - --inline option is not supported - I don't see a use-case for it now

I actually use mklog -i all the time.  But I can work around it if it
disappears.

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA



Re: New mklog script

2020-05-15 Thread Marek Polacek via Gcc
On Fri, May 15, 2020 at 03:12:27PM +0200, Martin Liška wrote:
> On 5/15/20 2:42 PM, Marek Polacek wrote:
> > I actually use mklog -i all the time.  But I can work around it if it
> > disappears.
> 
> Ah, I can see a consumer.
> There's an updated version that supports that.
> 
> For the future, will you still use the option? Wouldn't be better
> to put the ChangeLog content directly to commit message? Note
> that you won't have to copy the entries to a particular ChangeLog file.

The way I do it is to generate a patch using format-patch, use mklog -i
on it, then add the ChangeLog entry to the commit message via commit --amend.

Anything that has to do with ChangeLogs is pointless make-work, so the less
I have to do, the better.  ;-)

Marek



Re: ERR: file not changed in a patch:"gcc/cp/cp-tree.c"

2020-05-19 Thread Marek Polacek via Gcc
On Tue, May 19, 2020 at 01:03:09PM -0600, Martin Sebor via Gcc wrote:
> I'm having trouble with the commit hook that tries to enforce
> ChangeLog contents.  It fails with an error that doesn't make
> sense to me: the file it complains isn't mentioned clearly is
> listed there and I can't tell what about how it's mentioned
> the hook is having a problem with.
> 
> Thanks
> Martin
> 
> $ git push
> Enumerating objects: 23, done.
> Counting objects: 100% (23/23), done.
> Delta compression using up to 16 threads
> Compressing objects: 100% (12/12), done.
> Writing objects: 100% (12/12), 2.41 KiB | 2.41 MiB/s, done.
> Total 12 (delta 11), reused 0 (delta 0)
> remote: *** ChangeLog format failed:
> remote: ERR: file not changed in a patch:"gcc/cp/cp-tree.c"
> remote: ERR: changed file not mentioned in a ChangeLog:"gcc/cp/tree.c"
> remote:
> remote: Please see: https://gcc.gnu.org/codingconventions.html#ChangeLogs
> remote:
> remote: error: hook declined to update refs/heads/master
> To git+ssh://gcc.gnu.org/git/gcc.git
>  ! [remote rejected] master -> master (hook declined)
> error: failed to push some refs to
> 'git+ssh://mse...@gcc.gnu.org/git/gcc.git'
> 
> $ head gcc/cp/ChangeLog
> 2020-05-18  Martin Sebor  
> 
>   PR c++/94923
>   * call.c ((maybe_warn_class_memaccess): Use is_byte_access_type.
>   * cp-tree.h (is_dummy_object): Return bool.
>   (is_byte_access_type): Declare new function.
>   * cp-tree.c (is_dummy_object): Return bool.

There's no cp/cp-tree.c; does it work with just "tree.c" in the ChangeLog?



Re: [IMPORTANT] ChangeLog related changes

2020-06-10 Thread Marek Polacek via Gcc
On Wed, Jun 10, 2020 at 01:34:54PM +, Tamar Christina wrote:
> Hi All,
> 
> We've been wondering since we no longer list authors in the changelog (at 
> least mklog doesn't generate it),
> How do we handle multi author patches nowadays?
> 
> Tried searching for it on the website but couldn’t find anything.

You can add Co-authored-by: name  to your commit.

If we don't already document it, we should.

Marek



Re: gcc __attribute__

2020-08-06 Thread Marek Polacek via Gcc
On Thu, Aug 06, 2020 at 04:15:10PM +0100, Philip R Brenan via Gcc wrote:
> Hi *GCC*:
> 
> On page:
> 
> https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attributes.html
> 
> you show the attribute coming after the parameter list.  But when I try
> this, I  get the following:

That manual is for version 3.2, which is ancient.  See
https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/Attribute-Syntax.html#Attribute-Syntax

Marek



Re: is there a reason why "explicit specialization in non-namespace scope" is still an error in gcc-trunk?

2020-09-23 Thread Marek Polacek via Gcc
On Wed, Sep 23, 2020 at 02:42:01PM +0200, Dennis Luehring wrote:
> i've read that scoped template specalization is allowed in C++17
> 
> 
> clang supports it starting with release 7
> 
> MSVC supports it with VS2017(i don't know what revision)
> 
> Intel does not like it

Because CWG 727 isn't implemented yet:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85282

Marek



Re: What is pex_run

2021-02-25 Thread Marek Polacek via Gcc
On Thu, Feb 25, 2021 at 09:57:55PM +, Gary Oblock via Gcc wrote:
> I've got collect2 finding a linker error and I'm out of
> other options so I'm poking around in the collect2
> sources. I'm wondering what pex_run is (since it's
> getting handed the arguments this might mater?)

See libiberty/pex-common.c.

Marek



Re: Remove RMS from the GCC Steering Committee

2021-03-26 Thread Marek Polacek via Gcc
On Fri, Mar 26, 2021 at 04:02:30PM -0400, Nathan Sidwell wrote:
> [double sigh, attaching a pdf causes it to be blocked, and I guess the
> number of URLs is also triggering a spam trap for the follow up.  I have
> removed many of the URLS from this, you'll have to use your google-fu for
> sources.  I emailed several members of the SC, and don't want to bomb them
> with yet a third copy. ]
> 
> Dear members of the GCC Steering Committee (SC),  I ask you to remove
> Richard Stallman (RMS) from the SC, or, should you chose not to do so, make
> a clear statement as to why he remains.

[...]

> I am writing this publicly, as it is important we address the issue. In
> 2019, when RMS resigned from the FSF, I asked the SC about his status on the
> SC (the web site continued to list his affiliation as FSF).  I never saw as
> response. I failed to follow up. (FWIW, I never received a response to a
> technical licensing issue I asked in 2020. Something seems amiss.)
> In the alternative, I want you to make a definitive statement about why you
> choose not to make such a change.  Do not hide behind silence.  Silence is
> agreeing with the status quo.  Further, if you choose not to make a change,
> do not hide behind a technicality. (My understanding is that RMS has veto
> power.) The rules of the SC are not immutable laws of the universe, nor does
> humanity have immutable laws cast in stone.  The EGCS project showed that we
> can make changes with GCC’s social organization.  If we fail to do so, it
> will continue to be harder and harder to attract new talent to GCC
> development.

I support this and believe we ought to act now.

Marek



Re: help debug hash_map garbage collection issue

2021-04-20 Thread Marek Polacek via Gcc
On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
> I have a static hash_map object that needs to persist across passes:
> 
>   static GTY(()) hash_map *map;
> 
> I initialize the map like so:
> 
>   map = hash_map::create_ggc (4);
> 
> But I see crashes when accessing the map after adding and removing
> some number of entries in a few passes.  The crashes are due to
> the map having been garbage-collected and the memory allocated to
> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
> in GDD being written into the map by poison_pages()).
> 
> I assumed marking the map pointer GTY(()) would keep the garbage
> collector from touching the map.  Is there something else I need
> to do to keep it from doing that?
> 
> Thanks
> Martin
> 
> PS Just in case it matters, the bitmap in the table is allocated
> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
> the map.

My sense is that this is the problem.  What happens if you use
BITMAP_GGC_ALLOC?

Marek



Re: where is PRnnnn required again?

2021-07-06 Thread Marek Polacek via Gcc
On Tue, Jul 06, 2021 at 03:20:26PM -0600, Martin Sebor via Gcc wrote:
> I came away from the recent discussion of ChangeLogs requirements
> with the impression that the PR bit should be in the subject
> (first) line and also above the ChangeLog part but doesn't need
> to be repeated again in the ChangeLog entries.  But my commit
> below was rejected last Friday with the subsequent error.  Adding
> PR middle-end/98871 to the ChangeLog entry let me push the change:
> 
> https://gcc.gnu.org/g:6feb628a706e86eb3f303aff388c74bdb29e7381
> 
> I just had the same error happen now, again with what seems like
> a valid commit message.  Did I misunderstand something or has
> something changed recently?
> 
> Martin
> 
> commit 8a6d08bb49c2b9585c2a2adbb3121f6d9347b780 (HEAD -> master)
> Author: Martin Sebor 
> Date:   Fri Jul 2 16:16:31 2021 -0600
> 
> Improve warning suppression for inlined functions [PR98512].
> 
> Resolves:
> PR middle-end/98871 - Cannot silence -Wmaybe-uninitialized at
> declaration si
> te
> PR middle-end/98512 - #pragma GCC diagnostic ignored ineffective in
> conjunct
> ion with alias attribute

This should be just

PR middle-end/98871
PR middle-end/98512

, no?  Either here, or...

> gcc/ChangeLog:

...here.
 
> * diagnostic.c (get_any_inlining_info): New.
> (update_effective_level_from_pragmas): Handle inlining context.
> (diagnostic_enabled): Same.
> (diagnostic_report_diagnostic): Same.
> * diagnostic.h (struct diagnostic_info): Add ctor.
> (struct diagnostic_context): Add new member.
> * tree-diagnostic.c (set_inlining_locations): New.
> (tree_diagnostics_defaults): Set new callback pointer.
> 
> 
> 
> Enumerating objects: 11, done.
> Counting objects: 100% (11/11), done.
> Delta compression using up to 16 threads
> Compressing objects: 100% (6/6), done.
> Writing objects: 100% (6/6), 3.37 KiB | 3.37 MiB/s, done.
> Total 6 (delta 5), reused 0 (delta 0)
> remote: *** The following commit was rejected by your
> hooks.commit-extra-checker script (status: 1)
> remote: *** commit: 8a6d08bb49c2b9585c2a2adbb3121f6d9347b780
> remote: *** ChangeLog format failed:
> remote: *** ERR: PR 98512 in subject but not in changelog: "Improve warning
> suppression for inlined functions [PR98512]."
> remote: ***
> remote: *** Please see:
> https://gcc.gnu.org/codingconventions.html#ChangeLogs
> remote: ***
> remote: error: hook declined to update refs/heads/master
> To git+ssh://gcc.gnu.org/git/gcc.git
>  ! [remote rejected] master -> master (hook declined)
> error: failed to push some refs to
> 'git+ssh://mse...@gcc.gnu.org/git/gcc.git'
> 

Marek



Re: where is PRnnnn required again?

2021-07-07 Thread Marek Polacek via Gcc
On Wed, Jul 07, 2021 at 03:35:35PM -0600, Martin Sebor wrote:
> On 7/7/21 2:42 PM, Jonathan Wakely wrote:
> > 
> > 
> > On Wed, 7 Jul 2021, 17:39 Martin Sebor,  > > wrote:
> > 
> > On 7/6/21 4:09 PM, Jonathan Wakely wrote:
> >  >
> >  >
> >  > On Tue, 6 Jul 2021, 22:45 Martin Sebor via Gcc,  > 
> >  > >> wrote:
> >  >
> >  >     On 7/6/21 3:36 PM, Marek Polacek wrote:
> >  >      > On Tue, Jul 06, 2021 at 03:20:26PM -0600, Martin Sebor via
> > Gcc wrote:
> >  >      >> I came away from the recent discussion of ChangeLogs
> > requirements
> >  >      >> with the impression that the PR bit should be in the
> > subject
> >  >      >> (first) line and also above the ChangeLog part but
> > doesn't need
> >  >      >> to be repeated again in the ChangeLog entries.  But my commit
> >  >      >> below was rejected last Friday with the subsequent
> > error.  Adding
> >  >      >> PR middle-end/98871 to the ChangeLog entry let me push
> > the change:
> >  >      >>
> >  >      >>
> > https://gcc.gnu.org/g:6feb628a706e86eb3f303aff388c74bdb29e7381
> >  >      >>
> >  >      >> I just had the same error happen now, again with what
> > seems like
> >  >      >> a valid commit message.  Did I misunderstand something or has
> >  >      >> something changed recently?
> >  >      >>
> >  >      >> Martin
> >  >      >>
> >  >      >> commit 8a6d08bb49c2b9585c2a2adbb3121f6d9347b780 (HEAD ->
> > master)
> >  >      >> Author: Martin Sebor  >   > >>
> >  >      >> Date:   Fri Jul 2 16:16:31 2021 -0600
> >  >      >>
> >  >      >>      Improve warning suppression for inlined functions
> > [PR98512].
> >  >      >>
> >  >      >>      Resolves:
> >  >      >>      PR middle-end/98871 - Cannot silence
> > -Wmaybe-uninitialized at
> >  >      >> declaration si
> >  >      >> te
> >  >      >>      PR middle-end/98512 - #pragma GCC diagnostic ignored
> >  >     ineffective in
> >  >      >> conjunct
> >  >      >> ion with alias attribute
> >  >      >
> >  >      > This should be just
> >  >      >
> >  >      >       PR middle-end/98871
> >  >      >       PR middle-end/98512
> >  >      >
> >  >      > , no?
> >  >
> >  >     Does it matter if there's text after the PR ...?
> >  >
> >  >
> >  >
> >  > Yes. With extra text the whole line is just treated as arbitrary
> > text,
> >  > not a "PR component/" string. So with the extra text it won't be
> >  > added to the ChangeLog file, and won't match the PR in the
> > subject line.
> >  >
> >  >        I managed to push
> >  >
> >  > https://gcc.gnu.org/pipermail/gcc-cvs/2021-July/350316.html
> >  >
> >  >     that uses the same style earlier today
> >  >
> >  >
> >  > But will it add the PR numbers to the ChangeLog? I think the
> > answer is
> >  > no (in which case you could edit the ChangeLog tomorrow if you
> > want them
> >  > to be in there).
> > 
> > It updated Bugzilla but it didn't add the PR numbers to the ChangeLog
> > entries.  I still don't (obviously) understand the rules the hook uses
> > for what to update or the rationale for them.  It seems as though
> > the PR in the subject is used to update only Bugzilla but not also
> > update the ChangeLogs (why not?)
> > 
> > 
> > Because they are two completely separate processes. Verifying the commit
> > message format is done by a git hook, and you can run exactly the same
> > checks locally before pushing a commit.
> > 
> > Updating bugzilla is done by a separate and different process, which has
> > been in place for years (decades?) before we switched to git.
> 
> I don't mean to turn this into a contentious back and forth but
> "because this is how it works" or "because this is how it's been
> done for eons" aren't a rationale, at least not a satisfying one.
> 
> Do you not agree that it would be better to be able to mention
> the PR or PRs just once and have all our scripts work with it?
> If you do then is something keeping us from making those changes?
> 
> Martin
> 
> PS To be clear, I'm suggesting that all these work the same and
> update Bugzilla as well as ChangeLogs, both with and without
> a space after PR and both with and without a component name after
> the PR.
> 
> 1) PR only in title.
>   Fix foobar [PR12345]
> 
>   gcc/ChangeLog:
> * foo.c (bar): Fix it.

The script would have to derive the component name from the PR number. 
That might a complication.

> 2) PR (with or without additional text after it) after title and
>before ChageLogs.
>   Fix foobar.
> 
>   PR12345 - foobar b

Re: How to run C++ IPA tests?

2021-10-27 Thread Marek Polacek via Gcc
On Wed, Oct 27, 2021 at 04:29:32PM +0200, Erick Ochoa via Gcc wrote:
> Hi,
> 
> I have been adding tests to the gcc/testsuite/gcc.dg/ipa folder
> successfully for a while now. I am starting to add some tests into
> gcc/testsuite/g++.dg/ipa now but I am having some issues.
> 
> 1. Using `make check-g++` returns the following error message "No rule
> to make target 'check-g++'".
> 2. While `make check-gcc` works, this seems to be only for C tests
> (which is correct).
> 3. When I type `make check RUNTESTS="ipa.exp"`  but the section "g++
> Summary" looks empty.
> 4. I have added a test that I know will fail, but I don't see it
> (because I don't think I have run g++ tests correctly).
> 
> To make sure that I haven't changed anything with my transformation I
> actually checked out releases/gcc-11.2.0
> 
> Can someone tell me how to run the C++ IPA tests? I'm sure there is
> something silly that I'm doing wrong, but can't find out what it is.

There's no ipa.exp in gcc/testsuite/g++.dg/ipa.  Instead you probably
want

make check-c++ RUNTESTFLAGS=dg.exp=ipa/*

Marek



Re: What replaces FOR_EACH_LOOP_FN

2022-03-02 Thread Marek Polacek via Gcc
On Wed, Mar 02, 2022 at 10:04:40PM +, Gary Oblock via Gcc wrote:
> Guys,
> 
> I've been working on an optimization for quite a bit of time and
> in an attempt to move it to GCC 12 I found that FOR_EACH_LOOP_FN
> no longer exists. I poked around in the archives and tried a Google
> search but found nothing on it.
> 
> It suited my needs and I'd hate to have to rewrite a bunch of stuff.
> What replaces it and how do I use?

Look at this commit:

commit e41ba804ba5f5ca433e09238d561b1b4c8b10985
Author: Kewen Lin 
Date:   Thu Jul 29 22:26:25 2021 -0500

Use range-based for loops for traversing loops

This patch follows Martin's suggestion here[1], to support
range based loop for iterating loops, analogously to the
patch for vec[2].

For example, use below range-based for loop

for (auto loop : loops_list (cfun, 0))

to replace the previous macro FOR_EACH_LOOP

FOR_EACH_LOOP (loop, 0)

which shows a number of examples like

-  loop_p loop;
-  FOR_EACH_LOOP_FN (fn, loop, LI_INCLUDE_ROOT)
+  for (auto loop : loops_list (fn, LI_INCLUDE_ROOT))

Marek



Re: passing command-line arguments, still

2022-03-16 Thread Marek Polacek via Gcc
On Wed, Mar 16, 2022 at 02:34:09PM -0400, James K. Lowden wrote:
> [I sent this to gcc-help by mistake. I'm reposting it here in case
> anyone has a suggestion. I did take dje's advice, and deleted the build
> directory, except that I preserved config.status and regenerated
> Makefile.  The observed behavior remains unchanged.  TIA.]
> 
> https://git.symas.net:443/cobolworx/gcc-cobol/
> 
> My first question regards command-line options.  I've had no trouble
> defining switches (-f-foo), but no luck defining an option that takes
> an argument.  The latter are accepted by gcobol and not passed to
> cobol1.  

Let's avoid -f-foo; use -ffoo instead, like the rest of GCC.
 
> In cobol/lang.opt, I have:
> 
> indicator-column

Make this 'findicator-column='.  Does that help?

> Cobol Joined Separate UInteger Var(indicator_column) Init(0)
> IntegerRange(0, 8) -indicator-column=  Column after which
> Region B begins
> 
> strace(1) shows the problem:
> 
> [pid 683008] execve("../../../build/gcc/gcobol",
> ["../../../build/gcc/gcobol", "-main", "-o", "obj/SG105A", "-B",
> "../../../build/gcc/", "-f-flex-debug", "-f-yacc-debug",
> "-indicator-column", "1", "cbl/SG105A.cbl", "-lgcobol", "-lm", "-ldl"],
> 0x55a19b487940 /* 36 vars */ 
> 
> gcobol is being invoked with 3 options used by cobol1:
>   "-f-flex-debug", "-f-yacc-debug", "-indicator-column", "1"
> 
> where -indicator-column takes an argument, "1".  But it's not  passed to
> cobol1: 
> 
> [pid 683008] <... execve resumed>)  = 0
> [pid 683009] execve("../../../build/gcc/cobol1",
> ["../../../build/gcc/cobol1", "cbl/SG105A.cbl", "-quiet", "-dumpbase",
> "SG105A.cbl", "-main", "-mtune=generic", "-march=x86-64", "-auxbase",
> "SG105A", "-f-flex-debug", "-f-yacc-debug", "-o", "/tmp/ccIBQZv1.s"],
> 0x1578290 /* 40 vars */ 
> 
> The stanza in cobol/lang.opt looks similar to others in
> fortran/lang.opt. The gcc internals don't mention anything else that I
> could find that needs to be done.  I've done a complete rebuild after
> "make distclean".  And still no joy.

doc/options.texi describes options relative well, I think.
 
> We are working with a gcc fork of 10.2.  Our log message says (in part):
> 
> The "tiny" branch was started with the 10.2.1
> origin/releases/gcc-10 branch> c806314b32987096d79de21e72dc0cf783e51d57)
> 
> What am I missing, please?
> 
> --jkl
> 

Marek



Re: passing command-line arguments, still

2022-03-17 Thread Marek Polacek via Gcc
On Thu, Mar 17, 2022 at 12:21:36PM -0400, James K. Lowden wrote:
> On Wed, 16 Mar 2022 14:45:33 -0400
> Marek Polacek  wrote:
> 
> Hi Marek, 
> 
> > Let's avoid -f-foo; use -ffoo instead, like the rest of GCC.
> 
> Sure. I hadn't noticed the distinction. 
> 
> > > In cobol/lang.opt, I have:
> > > 
> > > indicator-column
> > 
> > Make this 'findicator-column='.  Does that help?
> 
> Yes, with that change, the option & argument are passed.  But ... why?

I think Jon and Joseph answered your questions now.
 
> It's my understanding that the -f prefix indicates a switch, meaning:  
> 
> 1.  It does not take an argument

It may, as Jon showed.

> 2.  GCC accepts a  -fno- alternative, automatically

Right, unless it's RejectNegative.

> 3.  The "f" stands for "flag", meaning on/off.  

It does stand for "flag", and it looks like at some point in ancient
history if was on/off, but then came options like -falign-loops=N.
 
> My option has no alternative, if you'll pardon the pun.  I don't see
> the point in confusing the user by suggesting it does.  
> 
> The fine manual says:
> 
>  By default, all options beginning with "f", "W" or "m" are
>  implicitly assumed to take a "no-" form.  This form should not be
>  listed separately.  If an option beginning with one of these
>  letters does not have a "no-" form, you can use the
>  'RejectNegative' property to reject it.
> 
> That isn't quite accurate.  The "no-" form isn't "implicitly assumed".
> What would "explicitly assumed" look like, btw?  

I guess that you'd have two separate entries in whatever.opt:

ffoo
...

fno-foo
...

> More accurate would be
> to say a "fno-" form is automatically accepted or generated. Computer
> code does not make assumptions; programmers do.  

TBH, I don't see how that would be any more accurate that what we have now.


Anyway, did you figure it out or are there some lingering questions?

Marek



Re: Buggy error message when dereferencing once a double pointer to struct

2022-05-23 Thread Marek Polacek via Gcc
On Mon, May 23, 2022 at 06:43:55PM +0200, Andrea Monaco via Gcc wrote:
> 
> This snippet that I wrote
> 
> 
>   struct
>   str
>   {
> int val;
>   };  
> 
> 
>   void
>   main (int argc, char **argv)
>   {
> struct str **p;
> int i;
>   
> i = p->val;
>   }
> 
> 
> is obviously incorrect.  But gcc 8.3.0 says
> 
>   pointer.c: In function ‘main’:
>   pointer.c:14:8: error: ‘*p’ is a pointer; did you mean to use ‘->’?
>  i = p->val;
>   ^~
>   ->
> 
> which seems a buggy error message to me: I wrote "p", not "*p"; also the
> compiler suggests replacing "->" with itself.

Yes, this is https://gcc.gnu.org/PR91134

Marek



Re: Usage of the C++ stdlib unordered_map in GCC

2022-08-30 Thread Marek Polacek via Gcc
On Tue, Aug 30, 2022 at 09:57:45PM +0200, Tim Lange wrote:
> Hello,
> 
> I was preparing a patch for GCC and used the unordered_map from the C++
> stdlib in my patch. Later on, I noticed that it is used nowhere else inside
> GCC except for some files in the go frontend.
> 
> I wondered, now that building GCC requires a C++11 host compiler, whether
> there is a consensus on which data structure implementation is preferred.
> Should I rather use a hash_map instead of an unordered_map or is it on my
> side to decide which one I choose?

I think you're probably better off using a hash_map; std::unordered_map
has efficiency issues as described in
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2028r0.pdf

Marek



Re: C2x features status

2022-10-21 Thread Marek Polacek via Gcc
On Fri, Oct 21, 2022 at 08:31:09PM +0200, Florian Weimer via Gcc wrote:
> * Joseph Myers:
> 
> > I'm working on adding various C2x features to the C front end (and 
> > elsewhere in GCC as applicable).
> >
> > I suspect I won't get all the C2x features done for GCC 13.  If anyone 
> > else is interested in adding C2x features, I'd encourage looking at some 
> > of the following, which I may well not get to for GCC 13 (and posting here 
> > to avoid duplication of effort if working on such a feature):
> >
> > * Bit-precise integer types (_BitInt) (see bug 102989) (integrated version 
> > based on N2763, plus literal suffixes from N2775 and bit-fields from 
> > N2969).  Would require working with back-end maintainers and upstream ABI 
> > groups, where available, to get ABIs defined for as many architectures as 
> > possible, as well as some default ABI choice in GCC for architectures that 
> > haven't defined the ABI for these types.
> >
> > * [[unsequenced]] and [[reproducible]] attributes for function types.  See 
> > N2956.  These are supposed to be similar to const and pure attributes, at 
> > least in the absence of pointer and array function parameters (but note 
> > they never affect type compatibility).
> >
> > * Tag compatibility (N3037, alternative wording).  Martin Uecker might 
> > have patches for a draft version of this?
> >
> > * Preprocessor #embed (N3017) (see bug 105863).
> 
> Do you have a list of C2X features that are likely to impact autoconf
> tests?  Or planned changes in the GCC 13 and 14 default language modes
> that reject constructs previous accepted as an extension?

At least this one:

commit 0a91bdaf177409a2a5e7895bce4f0e7091b4b3ca
Author: Joseph Myers 
Date:   Wed Sep 7 13:56:25 2022 +

c: New C2x keywords

which says:

As with the removal of unprototyped functions, this change has a high
risk of breaking some old code and people doing GNU/Linux distribution
builds may wish to see how much is broken in a build with a -std=gnu2x
default.
 
> I'm asking because I'm working on the implicit function declaration
> problem once more, and other things could be piggybacked on the tool
> support over time.  See the parallel “C89isms in the test suite” thread.
> 
> I wonder if anything went into the default C2X language mode already
> that could be similarly disruptive as the removal of implicit ints?  In
> that case, I should probably backport that change into my GCC test
> version.  (To avoid chasing ghosts, it's based off GCC 12, I've decided
> to decouple it from our planned switch to GCC 13.)
> 
> Thanks,
> Florian
> 

Marek



Re: -Wint-conversion, -Wincompatible-pointer-types, -Wpointer-sign: Are they hiding constraint C violations?

2022-11-10 Thread Marek Polacek via Gcc
On Thu, Nov 10, 2022 at 07:25:21PM +0100, Florian Weimer via Gcc wrote:
> GCC accepts various conversions between pointers and ints and different
> types of pointers by default, issuing a warning.
> 
> I've been reading the (hopefully) relevant partso f the C99 standard,
> and it seems to me that C implementations are actually required to
> diagnose errors in these cases because they are constraint violations:
> the types are not compatible.

It doesn't need to be a hard error, a warning is a diagnostic message, which
is enough to diagnose a violation of any syntax rule or constraint.

IIRC, the only case where the compiler _must_ emit a hard error is for #error.
 
> Is this interpretation correct?
> 
> Sorry if this questions this is more appropriate for the gcc-help list.
> 
> Thanks,
> Florian
> 

Marek



Re: Feature request: Warning when .c file gets #include'd

2022-12-02 Thread Marek Polacek via Gcc
On Fri, Dec 02, 2022 at 03:57:44PM +0100, Jiří Wolker via Gcc wrote:
> 
> Hi,
> 
> I've met a guy that is learning C and got stuck when the linker produced
> a screenful of messages about that he did something define multiple
> times. The cause of the problem was trivial:
> 
> He did ``#include "something.c"'' in his code.
> 
> In the past, I also did this thing multiple times and I would appreciate
> a warning produced by the gcc when compiling C/C++ file that includes a
> file ending in .c, .cx, .cpp or .cxx.
> 
> What do you think about that?
> 
> I would prefer to make this enabled when -Wextra is used.

The problem is that a lot of beginners don't use -Wextra, so the
warning wouldn't be displayed anyway.
 
> Based on my past experience with using .c files in the include
> directive, I can think of these problems.
> 
>   - Sometimes, I want to include a file that is definitely not a header,
> but also it is not a stand-alone source file. I personally prefer
> using .inc suffix, but some people possibly terminate the file names
> with .c suffix like a source file.
> 
> Example:  list of enum fields included from a separate file that was
>   generated by a script
> 
>   - In some projects, someone can deliberately want to include another
> source file. For example, this can happen when doing unit tests and
> you do not want to specify the source file name on the command line.

Indeed, e.g. elfutils uses this a lot:
#define LIBELFBITS 64
#include "elf32_xlatetom.c"

We use including .c in our testsuite a lot as well.

So I'm afraid this would get -1 from me personally, sorry.

> If you accept that, I can try to implement that. (That would be my first
> contribution to this project and I do not know gcc codebase, but it
> looks like a relatively simple change.) Do you think that it would need
> a copyright assignment?

I think it would.

Thanks,
Marek



Re: No warning about duplicate values in enum

2023-03-10 Thread Marek Polacek via Gcc
On Fri, Mar 10, 2023 at 01:57:06PM +0100, Andrea Monaco via Gcc wrote:
> 
> In gcc 8.3.0, compiling
> 
> 
> enum
> test
>   {
> FIRST = 1,
> SECOND = 1,
> THIRD = 2
>   };
> 
> int
> main (void)
> {
>   return 0;
> }
> 
> 
> generates no warning even with -Wextra.  That hit me today, because I
> had a large enum with many explicitly assigned constants and I
> accidentally used the same value twice, which is an obvious source of
> problems.

This is https://gcc.gnu.org/PR16186.

Marek



Re: No warning about duplicate values in enum

2023-03-13 Thread Marek Polacek via Gcc
On Sat, Mar 11, 2023 at 04:48:14PM +, Jonathan Wakely via Gcc wrote:
> On Sat, 11 Mar 2023, 12:53 Basile Starynkevitch, 
> wrote:
> 
> > Hello all,
> >
> >
> > Andrea observed that:
> >
> > In gcc 8.3.0, compiling
> >
> >
> > enum
> > test
> >{
> >  FIRST = 1,
> >  SECOND = 1,
> >  THIRD = 2
> >};
> >
> > int
> > main (void)
> > {
> >return 0;
> > }
> >
> >
> > generates no warning even with -Wextra.
> >
> > I believe that the C standard (which I don't have here, but see also
> > https://port70.net/~nsz/c/c11/n1570.html or buy it from ISO) explicitly
> > allow duplicate values in enum.
> >
> 
> 
> Of course it does, it's perfectly valid. Nobody has said it should be
> rejected. The request is for a warning, because for *some* uses of enums
> duplicates are not wanted.

And as I said in the other thread about the very same issue, it's
 which is assigned to me and I hope to
implement it for GC 14.

Marek



Re: lambda coding style

2024-01-10 Thread Marek Polacek via Gcc
On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> What formatting style do we want for non-trivial lambdas in GCC sources?
> I'm thinking the most consistent choice would be
> 
> auto l = [&] (parms) // space between ] (
>   {  // brace on new line, indented two spaces
> return stuff;
>   };

Sure, why not.  Consistency is what matters.  Thus far we seem
to have been very inconsistent.  ;)
 
> By default, recent emacs lines up the { with the previous line, like an
> in-class function definition; I talked it into the above indentation with
> 
> (defun lambda-offset (elem)
>   (if (assq 'inline-open c-syntactic-context) '+ 0))
> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> 
> I think we probably want the same formatting for lambdas in function
> argument lists, e.g.
> 
> algorithm ([] (parms)
>   {
> return foo;
>   });

And what about lambdas in conditions:

if (foo ()
&& [&] (params) mutable
   {
 return 42;
   } ())

should the { go just below [?

Marek



Re: lambda coding style

2024-01-10 Thread Marek Polacek via Gcc
On Wed, Jan 10, 2024 at 04:24:42PM -0500, Jason Merrill wrote:
> On 1/10/24 15:59, Marek Polacek wrote:
> > On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> > > What formatting style do we want for non-trivial lambdas in GCC sources?
> > > I'm thinking the most consistent choice would be
> > > 
> > > auto l = [&] (parms) // space between ] (
> > >{  // brace on new line, indented two spaces
> > >  return stuff;
> > >};
> > 
> > Sure, why not.  Consistency is what matters.  Thus far we seem
> > to have been very inconsistent.  ;)
> > > By default, recent emacs lines up the { with the previous line, like an
> > > in-class function definition; I talked it into the above indentation with
> > > 
> > > (defun lambda-offset (elem)
> > >(if (assq 'inline-open c-syntactic-context) '+ 0))
> > > (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> > > 
> > > I think we probably want the same formatting for lambdas in function
> > > argument lists, e.g.
> > > 
> > > algorithm ([] (parms)
> > >{
> > >  return foo;
> > >});
> > 
> > And what about lambdas in conditions:
> > 
> > if (foo ()
> >  && [&] (params) mutable
> > {
> >  return 42;
> > } ())
> > 
> > should the { go just below [?
> 
> I think we don't want the { to go below the [ in general; that was the old
> emacs default behavior, and it produced lambda bodies with excessive
> indentation.
> 
> With my adjustment above, emacs indents the { two spaces from the &&, which
> seems a bit arbitrary but reasonable.

Fair enough, I think that's better.

I suppose we should add a note wrt lambdas to
https://gcc.gnu.org/codingconventions.html#Cxx_Conventions

Marek



Re: [attribs.cc] ICE with 4x underscores

2024-01-30 Thread Marek Polacek via Gcc
On Tue, Jan 30, 2024 at 12:25:24PM +, Jonathan Wakely via Gcc wrote:
> On Tue, 30 Jan 2024, 10:35 Amol Surati via Gcc,  wrote:
> 
> > Hello,
> >
> > If a std attribute name is squeezed between 4x underscores,
> >
> 
> Which is undefined behaviour, but shouldn't crash.
> 
> the compiler (both 13.2 [1] and trunk [2]) experiences an ICE.
> >
> 
> Bug reports belong in bugzilla, not on the mailing list please.
> 
> https://gcc.gnu.org/bugs/

I've created :
[[attr]] causes internal compiler error: in decl_attributes, at 
attribs.cc:776

which crashes with both cc1{,plus}.  The problem happens when the attribute
is both in std_attributes and c_common_gnu_attributes, it seems.

Marek