forked from sbcl/sbcl
-
Notifications
You must be signed in to change notification settings - Fork 0
/
HACKING
206 lines (172 loc) · 8.22 KB
/
HACKING
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
SBCL Hacking Guide
(This is not a most actively maintained file, but recommended
reading anyways.)
Table of Contests
* Modifying
* Patch Submissions
* Coding Style
* Reader Conditionals
* Comments and Documentation
* Syntax-related Conventions
* Error and Warning Messages
* Format Strings
* Writing Tests
Modifying
=========
To change the code at run-time the :SB-DEVEL feature is almost always
necessary, otherwise a lot of macros, functions, and variables do not
survive in the final image.
Patch Submissions
=================
Preferred patch format is output from "git format-patch", including
the commit message.
The commit message should explain the why and how of the change. See
existing commit messages for examples -- for a truly trivial changes
little is needed, but in most cases more is better.
Please include test-cases in your patch if at all possible: if you're
not sure which file in tests/ to put your test case in, just pick one
that seems vaguely appropriate. See the "Writing Tests" section for
more information.
Please format your submission for ease of reading: unless the change
is whitespace only, avoid re-indenting code you are not touching, etc.
Unless your change is large and best understood as a series of
sequential changes, please send it in as single patch.
If your patch includes algorithmic changes, explain them. If your
patch uses a published algorithm, please include a link to the paper.
We aren't always as well-educated as we'd like to be...
Ready-to-apply patches should be submitted via Launchpad: please add
the tag "review" to the associated bug (create new bug with name if
there isn't one about the issue yet.)
Patches requiring more widespread discussion and feedback should be
sent to the sbcl-devel mailing list.
If you have any questions, feel free to ask them on sbcl-devel,
or the IRC channel #[email protected].
Coding Style
============
See also PRINCIPLES and TLA files.
Most of the style hints in the Lisp FAQ apply.
When porting code we prefer code which factors dependencies into a set
of interface functions and constants and includes implementations of
the interface for the different systems.
Reader conditionals are frowed upon. Sometimes they're the least
of all evils, but think thrice.
grammatical fussiness:
Phrases are not capitalized.
Sentences are capitalized.
Periods terminate sentences.
Periods separate phrases from succeeding sentences, e.g.
;;; the maximum number of transformations we'll make before
;;; concluding we're in an infinite loop and bailing. This can
;;; be changed, but it is an error to change it while we're
;;; solving a system.
(defvar *max-n-transformations* 10)
Lisp in comments is capitalized.
Symbol names are capitalized.
usage fussiness:
Function documentation can be a description of what the function
does, e.g.
;;; Parse the arguments for a BDEFSTRUCT call, and return
;;; (VALUES NAME DEFSTRUCT-ARGS MAKE-LOAD-FORM-FUN BDEFSTRUCT-STYPE),
;;; where NAME is the name of the new type, DEFSTRUCT-ARGS is the
;;; munged result suitable for passing on to DEFSTRUCT,
;;; MAKE-LOAD-FORM-FUN is the make load form function, or NIL if
;;; there's none, and BDEFSTRUCT-SUPERTYPE is the direct supertype
;;; of the type if it is another BDEFSTRUCT-defined type, or NIL
;;; otherwise.
(defun parse-bdefstruct-args (nameoid &rest rest)
..)
or a remark about the function, e.g.
;;; a helper function for BDEFSTRUCT in the #+XC-HOST case
(defun uncross-defstruct-args (defstruct-args)
..)
If you're talking about what the function does, ordinarily you
should just say what the function does, e.g.
;;; Return the first prime number greater than or equal to X.
(defun primify (x) ..)
instead of telling the reader that you're going to tell him what
the function does, e.g.
;;; PRIMIFY returns the first prime number greater than or
;;; equal to X.
(defun primify (x) ..)
or
;;; When you call this function on X, you get back the first
;;; prime number greater than or equal to X.
(defun primify (x) ..)
Documentation for public functions belongs in a docstring.
Documentation for internal functions belongs mostly in a comment.
In general, if you can express it in the code instead of the comments,
do so. E.g. the old CMUCL code has many comments above functions foo
that say things like
;;; FOO -- interface
If we were going to do something like that, we would prefer to do it by
writing
(EXPORT 'FOO)
(Instead, for various other reasons, we centralize all the exports
in package declarations.) The old "FOO -- interface" comments are bad
style because they duplicate information (and they illustrate one
of the evils of duplicating information by the way that they have
drifted out of sync with the code).
Writing Tests
=============
As mentioned in the "Patch Submissions" section, new features as well
as bug fixes should always be accompanied by smoke, unit or regression
tests as appropriate.
New tests should be added in the appropriate file in the "tests"
directory. Files named TOPIC.pure.lisp contain tests which do not have
side effects such as modifying the environment, defining functions or
variables or starting new threads while files named TOPIC.impure.lisp
contain test which do (impure test files are run each in a separate
SBCL process).
All tests should use the WITH-TEST macro to associate a unique name
with each test (see below) and prevent a whole test file from going up
in smoke without an adequate description of the problem. The basic
syntax is
(with-test (:name NAME)
BODY)
where NAME is either a symbol or a list of symbols. All of these
symbols have to be in one of the packages KEYWORD, CL or SB-* to make
test names READable in SBCL images that did not read the test
files. For tests associated to Launchpad bugs (because a feature
requested in the LP bug has been implemented or a bug has been fixed),
the LP bug number should be a part of the test name:
(with-test (:name (coerce :invalid-target-type :bug-12345))
…)
When a test can be associated to a name in the CL package or one of
the SB-* packages, that name should appear as a separate component in
the test name, like COERCE in the example above. This may be useful in
order to automatically associate functions and tests.
If possible, tests should not signal warnings during compilation or
runtime and should not produce output in order to not clutter the
output of the test runner and make real problems easier to identify.
Misc
----
There are a number of style practices on display in the code
which are not good examples to follow:
* using conditional compilation to support different architectures,
instead of factoring the dependencies into interfaces and providing
implementations of the interface for different architectures;
* in conditional compilation, using a common subexpression over and
over again, e.g. #+(OR X86 X86-64), when the important thing is
that the platform supports single-instruction CAS. If you have to
do something like that, define a new FOO feature, write #+FOO in
many places, and arrange for the FOO feature to be set once and
only once -- probably in make-config.sh. (That way future
maintainers won't curse you.)
* putting the defined symbol, and information about whether it's
exported or not, into the comments around the definition of the symbol;
* naming anything DO-FOO if it isn't an iteration macro
* not using a consistent abbreviation style in global names (e.g.
naming some things DEFINE-FOO and other things DEF-BAR, with
no rule to determine whether the abbreviation is used).
* using lots of single-colon package prefixes (distracting and hard
to read, and obstacles to reaching package nirvana where
package dependencies are a directed acyclic graph) or even
double-colon package prefixes (hard to understand and hard
to maintain). (One exception: I've sometimes been tempted to
add a CL: prefix to the definition of every CL symbol (e.g.
(DEFUN CL:CADDDR (..) ..) as reminders that they're required by
ANSI and can't be deleted no matter how obscure and useless some
of them might look.:-)
Many of these are common in the code inherited from CMUCL. We've
eliminated them in some places, but there's a *lot* of code inherited
from CMUCL..