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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
