Re: [devel] [APT PATCH] rpmSingle{Pkg, Src}Index::ArchiveURI(): avoid cases with undefined behavior

[Ivan Zakharyaschev <imz@altlinux.org>]

On Sun, 16 Feb 2020, Dmitry V. Levin wrote:

> On Sun, Feb 16, 2020 at 04:09:14AM +0300, Ivan Zakharyaschev wrote:

> > * File.length() < 2. Since File was a non-const string,
> >   File[File.length()] might be UB before C++11. Now, File is const, and
> >   it is guaranteed that File[File.length()] == 0.
> 
> We can safely assume C++11, but I don't think we have an UB here even
> before C++11.

FWIW, after thinkng over the corresponding piece[1] which you cited (a 
third time):

[1]: https://en.cppreference.com/w/cpp/string/basic_string/operator_at

> reference       operator[]( size_type pos );(1)
> const_reference operator[]( size_type pos ) const;(2)
> 
> Returns a reference to the character at specified location pos. [...]
> 
> 1) If pos == size(), the behavior is undefined.
> 2) If pos == size(), a reference to the character with value CharT() 
> (the null character) is returned.
> 
> (until C++11)

I can see a reason to agree with you: whether the object is const might 
not be the thing that determines which variant of the method is used.

The question was whether variant (1) or (2) was used here in
(File[1] == '/') if File was not const.

(If (1) is used, it is stated that it would be UB until C++11; if (2), 
then not.)

I doubt I'm able remember all the details of how the choice is done 
between overloaded methods, but common sense gives a hint: only methods 
labelled "const" can be invoked on a const object whereas all methods 
(both labelled "const" and not) can be invoked on a non-const object.

Under this view, we see that the methods labelled "const" are equally well 
available for non-const objects. So, another factor must make the choice 
between (1) and (2).

And here the only remaining factor seems to be the expected result 
type/constness in the expression where it is used (although that might 
seem weird because of its "implicitness"): a const reference will do for 
(_ == '/'), so (2) is probably inferred under the hood as the variant to 
use here.

Of course, I might be wrong in my reasoning, and it's not quite important 
for this place in APT's code anymore anyway (as there was another reason 
to rewrite it), but I wished to share this curiosity regarding how the 
meaning of C++ code can be non-obvious to mere mortals (like me).

-- 
Best regards,
Ivan