Add a warning for generic superclasses that don't include (?) #22784
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
bradcray
reviewed
Jul 20, 2023
bradcray
reviewed
Jul 20, 2023
bradcray
reviewed
Jul 20, 2023
mppf
added a commit
that referenced
this pull request
Jul 27, 2023
) Resolves Cray/chapel-private#5032 This PR adds a warning for these cases: * `var x: someGenericType` for fields and variables (need `someGenericType(?)`) * `proc foo(type t) { var x: t; }` -- similarly to `var x: someGenericType` * `var x: f();` where `f()` returns a generic type, for fields and variables In the above, `someGenericType` includes `domain` or user records/classes that are generic. Note that PR #22697 added a warning for fields declared with type `domain` or fields of class type with generic management. Reviewed by @vasslitvinov - thanks! - [x] full comm=none testing Future work * warn for `class C : SomeGenericType` -- PR #22784
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
--- Signed-off-by: Michael Ferguson <[email protected]>
mppf
force-pushed
the
warn-generic-superclass-no-q
branch
from
45d48f8
to
8604c89
Compare
vasslitvinov
approved these changes
Jul 27, 2023
--- Signed-off-by: Michael Ferguson <[email protected]>
mppf
added a commit
to mppf/chapel
that referenced
this pull request
Aug 4, 2023
Adjusts the warnings from PRs chapel-lang#22697 chapel-lang#22745 and chapel-lang#22784 to include a note about the warning possibly being an error in the future when compiling with `--warn-unstable`. --- Signed-off-by: Michael Ferguson <[email protected]>
1 task
mppf
added a commit
that referenced
this pull request
Aug 8, 2023
Adjusts the warnings from PRs #22697 #22745 and #22784 to include a note about the warning possibly being an error in the future when compiling with `--warn-unstable`. Reviewed by @lydia-duncan - thanks! - [x] full comm=none testing
vasslitvinov
added a commit
to vasslitvinov/chapel
that referenced
this pull request
Aug 15, 2023
Update CoMD's AccumStencilDist to eliminate recent deprecation warnings, especially from chapel-lang#22784 and chapel-lang#22858. CoMD's prediff filters out warnings so newly-introduced warnings go unnoticed in paratest testing. While there, make some more changes that make CoMD's AccumStencilDist closer to our standard StencilDist. In particular, change a couple of occurrences of `.low` and `.high` to `.lowBound` and `.highBound`. The corresponding changes in StencilDist were made in chapel-lang#19820 when the warnings on `.low` / `.high` were enabled. Now `.low` / `.high` no longer generate warnings, however have a (slightly) different meaning than they had before chapel-lang#19820. Future work: * Should we changes more occurrences of `.low` / `.high` to `.lowBound` / `.highBound` following the changes to StencilDist in chapel-lang#19820? * Eliminate deprecation warnings about TimeUnits and getCurrentTime() in the llnl version. * Perhaps the `.prediff` for CoMD should not filter out warnings any more? Implementation-wise, it means that these scripts now should include warnings, if any, in their output. Signed-off-by: Vassily Litvinov <[email protected]>
vasslitvinov
added a commit
that referenced
this pull request
Aug 15, 2023
Update CoMD's AccumStencilDist to eliminate recent deprecation warnings, especially from #22784 and #22858, following the changes to our standard StencilDist in those PRs. CoMD's prediff filters out warnings, so newly-introduced warnings go unnoticed in paratest testing. While there, make some more changes that make CoMD's AccumStencilDist closer to our standard StencilDist. In particular, change a couple of occurrences of `.low` and `.high` to `.lowBound` and `.highBound`. The corresponding changes in StencilDist were made in #19820 when the warnings on `.low` / `.high` were enabled. Now `.low` / `.high` no longer generate warnings, however have a (slightly) different meaning than they had before #19820. Future work: * Should we changes more occurrences of `.low` / `.high` to `.lowBound` / `.highBound` following the changes to StencilDist in #19820? * Eliminate deprecation warnings about TimeUnits and getCurrentTime() in the llnl version. * Perhaps the `.prediff` for CoMD should not filter out warnings any longer? Implementation-wise, it means that these scripts should now include warnings, if any, in their output. r: @benharsh
mppf
added a commit
that referenced
this pull request
Sep 21, 2023
This PR updates the language specification to cover the changes in PRs #23291 #22784 #22745 #23356 where marking a generic type with `(?)` is now needed. This PR adds several new spec sections: * Fields with Generic Types * Fully Defaulted Generic Types * Marking Generic Types Reviewed by @lydia-duncan - thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR takes two steps:
class C : GenericParent(?)in order to clearly indicate thatclass Cis generic due to inheriting from a generic class.class C : ConcreteParent(?).The rationale for the warning is that, if code compiling without these warnings (including the field warning from PRs #22745 and #22697) then it is syntactically apparent whether or not a class or record is generic. That means that, in the future, if we make these errors, a) the compiler doesn't have to work as hard to know if a type is generic and b) neither do users.
Reviewed by @vasslitvinov - thanks!