[llvm-commits] [llvm] r52800 - in /llvm/trunk: include/llvm/CodeGen/MachineDebugInfoDesc.h include/llvm/CodeGen/MachineModuleInfo.h lib/CodeGen/DwarfWriter.cpp lib/CodeGen/MachineDebugInfoDesc.cpp lib/CodeGen/MachineModuleInfo.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/VMCore/IntrinsicInst.cpp

[Chris Lattner]

On Jun 26, 2008, at 5:09 PM, Bill Wendling wrote:
> Log:
> Refactor the DebugInfoDesc stuff out of the MachineModuleInfo file.  
> Clean up
> some uses of std::vector, where it's return std::vector by value.  
> Yuck!

Nice, much better.  One request: when making a big reorganization  
change like this, it is vastly preferred to move the code without  
changing it at all.  Because you committed the move with the vector  
change, it is impossible to review the vector change.

Can you see if there are files #including MachineModuleInfo.h that can  
change to #include MachineDebugInfo.h instead?

> @@ -0,0 +1,706 @@
> +//===-- llvm/CodeGen/MachineDebugInfoDesc.h ---------------------*-  
> C++ -*-===//

How about just MachineDebugInfo.h?

> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +//
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//

What does this do?

>
> +
> +#ifndef LLVM_CODEGEN_MACHINEDEBUGINFODESC_H
> +#define LLVM_CODEGEN_MACHINEDEBUGINFODESC_H
> +
> +#include "llvm/GlobalValue.h"

Is it possible to just extern the classes that are used?

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h Thu Jun 26  
> 19:09:40 2008
> @@ -31,18 +31,20 @@
> #ifndef LLVM_CODEGEN_MACHINEMODULEINFO_H
> #define LLVM_CODEGEN_MACHINEMODULEINFO_H
>
> #include "llvm/GlobalValue.h"
> #include "llvm/Pass.h"
> +#include "llvm/ADT/SmallPtrSet.h"
> +#include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/UniqueVector.h"
> +#include "llvm/Support/DataTypes.h"
> +#include "llvm/Support/Dwarf.h"

Does this header depend on MachineDebugInfo.h?  If so, it should  
#include it.

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/DwarfWriter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/DwarfWriter.cpp Thu Jun 26 19:09:40 2008
> @@ -20,6 +20,7 @@
> #include "llvm/Module.h"
> #include "llvm/Type.h"
> #include "llvm/CodeGen/AsmPrinter.h"
> +#include "llvm/CodeGen/MachineDebugInfoDesc.h"
> #include "llvm/CodeGen/MachineModuleInfo.h"

Are these headers really independent of each other?

Thanks for tackling this Bill!

-Chris