[PATCH] D17981: [clang-tidy] Fix clang-tidy to support parsing of assembly statements.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 22 09:04:11 PDT 2016
alexfh added a comment.
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.
More information about the cfe-commits