Page MenuHomePhabricator

Expose NoReturn attribute to libclang
Needs ReviewPublic

Authored by harlanhaskins on Apr 17 2017, 8:48 PM.

Details

Reviewers
dblaikie
akyrtzi
milianw
skalinichev
Group Reviewers
Restricted Project
Summary

There's currently no way to get this information from libclang. This patch adds a small function to see if a given CXCursor representing a FunctionDecl is marked with __attribute__((noreturn)), [[gnu::noreturn]], or [[noreturn]] to libclang as CXCursor_NoReturnAttr.

Event Timeline

harlanhaskins created this revision.Apr 17 2017, 8:48 PM
milianw requested changes to this revision.Apr 19 2017, 12:54 AM
milianw added a subscriber: milianw.

lgtm in general, but this patch is missing a unit test

This revision now requires changes to proceed.Apr 19 2017, 12:54 AM
harlanhaskins edited edge metadata.
harlanhaskins edited the summary of this revision. (Show Details)

Add test for clang_Cursor_isFunctionNoReturn

harlanhaskins added a comment.EditedApr 20 2017, 10:00 AM

This fails to link when I compile it locally, and I have absolutely no idea why. So this is still untested.

Patch lacking changes in tools/libclang/libclang.exports

include/clang-c/Index.h
3320

I assume that you already tried something like:
r246931: Index: expose visibility attribute

But it doesn't work (no attributes in AST?), right?

unittests/libclang/LibclangTest.cpp
463 ↗(On Diff #95980)

Please add tests to test/Index/* it's easier to read/modify. (see e.g. before-mentioned commit for the reference)

harlanhaskins added inline comments.Apr 20 2017, 11:16 AM
include/clang-c/Index.h
3320

The main issue is that there are several different attributes (NoReturn, C11NoReturn, CXX11NoReturn, AnalyzerNoReturn) and I'm not sure whether to expose them all (meaning clients need to check for all of them) or roll them into one exposed attribute.

skalinichev added inline comments.Apr 21 2017, 10:40 AM
include/clang-c/Index.h
3320

One attribute should suffice I think, and you still can distinguish them if needed, e.g. via clang_getCursorSpelling. Again see how it's done in r246931

  • Change noreturn interface to expose NoReturn as an attribute
harlanhaskins retitled this revision from Add clang_Cursor_isFunctionNoReturn to libclang to Expose NoReturn attribute to libclang.Apr 21 2017, 1:44 PM
harlanhaskins edited the summary of this revision. (Show Details)
skalinichev added inline comments.
test/Index/noreturn.c
3

Please make sure to test all noreturn attributes (c11/cxx11, noretun). And verify that the test case actually passes!

tools/libclang/CXCursor.cpp
62–64

Ok, I've just tried a couple of test cases with different noreturn attributes:

  1. There are no attributes in the ast-dump for NoReturn
  2. There are attributes for CXX11NoReturn/C11NoReturn in the ast-dump

Therefore this code will only work with CXX11NoReturn/C11NoReturn attributes. And the test case you provided shouldn't work at all (as you're using NoReturn there)

So let's use clang_Cursor_isFunctionNoReturn after all, as it should cover all cases (or so I think)

harlanhaskins added inline comments.Apr 22 2017, 5:15 PM
tools/libclang/CXCursor.cpp
62–64

How should I unit test clang_Cursor_isFunctionNoReturn? I can't seem to find any tests for, say, clang_Cursor_isFunctionInlined...

skalinichev added inline comments.Apr 23 2017, 3:49 AM
tools/libclang/CXCursor.cpp
62–64

I guess something like this: https://reviews.llvm.org/D13388