Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typedef union not supported by public API visitor #921

Closed
jeremfg opened this issue Aug 1, 2016 · 5 comments · Fixed by #969 or #976
Closed

typedef union not supported by public API visitor #921

jeremfg opened this issue Aug 1, 2016 · 5 comments · Fixed by #969 or #976
Assignees
Labels
Milestone

Comments

@jeremfg
Copy link

jeremfg commented Aug 1, 2016

I have the following construct in C code:

/**
 * Combines together the different TEX configuration or extract the MPU_RASR values
 */
typedef union
{
    const uint8_t memOptions; /**< Allows casting of MemoryOptions into MemOptions */
    const TEXCBS texcbs;      /**< Access directly the values to be stored in the MPU_RASR register */
    const TEX0 tex0;          /**< Access meaning of a TEX0 configuration */
    const TEX1 tex1;          /**< Access meaning of a TEX1 configuration */
    const TEX2 tex2;          /**< Access meaning of a TEX2 configuration */
    const TEX3 tex3;          /**< Access meaning of a TEX3 configuration */
} MemOptions;

I get the following in my logs form sonar-runner:

14:25:59.094 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 190
14:25:59.099 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 191
14:25:59.101 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 192
14:25:59.105 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 193
14:25:59.108 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 194
14:25:59.111 ERROR - isPublicApiMember: failed to get enclosing classSpecifier for node at 195

Line 190-195 corresponds to the structure above.

Looking at class AbstractCxxPublicApiVisitor.java, line 671, it seems that a case is missing for "union".

@guwirth guwirth added this to the 0.9.6 milestone Aug 2, 2016
@guwirth guwirth self-assigned this Aug 2, 2016
@guwirth
Copy link
Collaborator

guwirth commented Aug 2, 2016

@jeremfg reason for this could be different things:

@jeremfg
Copy link
Author

jeremfg commented Aug 2, 2016

@guwirth,

@jeremfg reason for this could be different things:

I agree, this is why I did my homework before opening this issue.

I realize that perhaps I should have first introduced myself. We've been heavy users of SonarQube for Java code for many years now. We've adopted sonar-cxx a year and a half ago for our embedded software development projects written in C as it seemed to be the best sonar alternative to fit our needs. We use perhaps 75% of the features that sonar-cxx offers.
We'd be interested in collaborating/participating more in sonar-cxx as our time permits. See these issues as tiptoeing into what we can do to contribute. We're running sonar-cxx as part of our CI system over a pretty large (and growing) code base. I'm hoping that our experience can help sonar grow and improve.

Speaking of which, I've only opened two issues so far (2 that looked pretty obvious in my opinion) since I didn't want to submerge you guys :). Is there a mailing list or other places to discuss what we find?. Any guidelines I should read before we dig deeper?

By default parser of plugin is a C++ parser and not a C parser. You can try to set sonar.cxx.cFilesPatterns to suppress C++ keyword support (see https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Supported-configuration-properties)

Yes. Thankfully the default value of this configuration already covers .c files. Granted my issue occurs in a header file (.h) but this is not the issue here. This is as much valid C++ code as it would be C code. Unions haven't been removed in C++ ;-)

There are macros like TEXCBS in your code, they must be defined: https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Dealing-with-compiler-specific-code-pieces

I understand the confusion by the usage of CAPITALS in the name. But this is actually a convention we use/follow when naming symbols that represent hardware registers. There is no macros in this definition, the TEXxxx symbols are actually typedefed structures containing only primitive type members.

Maybe there are other undefined macros from your build environment you have to set?

There is no macro used within this union.

Maybe there are missing include paths you have to add? Turn debug log on to get more information: https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Get-debug-information

Everything this union uses is contained within the same file, defined above it. There is thus no external files to reference that could be missing.


As I said in my previous post, looking at class AbstractCxxPublicApiVisitor.java, it's pretty obvious to see that class, struct and enum are supported... But I see nothing to handle union.

@guwirth guwirth modified the milestones: 0.9.7, 0.9.6 Aug 2, 2016
@guwirth guwirth removed their assignment Aug 2, 2016
@guwirth
Copy link
Collaborator

guwirth commented Aug 2, 2016

Hi @jeremfg

We'd be interested in collaborating/participating more in sonar-cxx as our time permits.

You are always welcome to contribute to this plugin.

Is there a mailing list or other places to discuss what we find?.

No, there is only this issue tracker.

see that class, struct and enum are supported... But I see nothing to handle union.

Yes you are right. So this issue is a future request.

Regards,

@guwirth guwirth changed the title union not supported by public API vistor union not supported by public API visitor Aug 5, 2016
@guwirth guwirth self-assigned this Oct 9, 2016
@guwirth guwirth changed the title union not supported by public API visitor typedef union not supported by public API visitor Oct 13, 2016
@guwirth
Copy link
Collaborator

guwirth commented Oct 13, 2016

fix for

typedef union {
   /* ... */
} Sample;

@guwirth
Copy link
Collaborator

guwirth commented Oct 13, 2016

@jeremfg should be fixed. Please try and give us feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants