Skip to content
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

lang/stdlib: remove quick-win deprecated items #393

Closed

Conversation

saem
Copy link
Collaborator

@saem saem commented Aug 2, 2022

Summary

  • removes many deprecated parts of the language/standard library
  • should result in fewer dialects and faster CI to say the least

Details

  • to be filled in

Notes for Reviewers

still need to think of a merging strategy, inclined to cherry pick and merge one at a time
the above is slow from a CI/manual effort perspective, it's cleaner, however. suggestions?
(especially @alaviss)

Need to wrap this up, I left off at the 1.6 release notes.

saem added 12 commits July 31, 2022 14:34
deprecated stdlib module removal:
- removed events, LockFreeHash, parseopt2, and securehash
- associated clean-up in testament
- lib/deprecate/pure now only contains ospaths, an include for os
- remove events related tests
- can no longer use 0c1
- this has long since been deprecated and not referenced in the manual
not used in the code based, removing
Removed the this pragma, but codegen is now broken:
- codegen no longer renames parameters that happen to be called `this`
- can't find a reasonable causal chain that explains why
- disabled tests that fail on the cpp backend in the meantime

Added benefit:
- removed the `nfExprCall` flag
- remove the `sfIsSelf` const/flag
- CI should run faster and less cruft in dispatch

It also became clear that `semIndirectOp` and call patterns around it
need a complete rewrite. The results should be simplified dispatch.
note reset magic is still present
removed the previous deprecation where pragmas could preceed the generic
parameter list in a type definition, eg:

```
type Foo{.somePragma}[T] = ... # now an error
type Foo[T] {.somePragma.} = ... # acceptable
```

Due to lots of issues, mostly stylistic, I've pragmas on the RHS of a
type definition as legal. Largely because after changing it I realized a
more significant rethink of type declaration syntax should cover this in
the future.

Fixed a number of test failures along the way due to this change.
dropped proc from stdlib, it been deprecatd forever
functionality has long since been moved to `std/os`
Removed deprecated constructors and setters for `times.DateTime`

Updated associated broken tests.
`add` was deprecated because it allowed mapping multiple values to a key
This was understandably very confusing, now add is removed.

additionally, sharedtables and sharedlists were deprecated, only
sharedtables was removed as sharedlists is still used when threads are
enabled.

Fixed broken tests.
fileExists/dirExists are already present
supposedly it was ignored by the compiler, the fewer pragmas the better.
@saem saem changed the title Saem core quick wins 1 remove deprecated lang/stdlib: remove quick-win deprecated items Aug 2, 2022
@saem
Copy link
Collaborator Author

saem commented Aug 2, 2022

@Clyybber got any specific input on these two in particular?

Comment on lines +2072 to +2091
(genericParams, pragmas) =
case p.tok.tokType
of tkBracketLe:
(
(if p.validInd: parseGenericParamList(p) else: p.emptyNode),
optPragmas(p)
)
# xxx: should the else/empty generic branch invalid indentation error?
of tkCurlyDotLe:
let res = (p.emptyNode, optPragmas(p))

if p.validInd and p.tok.tokType == tkBracketLe:
p.localError ParserReport(kind: rparPragmaBeforeGenericParameters)

res
of tkEquals:
(p.emptyNode, p.emptyNode)
# parsing for everything after the equals is shared, we do that below
else:
(p.emptyNode, p.emptyNode) # xxx: error?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else case is handled by sem, type abc on it's own is valid in the parser, so I'd merge the tkEquals and the else branch. If we don't want/need a specialized error message for "pragma before generic param" then we can also remove the tkCurlyDotLe branch and have it handled with the generic "unexpected X" automatically I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to start with a focused grammar, then loosen restrictions over time (reasoning for all this is in the 'out loud thinking' below). So having the grammar restricted in the parser seems reasonable and I'd like to introduce more of that.

I likely don't understand the art of grammar construction well enough, so I'm wondering if it's reasonable to say that type abc shouldn't be valid here hence the else being separate?

As for the special case error message for pragma and position ordering, that's friendlier to humans.


(Out loud thinking)

I'm still getting a feel for what should be where, in particular a set of principles in place.

If we know there is an irregularity in the grammar we should reject it early, IMO. We get far more helpful error messages that way in that we've got better context.

There are a bunch of competing ideas in my head, but they boil down to two main "sides":

  1. the focused grammar of Nimskull the language itself and building to that, with crisp and clear error messages, quick detection of issues and syntax limiting is more useful, etc
  2. a superset grammar, that can represent all legal terms the former does and, is more forgiving for the purposes of building DSLs and other things macros should let you do. All within reason, not full reader macro control like lisp/racket.

The focused route (1) has lots of upsides, not the least of which is simplicity in how direct it is -- consider how much people struggled with |, which is alternation in regular grammars.

1 and 2 are somewhat mutually exclusive, we can start with one and move to 2, but the other way is much harder. We can also have 1 be the norm and in special sections drop to 2. In the end we need to pick a horse.

I started to sketch out grammar structures for proc and type definitions in my notepad to see what the symmetries are, mostly ignoring current syntax. I'm starting to get some insights, but it's very nascent and even if it was ready as an idea, it's an entirely different matter implementing it.

@alaviss
Copy link
Contributor

alaviss commented Aug 3, 2022

still need to think of a merging strategy, inclined to cherry pick and merge one at a time
the above is slow from a CI/manual effort perspective, it's cleaner, however. suggestions?
(especially @alaviss)

That's certainly the best when it comes to: "Every commit in devel is green".

If it's too much hassle a squash is certainly possible, though this change is a bit big for a squash, so you'd have to squash by categories.

- remove deprecated `delete(var string, int, int)`
- removed associated tests
- updated usage of removed proc in rstgen
`request` proc's `httpMethod` param no longer accepts strings
remove deprecated `delete[T](var T, int, int)` and its tests
- remove deprecated `reversed[T](var openArray[T], int, int): seq[T]`
- removed associated tests
- removed UnpackError, now uses UnpackDefect
- Defects in general are a bad idea, but less code
@saem saem marked this pull request as draft August 6, 2022 01:46
@saem
Copy link
Collaborator Author

saem commented Aug 7, 2022

All changes merged in, closing PR.

@saem saem closed this Aug 7, 2022
@haxscramper haxscramper added the stdlib Standard library label Nov 21, 2022
@saem saem deleted the saem-core-quick-wins-1-remove-deprecated branch January 22, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Standard library
Development

Successfully merging this pull request may close these issues.

None yet

4 participants