On 04/24/2018 06:27 PM, David Malcolm wrote:
> On Tue, 2018-04-24 at 16:45 +0200, Martin Liška wrote:
>> Hi.
>>
>> Some time ago, I investigated quite new feature of clang which
>> is support of --autocomplete argument. That can be run from bash
>> completion
>> script and one gets more precise completion hints:
>>
>> http://blog.llvm.org/2017/09/clang-bash-better-auto-completion-is.htm
>> l
>> https://www.youtube.com/watch?v=zLPwPdZBpSY
>>
>> I like the idea and I would describe how is that better to current
>> GCC completion
>> script sitting here:
>> https://github.com/scop/bash-completion/blob/master/completions/gcc
>>
>> 1) gcc -fsanitize=^ - no support for option enum values
>> 2) gcc -fno-sa^ - no support for negative options
>> 3) gcc --param=^ - no support for param names
>>
>> These are main limitations I see. I'm attaching working prototype,
>> which you
>> can test by installed GCC, setting it on $PATH and doing:
>> $ source gcc.sh
>>
>> Then bash completion is provided via the newly added option. Some
>> examples:
>>
>> 1)
>> $ gcc -fsanitize=
>> address                    bounds                     enum           
>>             integer-divide-by-zero     nonnull-
>> attribute          pointer-
>> compare            return                     shift-
>> base                 thread                     vla-bound
>> alignment                  bounds-strict              float-cast-
>> overflow        kernel-
>> address             null                       pointer-
>> overflow           returns-nonnull-attribute  shift-
>> exponent             undefined                  vptr
>> bool                       builtin                    float-divide-
>> by-zero       leak                       object-
>> size                pointer-
>> subtract           shift                      signed-integer-
>> overflow    unreachable                
>>
>> 2)
>> $ gcc -fno-ipa-
>> -fno-ipa-bit-cp         -fno-ipa-cp-alignment   -fno-ipa-
>> icf            -fno-ipa-icf-variables  -fno-ipa-profile        -fno-
>> ipa-pure-const     -fno-ipa-reference      -fno-ipa-struct-reorg   
>> -fno-ipa-cp             -fno-ipa-cp-clone       -fno-ipa-icf-
>> functions  -fno-ipa-matrix-reorg   -fno-ipa-pta            -fno-ipa-
>> ra             -fno-ipa-sra            -fno-ipa-vrp            
>>
>> 3)
>> $ gcc --param=lto-
>> lto-max-partition  lto-min-partition  lto-partitions    
>>
>> 4)
>> gcc --param lto-
>> lto-max-partition  lto-min-partition  lto-partitions     
>>
>> The patch is quite lean and if people like, I will prepare a proper
>> patch submission. I know about some limitations that can be then
>> handled incrementally.
>>
>> Thoughts?
>> Martin
> 
> Overall, looks good (albeit with various nits).  I like how you're
> reusing the m_option_suggestions machinery from the misspelled options
> code.  There are some awkward issues e.g. arch-specific completions,
> lang-specific completions, custom option-handling etc, but given that
> as-is this patch seems to be an improvement over the status quo, I'd
> prefer to tackle those later.

I'm sending second version of the patch. I did isolation of m_option_suggestions machinery
to a separate file. Mainly due to selftests that are linked with cc1.

> 
> The patch doesn't have tests.  There would need to be some way to
> achieve test coverage for the completion code (especially as we start
> to tackle the more interesting cases).  I wonder what the best way to
> do that is; perhaps a combination of selftest and DejaGnu?  (e.g. what
> about arch-specific completions? what about the interaction with bash?
> etc)

For now I come up with quite some selftests. Integration with bash&DejaGNU
would be challenging.

> 
> A few nits:
> * Do we want to hardcode that logging path in gcc.sh?

Sure, that needs to be purged. Crucial question here is where the gcc.sh script
should live. Note that clang has it in: ./tools/clang/utils/bash-autocomplete.sh
and:

head -n1 ./tools/clang/utils/bash-autocomplete.sh
# Please add "source /path/to/bash-autocomplete.sh" to your .bashrc to use this.

Which is not ideal. I would prefer to integrate the script into:
https://github.com/scop/bash-completion/blob/master/completions/gcc

Thoughts?

> 
> * Looks like m_option_suggestions isn't needed for handling the "-
> param" case, so maybe put the param-handling case before the "Lazily
> populate m_option_suggestions" code.
> 
> * You use "l" ("ell") as a variable name in two places, which I don't
> like, as IMHO it's too close to "1" (one) in some fonts.

Fixed both notes.
Thanks for fast review.

Martin

> 
> Thanks
> Dave
>