This is an archive of the discontinued LLVM Phabricator instance.

[clang-doc] Create a script to generate tests
ClosedPublic

Authored by juliehockett on Jul 12 2018, 2:18 PM.

Details

Summary

Upstreaming the script I use to generate clang-doc tests (and updating the existing tests to use it)

Diff Detail

Event Timeline

juliehockett created this revision.Jul 12 2018, 2:18 PM

Nice!
Some comments.
Sorry about lack of the review, this kinda fell off my radar.

Did you meant to commit clang-tools-extra/clang-doc/test_cases/ ?
I can't tell whether it is a temporary directory or not.

clang-tools-extra/clang-doc/gen_tests.py
1 ↗(On Diff #155272)

Does it work with python3?
If yes, i wonder if it would be a good idea to hardcode python3 requirement from the start?

62 ↗(On Diff #155272)

Ah, i see, so this happens sequentially, using the same source location for each test.

72–75 ↗(On Diff #155272)

The error msg will already be dumped to screen?

116–127 ↗(On Diff #155272)

The llvm/utils/update_*_test_checks.py seem to prefer --*-binary syntax.
I wonder whether consistency is good.

131–133 ↗(On Diff #155272)

Will the script work regardless of the pwd it is called from?

138 ↗(On Diff #155272)

Probably also compile_commands.json

142 ↗(On Diff #155272)

Any plans supporting docs for C code?
Maybe you want to just drop the last filename suffix.

156 ↗(On Diff #155272)

s/chance/change/

juliehockett marked 8 inline comments as done.

Addressing comments

Did you meant to commit clang-tools-extra/clang-doc/test_cases/ ?
I can't tell whether it is a temporary directory or not.

It is, it's the code for the actual test cases that are generated (so that if anyone not me is working on this, they can easily generate the same test code, with only minor adjustments, for their changes if necessary). Maybe it would make more sense to move this to the test dir.

clang-tools-extra/clang-doc/gen_tests.py
62 ↗(On Diff #155272)

Yup, so that the resulting file has the right USR for the one that depends on the filename (n.b. that particular issue should go away after D48908, since afaik the only USRs that depend on filename are function-internals)

131–133 ↗(On Diff #155272)

Yup, __file__ is the location of this script, i.e. <path to cte>/clang-doc/gen_tests.py

138 ↗(On Diff #155272)

See note on the test_cases above, since the test_cases file only has compile_flags.txt, but I'll add it in case that changes in the future

lebedev.ri accepted this revision.Jul 13 2018, 9:27 AM

I have no further comments here.
I assume this works as intended; if not, since it is mainly a developer-only tool, further issues could be addressed later on.

Maybe wait a bit just in case someone else wants to comment, too.

This revision is now accepted and ready to land.Jul 13 2018, 9:27 AM
This revision was automatically updated to reflect the committed changes.
test/clang-doc/yaml-comment.cpp