[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] build: Rename as-insn-check to as-insn-add

On 23/02/18 11:58, Jan Beulich wrote:
>>>> On 23.02.18 at 12:40, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 22/02/18 13:39, Jan Beulich wrote:
>>>>>> On 22.02.18 at 13:39, <george.dunlap@xxxxxxxxxx> wrote:
>>>> On 02/22/2018 12:22 PM, Jan Beulich wrote:
>>>>>>>> On 22.02.18 at 12:41, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>> On 22/02/18 11:33, Jan Beulich wrote:
>>>>>>>>>> On 22.02.18 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>>> as-insn-check mutates the passed-in flags.  Rename it to as-insn-add, 
>>>>>>>> in 
>> line
>>>>>>>> with cc-option-add.  Update all callers.
>>>>>>> I'm not convinced - cc-option-add makes relatively clear that
>>>>>>> something is being added to the options passed to CC. If I
>>>>>>> take as-insn-add this way, the macro would need to add an
>>>>>>> insn to the AS invocation. While I agree as-insn-check doesn't
>>>>>>> make clear that it adds any options, I still find this less
>>>>>>> misleading than the suggested new name. Let's see what
>>>>>>> others think.
>>>>>> I'm open to better name suggestions.
>>>>> The best I can come up with is, well, as-insn-check, as that
>>>>> reasonably describes at least part of what the construct does.
>>>>> as-insn-check-and-add-option, besides being too long, isn't
>>>>> meaningfully better.
>>>> We're definitely getting into bikeshed territory here.
>>> Indeed, but I think a change in name should be an improvement,
>>> not going from one questionable name to another questionable
>>> one.
>>>>  I agree with
>>>> Andy that 'check' doesn't really convey that something changed.  Is the
>>>> check-and-add "add it if it doesn't exist already"?  Or add it if some
>>>> other check passes / fails?
>>> It is "check if this piece of assembly assembles and add the
>>> provided option to the indicated variable", extended by Roger's
>>> patch to "..., and add the other provided option if it doesn't
>>> assemble".
>> Ok - how do we unblock this?
>> There appears to be agreement that as-insn-check isn't a great name, and
>> my proposed as-insn-add isn't much better.
>> The base runes of as-insn and cc-option are compatible.  They check the
>> fragment, and yield one of two options.  cc-option-add and as-insn-check
>> are built on top of the base runes, and mutate the flags passed in.
>> as-check-frag-update-option ?
> as-insn-option-add? Or just as-option-add, considering Roger's
> new use cases which don't check insns?

Lets go with as-option-add.  I'm happy with that.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.