Antaris / RazorEngine

Open source templating engine based on Microsoft's Razor parsing engine
http://antaris.github.io/RazorEngine
Other
2.13k stars 576 forks source link

CompilerOptions should not include "/optimize" when "CompilerServiceBase.Debug" is "true". #361

Open cklutz opened 8 years ago

cklutz commented 8 years ago

Currently, the compiler options (see DirectCompilerServiceBase.Compile()) always include "/optimize". However, when the "CompilerServiceBase.Debug" property is "true", this should not be the case (or alternatively be triggered through an additional property, e.g. "CompilerServiceBase.Optimize").

The reason for this is that the line numbers included in "runtime" stack traces, e.g.

 NullReferenceException: Object reference not set to an instance of an object.
      at CompiledRazorTemplates.Dynamic.RazorEngine_8b57f1088336400b8b8ee1f78a6d1746.Execute() in Whatever.cshtml:line 123

are otherwise wrong. In the above example, the 123 is not the actual line number from "Whatever.cshtml", because of optimization turned on.

(Probably likewise for the "Razor 4.0" code, but I haven't checked.)

matthid commented 8 years ago

Can you come up with a pr?

cklutz commented 8 years ago

I'd like to, unfortunately I'm not able to come up with a proper PR (with tests and also for Razor) until at least mid/end of April (I'm on a prolonged vacation in a couple of weeks and too busy before that :-( ) If you want to wait for at least that long, then do. I'll see what I can do then.

Alas, the fix for the CodeDom based code is rather trivial. Just replace this in DirectCompilerServiceBase.cs:

            var @params = new CompilerParameters
            {
                GenerateInMemory = false,
                GenerateExecutable = false,
                IncludeDebugInformation = Debug,
                TreatWarningsAsErrors = false,
                TempFiles = new TempFileCollection(GetTemporaryDirectory(), true),
                CompilerOptions =
                    string.Format("/target:library /optimize /define:RAZORENGINE {0}",
                        haveMscorlib ? "/nostdlib" : "")
            };

with something like this:

            var compilerOptions = new StringBuilder("/target:library /define:RAZORENGINE");
            if (haveMscorlib)
                compilerOptions.Append(" /nostdlib");
            if (!Debug)
                compilerOptions.Append(" /optimize");

            var @params = new CompilerParameters
            {
                GenerateInMemory = false,
                GenerateExecutable = false,
                IncludeDebugInformation = Debug,
                TreatWarningsAsErrors = false,
                TempFiles = new TempFileCollection(GetTemporaryDirectory(), true),
                CompilerOptions = compilerOptions.ToString()
            };
cklutz commented 8 years ago

Dang, "(with tests and also for Razor)" should be "(with tests and also for Roslyn)"