lebedev.ri added a comment.
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.
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:1
+#!/usr/bin/env python
+#
----------------
Does it work with python3?
If yes, i wonder if it would be a good idea to hardcode python3 requirement
from the start?
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:62
+ test_file = os.path.join(test_cases_path, 'test.cpp')
+ shutil.copyfile(test_case_path, test_file)
+ return test_file
----------------
Ah, i see, so this happens sequentially, using the same source location for
each test.
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:72-75
+ return_code = subprocess.call(current_cmd)
+ if return_code:
+ print('Error running clang-doc on ' + os.path.basename(test_case_path))
+ return 1
----------------
The error msg will already be dumped to screen?
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:116-127
+ parser = argparse.ArgumentParser(description='Generate clang-doc tests.')
+ parser.add_argument('-flag', action='append', default=[], dest='flags',
+ help='Flags to pass to clang-doc.')
+ parser.add_argument('-prefix', type=str, default='', dest='prefix',
+ help='Prefix for this test group.')
+ parser.add_argument('-clangdoc', dest='clangdoc',
+ default='clang-doc',
----------------
The `llvm/utils/update_*_test_checks.py` seem to prefer `--*-binary` syntax.
I wonder whether consistency is good.
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:131-133
+ clang_doc_path = os.path.dirname(__file__)
+ test_cases_path = os.path.join(clang_doc_path, 'test_cases')
+ tests_path = os.path.join(clang_doc_path, '..', 'test', 'clang-doc')
----------------
Will the script work regardless of the `pwd` it is called from?
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:138
+ for test_case_path in glob.glob(os.path.join(test_cases_path, '*')):
+ if test_case_path.endswith('compile_flags.txt'):
+ continue
----------------
Probably also `compile_commands.json`
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:142
+ # Name of this test case
+ case_name = os.path.basename(test_case_path).replace('.cpp', '')
+
----------------
Any plans supporting docs for C code?
Maybe you want to just drop the last filename suffix.
================
Comment at: clang-tools-extra/clang-doc/gen_tests.py:156
+ # Make the file check the first 3 letters (there's a very small chance
+ # that this will collide, but the fix is to simply chance the decl
name)
+ usr = os.path.basename(out_file).split('.')
----------------
s/chance/change/
https://reviews.llvm.org/D49268
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
