-
Notifications
You must be signed in to change notification settings - Fork 61
Reorder comments, function attributes, and macro def args #251
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
Merged
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
14e36ca
initial implementation
elcritch 292c4e3
add example
elcritch e313204
updates
elcritch db45d29
updates
elcritch 2b689db
plumb in comments
elcritch 14c7918
plumb in comments
elcritch 8487f2c
add ability to define macros
elcritch 0177c71
cleanup
elcritch adc221f
cleanup
elcritch 2916d19
space procs
elcritch 9899078
describe additions
elcritch 0389190
describe additions
elcritch c4185d1
change to getTempDir
elcritch d84803c
add consts
elcritch cea910d
move test files
elcritch 0d225bc
reorganizing
elcritch 29ef92c
plumb in renber options
elcritch b43d4ad
update tests
elcritch 1f6833d
updates
elcritch 37777bc
handle /// comments
elcritch d0ccc62
tweak comments
elcritch 6c06b63
tweak comments
elcritch aad6b5c
tweak comments
elcritch 5ad5b38
more comment fixups
elcritch abb23e5
more comment fixups
elcritch 2c53036
revert comment spacing to keep consistent for now
elcritch b0c35df
fix macro args
elcritch c09c2b2
fix macro args
elcritch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
nimcache | ||
testsuite/tests/*.nim | ||
testsuite/cppkeepbodies/*.nim | ||
testsuite/cextras/*.nim | ||
testsuite/tester | ||
*.exe | ||
/c2nim |
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
# distribution, for details about the copyright. | ||
# | ||
|
||
import std / [strutils, os, times, parseopt, strscans] | ||
import std / [strutils, os, times, md5, parseopt, strscans] | ||
|
||
import compiler/ [llstream, ast, renderer, options, msgs, nversion] | ||
|
||
|
@@ -43,6 +43,10 @@ Options: | |
--importc annotate procs with ``{.importc.}`` | ||
--importdefines import C defines as procs or vars with ``{.importc.}`` | ||
--importfuncdefines import C define funcs as procs with ``{.importc.}`` | ||
--def:SYM='macro()' define a C macro that gets replaced with the given | ||
definition. It's parsed by the lexer. Use it to fix | ||
function attributes: ``--def:PUBLIC='__attribute__ ()'`` | ||
--reordercomments reorder C comments to match Nim's postfix style | ||
--ref convert typ* to ref typ (default: ptr typ) | ||
--prefix:PREFIX strip prefix for the generated Nim identifiers | ||
(multiple --prefix options are supported) | ||
|
@@ -52,6 +56,7 @@ Options: | |
for example `--mangle:'{u?}int{\d+}_t=$1int$2'` to | ||
convert C <stdint.h> to Nim equivalents | ||
(multiple --mangle options are supported) | ||
--stdints Mangle C stdint's into Nim style int's | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this would feel more natural as a |
||
--paramprefix:PREFIX add prefix to parameter name of the generated Nim proc | ||
--assumedef:IDENT skips #ifndef sections for the given C identifier | ||
(multiple --assumedef options are supported) | ||
|
@@ -87,7 +92,10 @@ proc parse(infile: string, options: PParserOptions; dllExport: var PNode): PNode | |
var p: Parser | ||
if isCpp: options.flags.incl pfCpp | ||
openParser(p, infile, stream, options) | ||
result = parseUnit(p).postprocess(pfStructStruct in options.flags) | ||
result = parseUnit(p).postprocess( | ||
structStructMode = pfStructStruct in options.flags, | ||
reorderComments = pfReorderComments in options.flags | ||
) | ||
closeParser(p) | ||
if isCpp: options.flags.excl pfCpp | ||
if options.exportPrefix.len > 0: | ||
|
@@ -100,18 +108,52 @@ proc parse(infile: string, options: PParserOptions; dllExport: var PNode): PNode | |
|
||
proc isC2nimFile(s: string): bool = splitFile(s).ext.toLowerAscii == ".c2nim" | ||
|
||
proc parseDefines(val: string): seq[ref Token] = | ||
let tpath = getTempDir() / "macro_" & getMD5(val) & ".h" | ||
let tfl = (open(tpath, fmReadWrite), tpath) | ||
let ss = llStreamOpen(val) | ||
var lex: Lexer | ||
openLexer(lex, tfl[1], ss) | ||
var tk = new Token | ||
var idx = 0 | ||
result = newSeq[ref Token]() | ||
while tk.xkind != pxEof: | ||
tk = new Token | ||
lex.getTok(tk[]) | ||
if tk.xkind == pxEof: | ||
break | ||
result.add tk | ||
inc idx | ||
if idx > 1_000: raise newException(Exception, "parse error") | ||
tfl[0].close() | ||
tfl[1].removeFile() | ||
|
||
proc parseDefineArgs(parserOptions: var PParserOptions, val: string) = | ||
let defs = val.split("=") | ||
var mc: cparser.Macro | ||
let macs = parseDefines(defs[0]) | ||
let toks = parseDefines(defs[1]) | ||
mc.name = macs[0].s | ||
mc.params = -1 | ||
mc.body = toks | ||
for m in macs[1..^1]: | ||
if m.xkind == pxParLe: mc.params = 0 | ||
if m.xkind == pxSymbol: inc mc.params | ||
parserOptions.macros.add(mc) | ||
|
||
|
||
var dummy: PNode | ||
|
||
when not compiles(renderModule(dummy, "")): | ||
# newer versions of 'renderModule' take 2 parameters. We workaround this | ||
# problem here: | ||
proc renderModule(tree: PNode; filename: string) = | ||
renderModule(tree, filename, filename) | ||
proc renderModule(tree: PNode; filename: string, renderFlags: TRenderFlags) = | ||
renderModule(tree, filename, filename, renderFlags) | ||
|
||
proc myRenderModule(tree: PNode; filename: string) = | ||
proc myRenderModule(tree: PNode; filename: string, renderFlags: TRenderFlags) = | ||
# also ensure we produced no trailing whitespace: | ||
let tmpFile = filename & ".tmp" | ||
renderModule(tree, tmpFile) | ||
renderModule(tree, tmpFile, renderFlags) | ||
|
||
let b = readFile(tmpFile) | ||
removeFile(tmpFile) | ||
|
@@ -149,21 +191,21 @@ proc main(infiles: seq[string], outfile: var string, | |
if not isC2nimFile(infile): | ||
if outfile.len == 0: outfile = changeFileExt(infile, "nim") | ||
for n in m: tree.add(n) | ||
myRenderModule(tree, outfile) | ||
myRenderModule(tree, outfile, options.renderFlags) | ||
else: | ||
for infile in infiles: | ||
let m = parse(infile, options, dllexport) | ||
if not isC2nimFile(infile): | ||
if outfile.len > 0: | ||
myRenderModule(m, outfile) | ||
myRenderModule(m, outfile, options.renderFlags) | ||
outfile = "" | ||
else: | ||
let outfile = changeFileExt(infile, "nim") | ||
myRenderModule(m, outfile) | ||
myRenderModule(m, outfile, options.renderFlags) | ||
if dllexport != nil: | ||
let (path, name, _) = infiles[0].splitFile | ||
let outfile = path / name & "_dllimpl" & ".nim" | ||
myRenderModule(dllexport, outfile) | ||
myRenderModule(dllexport, outfile, options.renderFlags) | ||
when declared(NimCompilerApiVersion): | ||
rawMessage(gConfig, hintSuccessX, [$gLinesCompiled, $(getTime() - start), | ||
formatSize(getTotalMem()), ""]) | ||
|
@@ -195,8 +237,13 @@ for kind, key, val in getopt(): | |
" use a list of files and --concat instead" | ||
of "exportdll": | ||
parserOptions.exportPrefix = val | ||
of "def": | ||
parserOptions.parseDefineArgs(val) | ||
else: | ||
if not parserOptions.setOption(key, val): | ||
if key.normalize == "render": | ||
if not parserOptions.renderFlags.setOption(val): | ||
quit("[Error] unknown option: " & key) | ||
elif not parserOptions.setOption(key, val): | ||
quit("[Error] unknown option: " & key) | ||
of cmdEnd: assert(false) | ||
if infiles.len == 0: | ||
|
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
does this really need to be an option? ie why not always do this (since it's needed to make the comments compatible with nim doc)
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.
Two reasons: 1) to make it easier to get accepted 2) because I’d have to reorder the comments in all the tests.
Though perhaps it could default to on and use the option to disable it. Plus there may be cases it’s wrong. I sorta hack the new lines with a heuristic.