Page MenuHomePhabricator

[libclang] Add missing fixed point literal CursorKind to cindex.py
AcceptedPublic

Authored by vabridgers on Feb 17 2021, 2:36 PM.

Details

Summary

Experimenting with parsing AST from Python using cindex.py, I discovered
that a CursorKind defintion for fixed point literals was missing. This
change simply adds this defintion following the defintion in
clang/include/clang-c/Index.h

Diff Detail

Event Timeline

vabridgers created this revision.Feb 17 2021, 2:36 PM
vabridgers requested review of this revision.Feb 17 2021, 2:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2021, 2:36 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Add test case

vabridgers retitled this revision from Add missing fixed point literal CursorKind to cindex.py to [libclang] Add missing fixed point literal CursorKind to cindex.py.Feb 17 2021, 5:05 PM

Remove extraneous line

Polite ping. This is a small and simple change, my only doubt is the test coverage. But frankly I see none of the literals are covered in any of the test cases (unless I'm missing something). Could I get some guidance on required test coverage for this simple change, or a LGTM?

Thanks - Vince

steakhal accepted this revision.Feb 19 2021, 5:08 AM

LGTM.

Polite ping. This is a small and simple change, my only doubt is the test coverage. But frankly I see none of the literals are covered in any of the test cases (unless I'm missing something). Could I get some guidance on required test coverage for this simple change, or a LGTM?

I think if you really wish to test this, I think you should do something similar to c-index-getCursor-pp.

You could call it like c-index-getCursor-fixed-point.c:

long _Fract longfractliteral = 0.99999999lr;
unsigned _Accum uliteral1 = 2.5uk;
_Accum literal8 = -2.5k;

// RUN: c-index-test -cursor-at=%s:1:1 -ffixed-point %s | FileCheck -check-prefix=CHECK-1 %s
// CHECK-1: 1:13 VarDecl=longfractliteral:1:13 (Definition) Extent=[1:1 - 1:44] Spelling=longfractliteral ([1:13 - 1:29])
// CHECK-1-NEXT: Completion string: {ResultType long _Fract}{TypedText longfractliteral}

// RUN: c-index-test -cursor-at=%s:2:1 -ffixed-point %s | FileCheck -check-prefix=CHECK-2 %s
// CHECK-2: 2:17 VarDecl=uliteral1:2:17 (Definition) Extent=[2:1 - 2:34] Spelling=uliteral1 ([2:17 - 2:26])
// CHECK-2-NEXT: Completion string: {ResultType unsigned _Accum}{TypedText uliteral1}

// RUN: c-index-test -cursor-at=%s:3:1 -ffixed-point %s | FileCheck -check-prefix=CHECK-3 %s
// CHECK-3: 3:8 VarDecl=literal8:3:8 (Definition) Extent=[3:1 - 3:24] Spelling=literal8 ([3:8 - 3:16])
// CHECK-3-NEXT: Completion string: {ResultType _Accum}{TypedText literal8}

But I haven't used this binding stuff ever. Take my comments with a pinch of salt.

This revision is now accepted and ready to land.Feb 19 2021, 5:08 AM

You could call it like c-index-getCursor-fixed-point.c.

Hmm, to be honest, this was probably not what you wanted to test.
What about simply inserting your own use-case which uncovered this issue in the first place?