-
Notifications
You must be signed in to change notification settings - Fork 265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: DafnyOptions.O.Compiler is null, preventing instantiation of ModuleExportDecl #1933
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the right fix: we should make sure that the compiler is properly instantiated.
Source/Dafny/AST/DafnyAst.cs
Outdated
EnclosingModuleDefinition.CompileName, CompileName); | ||
} else { | ||
return Declaration.IdProtect(EnclosingModuleDefinition.CompileName) + "." + Declaration.IdProtect(CompileName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we not instantiate the compiler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the AST is the right place to instantiate a compiler anyway, it should be instantiated in the options if really instantiated manually. I we don't want the compiler to be null, I think we should always instantiate a default C# compiler. Would you prefer that?
RELEASE_NOTES.md
Outdated
@@ -1,7 +1,8 @@ | |||
# Upcoming | |||
|
|||
- fix: Miscompilation due to incorrect parenthesization in C# output for casts. (#1908) | |||
- fix: Miscompilation due to incorrect parenthesization in C# output for casts. (https://github.com/dafny-lang/dafny/pull/1908) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you add the full link?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that it's browsable when reading the README.md from a text editor, and in Github markdown it still displays nicely "#1908"
Could you have a look at #1945 ? I don't think defaulting to the C# compiler is the right fix. |
Can you fix the conflicts in RELEASE_NOTES? |
Fixes #1932
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.