Code generator: allow for variables to be tracked/untracked

[agarny]

Fixes #1244.

[hsorby]

What is the thinking behind always passing an analyser model to the generator class, as opposed to setting the analyser model for the generator to work on?

[hsorby]

Other than this question:

What is the thinking behind always passing an analyser model to the generator class, as opposed to setting the analyser model for the generator to work on?

I am happy with this work.

[agarny]

What is the thinking behind always passing an analyser model to the generator class, as opposed to setting the analyser model for the generator to work on?

Oops, I went through the inline comments completely overlooking this one.

Anyway, are you talking about methods like un/trackAllConstants(), un/trackAllComputedConstants(), etc. that take an analyser model as an input? If so, it's because a generator can be reused with different analyser models. This is why we don't allow the creation of a generator for a given analyser model.

[hsorby]

You are asking the Generator to hold a map of analysermodels and their map of tracked and untracked variables. The Generator should really be stateless. I have three problems with this approach:

1. mTrackedVariables will never shrink while the generator object is alive
2. This is not thread safe
3. The genereator is responsible for the lifecycle management of the mTrackedVariables when that is not its concern

For me the tracked varaibles belong with the AnalyserModel itself. Alternatively, you could create a generation context that holds the tracked variable information, and generator profile and you pass the AnalyserModel and GenerationContext when asking the generator to generate. This also means that the Generator doesn’t hold a reference to a generator profile this is passed in when the generation happens. For me this would be a massive archetictural improvement for the library.

[agarny]

The un/tracking of variables is only relevant in the context of code generation, which is why mTrackedVariables is managed by the generator. To have mTrackedVariables managed by the analyser model would prevent the analyser model from being used to generate code with different un/tracked variables, so I don't think the analyser model should manage mTrackedVariables.

You are asking the Generator to hold a map of analysermodels and their map of tracked and untracked variables. The Generator should really be stateless. I have three problems with this approach:

1. mTrackedVariables will never shrink while the generator object is alive

Correct, although we could always a method to reset mTrackedVariables, if needed.

2. This is not thread safe

True, but then again the generator is not thread-safe to start with.

3. The genereator is responsible for the lifecycle management of the mTrackedVariables when that is not its concern

For me the tracked varaibles belong with the AnalyserModel itself. Alternatively, you could create a generation context that holds the tracked variable information, and generator profile and you pass the AnalyserModel and GenerationContext when asking the generator to generate. This also means that the Generator doesn’t hold a reference to a generator profile this is passed in when the generation happens. For me this would be a massive archetictural improvement for the library.

I agree, a better approach would be to have a new class (GeneratorContext) that manages un/tracked variables for a given analyser model. Kind of similar to what was done with GeneratorProfile, which means that we would then have Generator::context() and Generator::setContext(const GeneratorContextPtr &context).

However, I would do that as part of a different GitHub issue. Time is pressing.

[hsorby]

This PR also:

• Fixes Change Anaylser API: model() -> analyserModel() #1056.