Tamar Christina <tamar.christ...@arm.com> writes:
> The 09/27/2018 09:40, Richard Sandiford wrote:
>> Tamar Christina <tamar.christ...@arm.com> writes:
>> > @@ -120,22 +121,41 @@ proc check_cached_effective_target { prop args } {
>> > global et_prop_list
>> >
>> > set target [current_target_name]
>> > - if {![info exists et_cache($prop,target)]
>> > - || $et_cache($prop,target) != $target} {
>> > + if {![info exists et_cache($prop,$target)]} {
>> > verbose "check_cached_effective_target $prop: checking $target" 2
>> > - set et_cache($prop,target) $target
>> > - set et_cache($prop,value) [uplevel eval $args]
>> > + if {[string is true -strict $args] || [string is false -strict $args]} {
>> > + set et_cache($prop,$target) $args
>> > + } else {
>> > + set et_cache($prop,$target) [uplevel eval $args]
>> > + }
>>
>> Why are the checks for true and false needed? We shouldn't be
>> using this function for something that's already true or false.
>>
>
> I had wanted to be a bit lenient here in accepting it if you have a
> function that already fully evaluated the expr before passing it to
> check_cached_effective_target.
>
> i.e. something like
>
> proc check_effective_target_vect_int { } {
> return [check_cached_effective_target_indexed <name> [
> expr {
> <condition>
> }]]
> }
>
> The error you would get if you do this is very confusing so I thought
> since it didn't matter much for the regexp only target triple tests
> that just accepting this would be fine.
Seems a good thing that that's a noisy failure; the function should
make up its mind whether it wants to cache (use curly braces) or not
(just return the expr directly).
> Should I drop it or keep it?
Think we should either drop it or make it into a more user-friendly
error, e.g.:
if {[string is true -strict $args] || [string is false -strict $args]} {
error {check_cached_effective_target condition already evaluated;
did you pass [...] instead of the expected {...}?}
} else {
set et_cache($prop,$target) [uplevel eval $args]
}
Thanks,
Richard
