Hi Steve, Concerning the different proposals:
1. I'm fine with this, but would rather go for one of the other solutions to not create too much clutter. 2. clang does support the -w flag 3. See 2. 4. It is a sensible idea, but it also opens up for questions concerning our own auto-generated code. For example, I tried adding a warning for self-assignment, and it turns out we do this a lot in the generated decoders (in fact the behaviour seems to rely on it and breaks if they are removed). If we go through with this "tiering" approach then I think we should have user generated, machine generated (that we control) and other machine generated that we believe to be correct. 5. I would even say why not simply add the no-uninitialized and be done with it? Andreas On 20/03/2013 23:16, "Steve Reinhardt" <[email protected]> wrote: >Yea, Ubuntu 10.04 LTS is what I have too. > >So unsurprisingly this ended up being more complicated than I expected. > Getting rid of swig_env entirely and just building swig files with >'Werror=False' works, but spews out a zillion warnings, because a lot of >what we do with swig_env is add '-Wno-foo' for all the '-Wfoo' warnings we >normally want. > >Unfortunately all the warnings are baked in to env up front, so even >though >the logical thing would be not to add them in the first place, that's a >non-trivial change. > >I did have success with adding '-w' in swig_env, which is a gcc option >that >suppresses all warnings. I don't know whether clang supports that or not >though. > >So I see several paths here: > >1. Just stop adding -Werror to swig_env (like Mitch has done). Doesn't >really simplify anything, but at least the few warnings we're not managing >to avoid now don't kill your build. > >2. Stop trying to undo specific warnings that we know the generated code >will violate, and use '-w' instead just to suppress all warnings. This is >only a reasonable solution if clang (and other compilers we might want to >use) also support -w. > >3. Find a way to go through the compiler flags for swig_env and delete all >the ones that match '-W*'. This is like #2, but has the advantage of not >relying on '-w'. I'm also not 100% sure how to do this in scons, but I >assume it is possible. > >4. Instead of accumulating compiler warning flags directly into the main >build environment, accumulate them off to the side somewhere, then have >the >default environment be the main environment plus all the warnings. This >allows us to create swig_env by cloning the main environment *before* the >warning flags are added. > > >Given these options, #1 is the easiest but still leaves some ugly code in >place; #2 is nice because we can get rid of all the '-Wno-foo' stuff, but >concerns me because I'm not sure how universal '-w' is; #4 is basically >the >way I wish we'd done it in the first place, but is by far the most effort >at this point; and #3's main attraction is that it is sort of like #4 but >is probably less work. > >Any thoughts? Any alternatives I missed? > >Steve > > > >On Wed, Mar 20, 2013 at 1:11 PM, Mitch Hayenga ><[email protected] >> wrote: > >> I have same error/warning on the ECE cluster here at Wisconsin (Ubuntu >> 10.04 LTS which happens to be gcc 4.4 and swig 1.3.40). I just disabled >> Werror for swig. >> >> On Wed, Mar 20, 2013 at 1:45 PM, nathan binkert <[email protected]> >>wrote: >> >> > It's probably all my fault, and my guess is that the swig_env predated >> > the Werror flag. I'm fine with your proposal. >> > >> > Nate >> > >> > On Wed, Mar 20, 2013 at 11:33 AM, Steve Reinhardt <[email protected]> >> > wrote: >> > > Why do we care about compiler warnings in swig-generated code? >> > > >> > > I just started running some tests on my Ubuntu 10.04 machine (swig >> > 1.3.40, >> > > gcc 4.4.3), and I ran into this on a few different _wrap.cc files: >> > > >> > > [ CXX] ALPHA/python/swig/event_wrap.cc -> .fo >> > > cc1plus: warnings being treated as errors >> > > build/ALPHA/python/swig/event_wrap.cc: In function 'PyObject* >> > > _wrap_new_Cycles(PyObject*, PyObject*)': >> > > build/ALPHA/python/swig/event_wrap.cc:3380: error: 'argv[0]' may be >> used >> > > uninitialized in this function >> > > build/ALPHA/python/swig/event_wrap.cc: In function 'PyObject* >> > > _wrap_simulate(PyObject*, PyObject*)': >> > > build/ALPHA/python/swig/event_wrap.cc:4590: error: 'argv[0]' may be >> used >> > > uninitialized in this function >> > > scons: *** [build/ALPHA/python/swig/event_wrap.fo] Error 1 >> > > scons: building terminated because of errors. >> > > >> > > I went poking around in src/SConscript and I see that we have a >>mildly >> > > elaborate setup to (1) use a different build environment for swig >> files, >> > > swig_env, so we can have finer-grain control over warnings, and (2) >> > figure >> > > out based on the compiler type and version which warnings to >>suppress. >> I >> > > saw that the -Wno-maybe-uninitialized flag was added to swig_env >>only >> for >> > > gcc versions >= 4.7, so I changed that to make it unconditional, >>only >> to >> > > find that that flag isn't even valid for gcc 4.4; I had to use >> > > -Wno-uninitialized instead. >> > > >> > > So I could solve my immediate problem by adding -Wno-uninitialized >>for >> > gcc >> > > 4.4, though I'm not sure if this is really a gcc 4.4 issue or a swig >> > 1.3.40 >> > > issue (or maybe it's specific to that particular combination). But >>in >> > the >> > > bigger picture I wonder: why do we really care? We're not going to >>go >> > fix >> > > the swig output to get rid of warnings, and as long as the code >> compiles >> > > and runs, we probably don't care about them. Wouldn't it be >>simpler to >> > get >> > > rid of the whole swig_env thing and all the code that sets it up, >>and >> > just >> > > add 'Werror=False' when we compile swig sources? >> > > >> > > Steve >> > > _______________________________________________ >> > > gem5-dev mailing list >> > > [email protected] >> > > http://m5sim.org/mailman/listinfo/gem5-dev >> > _______________________________________________ >> > gem5-dev mailing list >> > [email protected] >> > http://m5sim.org/mailman/listinfo/gem5-dev >> > >> _______________________________________________ >> gem5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/gem5-dev >> >_______________________________________________ >gem5-dev mailing list >[email protected] >http://m5sim.org/mailman/listinfo/gem5-dev > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
