Attribute position in parsing class member function declarations

Clang Frontend
1

I have a couple of cases where I’m getting an error from Clang from code that compiles with gcc.

The code reduces to something like (members in a class declaration):

virtual void Log(const char*, …) attribute (( format(printf,1,2) )) = 0;
virtual void PushContextMessage(const char* , …) attribute (( format(printf,1,2) )) {};

Both get “expected ‘;’ at end of declaration list” errors.

The enclosed patch takes a stab at fixing it with some copy/paste programming, assuming it needs fixing, but you might have a better fix or feedback.

-John

attr_pos.patch (11 KB)

2

Oh, I’m sorry, I forgot to hit save after editing the patch. Here it is again with just the relevant changes. I also beefed up the C test a little, but it just occured to me that there probably should be an additional test file in C++, nonetheless here’s a start.

attr_pos.patch (1016 Bytes)

3

Here’s an updated version of my attempt to fix a problem with attributes coming after a declaration, but before other stuff, plus the beginnings of C++ test for it. Sorry, I don’t know attributes well, so I didn’t try to make it more exhaustive.

Could someone check this out for me, or do a more correct fix?

Thanks.

-John

attr_pos1.patch (1019 Bytes)

cxx-attributes.cpp (252 Bytes)

4

Here’s an updated version of my attempt to fix a problem with attributes coming after a declaration, but before other stuff, plus the beginnings of C++ test for it. Sorry, I don’t know attributes well, so I didn’t try to make it more exhaustive.

Could someone check this out for me, or do a more correct fix?

The patch looks fine, thanks! Any chance you’re also interested in tackling the related PR here?

http://llvm.org/bugs/show_bug.cgi?id=5605

  • Doug the Insatiable
5

Thanks.

Any chance you’re also interested in tackling the related PR here?

Sure. I thought it was the same issue, but I guess it’s a different code path. I’m working on it.

-John

6

I’m not sure this is optimal, but it seems to fix Bug 5605.

-John

attr_pos2.patch (1.12 KB)

7

I’m not sure this is optimal, but it seems to fix Bug 5605.

The parsing is fine, but this is missing some kind of diagnostic, because GCC rejects the test case:

blackthorn:~ dgregor$ gcc t.c
t.c:3: warning: ‘cf_returns_retained’ attribute directive ignored
t.c:3: error: expected ‘,’ or ‘;’ before ‘{’ token
blackthorn:~ dgregor$ g++ t.c
t.c:2: error: attributes are not allowed on a function-definition

  • Doug
8

Doug,

Sorry, I missed noticing your response.

I’m not sure what to do here. The Clang docs (http://clang-analyzer.llvm.org/annotations.html#attr_cf_returns_retained) note that as a Clang-specific attribute. Do you mean that we should put out a warning if some more strict standard mode is used, or that there should be a warning unless clang-specific attributes are explicitly enabled by some new option? Or should the test use some sort of “#if __has_feature(attribute_cf_returns_retained)” conditional? Or is the attribute not allowed after the function declation? Please instruct.

-John