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.

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)

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

* 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.

Thanks
Dave

Reply via email to