> On Oct. 7, 2016, 10:31 a.m., Andreas Sandberg wrote:
> > Thanks for doing this! This is something that has annoyed me for quite some 
> > time as well.
> > 
> > Thee are quite a few files modified in patch where the header order isn't 
> > compliant with the style guide, which is probably why you are seeing style 
> > checker errors. Ideally, I'd like to see the primary header first rule 
> > enforced in all CC files since that ensures that header files include all 
> > of their dependencies, but I wouldn't say that's a blocker.
> 
> Andreas Sandberg wrote:
>     I have just added the necessary fixes to the style checker to automate 
> this (see [RB3648](http://reviews.gem5.org/r/3648/)). We (I'm happy to do it) 
> should be able to fix the include order automagically using this:
>     ````
>     ./util/style.py -f -c SortedIncludes `find src/ -name *.cc`
>     ````
> 
> Brandon Potter wrote:
>     Your patch appears to fix the qref issue so thanks for posting it and for 
> the quick response!
>     
>     I've created a patch using the command that you've provided; it's sitting 
> in my patch queue before my patches. There weren't too many merge conflicts 
> with my patches which is nice. However, I've found that the new style 
> checker's header reorganization exposes problems with missing includes in the 
> header files when I subsequently try to compile. __Do you mind if I go 
> through the process of resolving the fallout or do you want to do it?__ I 
> don't mind doing it myself since it's currently blocking the header file 
> changes that I'd like to post, but I don't want to steal the limelight given 
> that you wrote the original style fix.
>     
>     Please let me know what you want to do.

I'm not surprised that the new header order exposes issues with existing files. 
After all, that style rule is designed to suss out headers that don't include 
all their dependencies. I don't mind at all if you sort out the header issues, 
please go ahead and do it. ... and thanks for offering to fix this!


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3643/#review8779
-----------------------------------------------------------


On Oct. 5, 2016, 5:27 p.m., Brandon Potter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2016, 5:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11660:1c76ab8bafec
> ---------------------------
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -----
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table_impl.hh 
> b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/arguments.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/generic/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/power/interrupts.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pseudo_inst.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/system.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
> 
> Diff: http://reviews.gem5.org/r/3643/diff/
> 
> 
> Testing
> -------
> 
> util/regress
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to