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.


http://reviews.llvm.org/D17981



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to