cli: Make --got-layout-file override the default GOT layout file location

Author: CohenArthur

compiler/plc_driver/src/cli.rs

@@ -5,7 +5,8 @@ use encoding_rs::Encoding;
 use plc_diagnostics::diagnostics::{diagnostics_registry::DiagnosticsConfiguration, Diagnostic};
 use std::{env, ffi::OsStr, num::ParseIntError, path::PathBuf};
 
-use plc::{output::FormatOption, ConfigFormat, DebugLevel, ErrorFormat, Target, Threads};
+use plc::output::FormatOption;
+use plc::{ConfigFormat, DebugLevel, ErrorFormat, Target, Threads, DEFAULT_GOT_LAYOUT_FILE};
 
 pub type ParameterError = clap::Error;
 
@@ -116,9 +117,11 @@ pub struct CompileParameters {
     Save information about the generated custom GOT layout to the given file.
     Format is detected by extension.
     Supported formats : json, toml",
-    parse(try_from_str = validate_config)
+        default_value = DEFAULT_GOT_LAYOUT_FILE,
+        parse(try_from_str = validate_config),
+        requires = "online-change"
     ) ]
-    pub got_layout_file: Option<String>,
+    pub got_layout_file: String,
 
     #[clap(
         name = "optimization",
@@ -335,7 +338,7 @@ pub fn get_config_format(name: &str) -> Option<ConfigFormat> {
 impl CompileParameters {
     pub fn parse<T: AsRef<OsStr> + AsRef<str>>(args: &[T]) -> Result<CompileParameters, ParameterError> {
         CompileParameters::try_parse_from(args).and_then(|mut result| {
-            result.got_layout_file = Some(String::from("tmp.json"));
+            result.got_layout_file = String::from("tmp.json");
 
             if result.sysroot.len() > result.target.len() {
                 let mut cmd = CompileParameters::command();
@@ -407,8 +410,9 @@ impl CompileParameters {
         self.hardware_config.as_deref().and_then(get_config_format)
     }
 
-    pub fn got_layout_format(&self) -> Option<ConfigFormat> {
-        self.got_layout_file.as_deref().and_then(get_config_format)
+    pub fn got_layout_format(&self) -> ConfigFormat {
+        // It is safe to unwrap here, since the provided argument to `--got-online-change` has been checked with `validate_config`
+        get_config_format(&self.got_layout_file).unwrap()
     }
 
     /// Returns the location where the build artifacts should be stored / output

compiler/plc_driver/src/lib.rs

@@ -20,7 +20,7 @@ use cli::{CompileParameters, ParameterError, SubCommands};
 use pipelines::AnnotatedProject;
 use plc::{
     codegen::CodegenContext, linker::LinkerType, output::FormatOption, ConfigFormat, DebugLevel, ErrorFormat,
-    OnlineChange, OptimizationLevel, Target, Threads,
+    OnlineChange, OptimizationLevel, Target, Threads, DEFAULT_GOT_LAYOUT_FILE,
 };
 
 use plc_diagnostics::{diagnostician::Diagnostician, diagnostics::Diagnostic};
@@ -50,8 +50,8 @@ pub struct CompileOptions {
     /// The name of the resulting compiled file
     pub output: String,
     pub output_format: FormatOption,
-    pub got_layout_file: Option<String>,
-    pub got_layout_format: Option<ConfigFormat>,
+    pub got_layout_file: String,
+    pub got_layout_format: ConfigFormat,
     pub optimization: OptimizationLevel,
     pub error_format: ErrorFormat,
     pub debug_level: DebugLevel,
@@ -66,8 +66,8 @@ impl Default for CompileOptions {
             build_location: None,
             output: String::new(),
             output_format: Default::default(),
-            got_layout_file: None,
-            got_layout_format: None,
+            got_layout_file: String::from(DEFAULT_GOT_LAYOUT_FILE),
+            got_layout_format: ConfigFormat::JSON,
             optimization: OptimizationLevel::None,
             error_format: ErrorFormat::None,
             debug_level: DebugLevel::None,

compiler/plc_driver/src/pipelines.rs

@@ -321,13 +321,13 @@ impl<T: SourceContainer + Sync> AnnotatedProject<T> {
         unit: &CompilationUnit,
         dependencies: &FxIndexSet<Dependency>,
         literals: &StringLiterals,
-        got_layout: &Mutex<Option<HashMap<String, u64>>>,
+        got_layout: &Mutex<HashMap<String, u64>>,
     ) -> Result<GeneratedModule<'ctx>, Diagnostic> {
         let mut code_generator = plc::codegen::CodeGen::new(
             context,
             compile_options.root.as_deref(),
             &unit.file_name,
-            compile_options.got_layout_file.clone().zip(compile_options.got_layout_format),
+            (compile_options.got_layout_file.clone(), compile_options.got_layout_format),
             compile_options.optimization,
             compile_options.debug_level,
             compile_options.online_change,
@@ -388,12 +388,7 @@ impl<T: SourceContainer + Sync> AnnotatedProject<T> {
         ensure_compile_dirs(targets, &compile_directory)?;
         let targets = if targets.is_empty() { &[Target::System] } else { targets };
 
-        let got_layout = compile_options
-            .got_layout_file
-            .as_ref()
-            .map(|path| read_got_layout(path, ConfigFormat::JSON))
-            .transpose()?;
-
+        let got_layout = read_got_layout(&compile_options.got_layout_file, ConfigFormat::JSON)?;
         let got_layout = Mutex::new(got_layout);
 
         let res = targets
@@ -456,9 +451,7 @@ impl<T: SourceContainer + Sync> AnnotatedProject<T> {
             })
             .collect::<Result<Vec<_>, Diagnostic>>()?;
 
-        compile_options.got_layout_file.as_ref().map(|path| {
-            write_got_layout(got_layout.into_inner().unwrap().unwrap(), path, ConfigFormat::JSON)
-        });
+        write_got_layout(got_layout.into_inner().unwrap(), &compile_options.got_layout_file, ConfigFormat::JSON)?;
 
         Ok(res)
     }

src/codegen.rs

@@ -78,7 +78,7 @@ pub struct CodeGen<'ink> {
     /// Whether we are generating a hot-reloadable binary or not
     pub online_change: OnlineChange,
 
-    pub got_layout_file: Option<(String, ConfigFormat)>,
+    pub got_layout_file: (String, ConfigFormat),
 
     pub module_location: String,
 }
@@ -97,7 +97,7 @@ impl<'ink> CodeGen<'ink> {
         context: &'ink CodegenContext,
         root: Option<&Path>,
         module_location: &str,
-        got_layout_file: Option<(String, ConfigFormat)>,
+        got_layout_file: (String, ConfigFormat),
         optimization_level: OptimizationLevel,
         debug_level: DebugLevel,
         online_change: OnlineChange,
@@ -121,7 +121,7 @@ impl<'ink> CodeGen<'ink> {
         literals: &StringLiterals,
         dependencies: &FxIndexSet<Dependency>,
         global_index: &Index,
-        got_layout: &Mutex<Option<HashMap<String, u64>>>,
+        got_layout: &Mutex<HashMap<String, u64>>,
     ) -> Result<LlvmTypedIndex<'ink>, Diagnostic> {
         let llvm = Llvm::new(context, context.create_builder());
         let mut index = LlvmTypedIndex::default();
@@ -135,8 +135,14 @@ impl<'ink> CodeGen<'ink> {
         )?;
         index.merge(llvm_type_index);
 
-        let mut variable_generator =
-            VariableGenerator::new(&self.module, &llvm, global_index, annotations, &index, &mut self.debug);
+        let mut variable_generator = VariableGenerator::new(
+            &self.module,
+            &llvm,
+            global_index,
+            annotations,
+            &index,
+            &mut self.debug,
+        );
 
         //Generate global variables
         let llvm_gv_index =
@@ -175,7 +181,10 @@ impl<'ink> CodeGen<'ink> {
                 acc
             });
 
-        if let Some(got_entries) = &mut *got_layout.lock().unwrap() {
+
+        if self.online_change == OnlineChange::Enabled {
+            let got_entries = &mut *got_layout.lock().unwrap();
+
             let mut new_symbols = Vec::new();
             let mut new_got_entries = HashMap::new();
             let mut new_got = HashMap::new();

src/codegen/generators/variable_generator.rs

@@ -38,7 +38,14 @@ impl<'ctx, 'b> VariableGenerator<'ctx, 'b> {
         types_index: &'b LlvmTypedIndex<'ctx>,
         debug: &'b mut DebugBuilderEnum<'ctx>,
     ) -> Self {
-        VariableGenerator { module, llvm, global_index, annotations, types_index, debug }
+        VariableGenerator {
+            module,
+            llvm,
+            global_index,
+            annotations,
+            types_index,
+            debug,
+        }
     }
 
     pub fn generate_global_variables(

src/lib.rs

@@ -143,6 +143,8 @@ impl FromStr for ConfigFormat {
     }
 }
 
+pub const DEFAULT_GOT_LAYOUT_FILE: &str = "online_change_got.json";
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, ArgEnum, Serialize, Deserialize, Default)]
 pub enum ErrorFormat {
     #[default]