This is an archive of the discontinued LLVM Phabricator instance.

Support for querying the exception specification type through libclang
ClosedPublic

Authored by ajbennieston on Jun 11 2017, 1:38 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

ajbennieston created this revision.Jun 11 2017, 1:38 PM
jbcoe added a subscriber: jbcoe.
aaron.ballman added inline comments.Jun 14 2017, 4:36 PM
include/clang-c/Index.h
185 ↗(On Diff #102137)

You can drop the trailing comment.

208 ↗(On Diff #102137)

Spurious trailing full stop.

213 ↗(On Diff #102137)

Same here.

test/Index/get-cursor.cpp
152 ↗(On Diff #102137)

The comment isn't helpful and can be removed.

tools/libclang/CXType.cpp
689 ↗(On Diff #102137)

Can elide the braces.

693 ↗(On Diff #102137)

Use const auto * and elide the braces.

695 ↗(On Diff #102137)

No else after a return; you can just lower this into the function scope and remove the else.

703 ↗(On Diff #102137)

No else after a return and can elide the braces for the if.

Fixes for review comments.

aaron.ballman added inline comments.Jun 15 2017, 2:27 PM
include/clang-c/Index.h
213 ↗(On Diff #102707)

This comment is now missing the full-stop at the end of the sentence (you dropped one too many periods).

tools/libclang/CXType.cpp
693 ↗(On Diff #102137)

You should run your patch through clang-format; the asterisk should bind to FD rather than auto and the indentation is too large in the patch.

V3 with indentation and punctuation fixes.

This revision is now accepted and ready to land.Jun 26 2017, 6:05 AM
jbcoe added a comment.Jun 26 2017, 5:03 PM

I can merge this for you Andrew.

Jon, by all means go ahead!

This revision was automatically updated to reflect the committed changes.
mgorny added a subscriber: mgorny.Jul 26 2017, 7:43 AM

This patch has introduced a test suite failure:

======================================================================
ERROR: Failure: ImportError (No module named util)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/nose/loader.py", line 420, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib64/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib64/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/test_exception_specification_kind.py", line 3, in <module>
    from .util import get_tu
ImportError: No module named util

I presume that this patch meant to include the util module but you forgot to add the file. Could you fix that, please?

cfe/trunk/bindings/python/tests/test_exception_specification_kind.py
3

It seems that the util module has never been committed.

mgorny added a comment.Sep 1 2017, 6:42 AM

Nevermind. I found out what's wrong via looking at the older patch versions.