[PATCH] D17981: [clang-tidy] Fix clang-tidy to support parsing of assembly statements.

[Etienne Bergeron via cfe-commits]

etienneb added a comment.

In http://reviews.llvm.org/D17981#380350, @alexfh wrote:

> Adding Manuel, who might have better ideas.
>
> In http://reviews.llvm.org/D17981#374904, @rnk wrote:
>
> > In http://reviews.llvm.org/D17981#374553, @etienneb wrote:
> >
> > > This is a huge difference. I didn't expect dependencies to bring so much code.
> > >  I'm not a fan of having an empty statement and increasing false positives ratio.
> > >  Would it be possible to skip whole declarations with asm-stm, and flag them as "ignored / not parsable"?
> >
> >
> > I don't actually think there are that many false positives, but I wanted to hear from Alex in case I'm wrong. I was hoping he had better ideas on how to suppress a diagnostic error in clang and run the clang-tidy checks anyway.
>
>
> I'm not familiar with the capabilities of MS-asm blocks, but if they can contain function calls, for example, we might lose valuable information, if we skip them. The possibility of a false positive depends on a specific check, it's hard to tell in general. There's also a more generic thing that can stop working properly: finding compilation errors inside asm blocks and applying fixes to these errors (if there are any fixes generated from parsing MS-asm blocks). Not sure how frequently this will happen though.
>
> > My best idea is that we make this diagnostic a default-error warning and then build with -Wno-unparseable-assembly or something. That's not a very good solution, though. =\
>
>
> Yes, not a very good solution for the reasons outlined above.
>
> > 
>
> > 
>
> > > We could gate this code under a define. I'm not a fan of define, but it seems to be a compromise for the size.
>
> > 
>
> > > 
>
> > 
>
> > > Something like: LIBTOOLING_ENABLE_INLINE_ASM_PARSER
>
> > 
>
> > > 
>
> > 
>
> > > If we decide to pursue that direction, then it should probably be for every tools.
>
> > 
>
> > 
>
> > I'd really rather not do that.
>
>
> What's your concern? If we want parsing code inside MS asm blocks like the compiler does, we'll need to pay the cost of the asm parser. If the binary size is a large problem here, can we maybe reduce the set of dependencies of the MS asm parser?
>
> In any case, I'm sure many users don't need this functionality, so it should be made compile-time configurable.


The alternative was to plug a dummy asm parser in clang-tidy, to mock the real one in the back-end.
I think it's doable, but didn't investigate it.

Or we can add a fake browser to avoid paying the cost of the full one and all the dependencies.

Both direction make sense, and I will let you choose:

- Adding the real parser and paying the cost
- Finding a way to parse and ignore any errors related to assembly statement

If we choose to add the expensive dependencies, then we need to choose to gate it or not under a "define".

Also, a point to bring here: this is applicable to every tool built with libtooling.


http://reviews.llvm.org/D17981