[llvm] r202190 - Store a DataLayout in Module.

Philip Reames listmail at philipreames.com
Tue Feb 25 12:58:54 PST 2014


Late review comments below.

On 02/25/2014 12:01 PM, Rafael Espindola wrote:
> Author: rafael
> Date: Tue Feb 25 14:01:08 2014
> New Revision: 202190
>
> URL: http://llvm.org/viewvc/llvm-project?rev=202190&view=rev
> Log:
> Store a DataLayout in Module.
>
> Now that DataLayout is not a pass, store one in Module.
>
> Since the C API expects to be able to get a char* to the datalayout description,
> we have to keep a std::string somewhere. This patch keeps it in Module and also
> uses it to represent modules without a DataLayout.
>
> Once DataLayout is mandatory, we should probably move the string to DataLayout
> itself since it won't be necessary anymore to represent the special case of a
> module without a DataLayout.
>
> ...
>   
> +void Module::setDataLayout(StringRef Desc) {
> +  if (Desc.empty()) {
> +    DataLayoutStr = "";
> +  } else {
> +    DL.init(Desc);
> +    DataLayoutStr = DL.getStringRepresentation();
> +  }
> +}
Shouldn't we clear DL if you store an empty string?

Is it expected that DataLayoutStr will equal Desc after this method is 
called?  If so, please add an assert.  Otherwise, please document.

Philip



More information about the llvm-commits mailing list