finde den Fehler

Kellerautomat

///  Extraction operator for complex values.
  template<typename _Tp, typename _CharT, class _Traits>
    basic_istream<_CharT, _Traits>&
    operator>>(basic_istream<_CharT, _Traits>& __is, complex<_Tp>& __x)
    {
      _Tp __re_x, __im_x;
      _CharT __ch;
      __is >> __ch; // schlägt fehlt, __ch enthält irgendwas, aber nicht '('
      if (__ch == '(') // nöp
	{
            // ...
	}
      else 
	{
	  __is.putback(__ch); // irgendwas wird zurückgeworfen
	  __is >> __re_x;
	  __x = __re_x;
	}
      return __is;
    }

Kellerautomat schrieb:

Es wird nie überprüft, ob das Einlesen erfolgreich war, daher ist der Wert von __ch jedes mal undefiniert im Fehlerfall. Des Weiteren wird in diesem Fall ein Zeichen zurück in den Stream geworfen, das dort nicht herkommt (Zeile 27).

Das ist nicht so tragisch wie es aussieht.

Angenommen, das erste Einlesen läuft schief. Dann ist bei __is schon ein Failbit gesetzt.

__ch ist zufällig ungleich ')': Es wird ein zufälliges Zeichen zurückgeschoben, State wird intern auf failbit gesetzt => ok
__ch ist zufällig gleich '(': __re_x wird eingelesen, danach wird (da sich __ch noch nicht geändert hat), der State auf failbit gesetzt => ok

[quote=Standard, $26.4.6.13]Requires: The input values shall be convertible to T.
If bad input is encountered, calls is.setstate(ios_base::failbit)
[/quote]

Das einzige, was man kritisieren könnte, ist, dass nach einem clearen des States ein zufälliges Zeichen im Stream steckt. Es steht im Standard aber nichts darüber, was schon alles aus dem Stream gelesen werden sein muss.
Wenn die Eingabe z.B. "(1,bad)" ist, steht nachher ganz sicher nicht mehr das gleiche drin.

Es gibt noch einen weiteren Fall: Bei "__is >> __im_x >> __ch;" tritt ein Fehler auf; dann ist __ch aber noch "(" und der State wird auf fail gesetzt => ok.

Das von Kellerautomat kann es also nicht sein.

BTW @SeppJ:

Standard, $27.7.2.3.34 schrieb:

Behaves as an unformatted input function (as described in 27.7.2.3, paragraph 1), except that
the function first clears eofbit. After constructing a sentry object, if !good() calls setstate(failbit)
which may throw an exception, and return. If rdbuf() is not null, calls rdbuf->sputbackc(). If
rdbuf() is null, or if sputbackc() returns traits::eof(), calls setstate(badbit) (which may throw
ios_base::failure (27.5.5.4)).

Es wirft etwas zurück, aber das ist kein Problem (s.o.)

EDIT: Oh, ich hab den Fehler! Wenn rdbuf() null ist, wird setstate(badbit) gesetzt, anstatt das vorgeschriebene setstate(failbit).

SeppJ

Drücke ich mich so unklar aus? Selbst der Standard stimmt mir doch zu:

Behaves as an unformatted input function (as described in 27.7.2.3, paragraph 1), except that
the function first clears eofbit. After constructing a sentry object, if !good() calls setstate(failbit)
which may throw an exception, and return. If rdbuf() is not null, calls rdbuf->sputbackc(). If
rdbuf() is null, or if sputbackc() returns traits::eof(), calls setstate(badbit) (which may throw
ios_base::failure (27.5.5.4)).

Das heißt es passiert im Fehlerfall nichts!

SeppJ schrieb:

Drücke ich mich so unklar aus?

Nein, nicht du, der Standard. In dem Fall sehe ich den Fehler doch nicht.

Kellerautomat

Und soweit ich weiß, ist es undefiniert, ein Zeichen in einen Stream zu putback()-en, wenn man vorher noch keins rausgenommen hat...

SeppJ

Kellerautomat schrieb:

Und soweit ich weiß, ist es undefiniert, ein Zeichen in einen Stream zu putback()-en, wenn man vorher noch keins rausgenommen hat...

Man hat ja auch keines rausgenommen, schließlich ist die Aktion gescheitert. Dann scheitert auch putback, da zwischenzeitlich kein clear erfolgt.

War das Lesen jedoch erfolgreich, dann ist auch das (dann funktionierende) putback wohldefiniert.

Kellerautomat

Ok, so gut kenne ich mich mit den IO-Streams nicht aus. Dann ist der Fehler, dass der Wert von __x im Fehlerfall verändert wird, nämlich auf den undefinierten Wert von __re_x.

SeppJ

Kellerautomat schrieb:

Ok, so gut kenne ich mich mit den IO-Streams nicht aus. Dann ist der Fehler, dass der Wert von __x im Fehlerfall verändert wird, nämlich auf den undefinierten Wert von __re_x.

Auch dies ist bei den IO-Streams so vorgesehen.

Kellerautomat

Wenn das einlesen fehlschlägt, sollte das Objekt doch unverändert bleiben, oder nicht?

SeppJ

Kellerautomat schrieb:

Wenn das einlesen fehlschlägt, sollte das Objekt doch unverändert bleiben, oder nicht?

Wird oft so angenommen, ist aber im Allgemeinen falsch. Zumindest ist es nicht so bei short, int, float, double, bool & Co., die hier wohl imitiert werden sollen.

Jetzt wirst du sicherlich einwenden, dass dann auch hier nicht sicher ist, dass __ch nicht verändert wird, das wäre aber auch falsch, da für char wieder andere Regeln gelten.

Ja, die ganzen Regeln sind teilweise ziemlich arkan, man muss sich wirklich tief reindenken, um zu sehen, was der Sinn hinter den Definitionen ist. Im alten Standard war auch einiges sehr schlecht formuliert, das wurde in C++11 verbessert.

Einen Fehler sehe ich aber immer noch nicht.

camper

SeppJ schrieb:

Einen Fehler sehe ich aber immer noch nicht.

Hast du doch schon angesprochen:
Falls

__is >> __ch

fehlschlägt, hat __ch weiter einen unbestimmten Wert, die folgende Abfrage wäre also UB. I.d.R. wird der Code sicher trotzdem funktionieren, allerdings wird valgrind damit nicht glücklich.
Die Zeichextraktoren verhalten sich bei Fehler anders als die arthmetischen Extraktoren, denn im Fehlerfall gibt num_get im Zweifel 0 zurück.

SeppJ

camper schrieb:

SeppJ schrieb:

Einen Fehler sehe ich aber immer noch nicht.

Hast du doch schon angesprochen:
Falls
__is >> __ch
fehlschlägt, hat __ch weiter einen unbestimmten Wert, die folgende Abfrage wäre also UB.

Laut Standard vielleicht, der Implementierer des GCC weiß aber ganz genau (und wir ebenfalls), dass der Computer nicht wirklich die Festplatte löschen wird, sondern bloß gegen irgendeinen Müllwert vergleicht. Da er beide Pfade abgedeckt hat (entweder ist das Zeichen zufällig '(' oder eben nicht), kann also nichts passieren.

I.d.R. wird der Code sicher trotzdem funktionieren, allerdings wird valgrind damit nicht glücklich.

valgrind defineirt aber nicht, was ein Fehler ist oder nicht, sondern im Zweifelsfalle die korrekte Funktion unter allen Umständen. Diese ist gegeben, sofern man den Code nur mit dem GCC (oder einem anderen nicht-geisteskranken Compiler) übersetzt.

Die Zeichextraktoren verhalten sich bei Fehler anders als die arthmetischen Extraktoren, denn im Fehlerfall gibt num_get im Zweifel 0 zurück.

Hmm, eigentlich lese ich den Standard so, dass bei char-Typen keine Änderung erfolgt:

template<class charT, class traits>
basic_istream<charT,traits>& operator>>(basic_istream<charT,traits>& in,
charT& c);
template<class traits>
basic_istream<char,traits>& operator>>(basic_istream<char,traits>& in,
unsigned char& c);
template<class traits>
basic_istream<char,traits>& operator>>(basic_istream<char,traits>& in,
signed char& c);

Eﬀects: Behaves like a formatted input member (as described in 27.7.2.2.1) of in. After a sentry
object is constructed a character is extracted from in, if one is available, and stored in c. Otherwise,
the function calls in.setstate(failbit).

Da steht weder was von num_get, noch dass das c im Fehlerfall überhaupt angepackt wird. Bei den "formatted input membern" steht auch nur allgemeines Gelaber, num_get taucht erst speziell bei den "arithmetic extractors" auf.

camper

SeppJ schrieb:

camper schrieb:
SeppJ schrieb:

Einen Fehler sehe ich aber immer noch nicht.

Hast du doch schon angesprochen:
Falls
__is >> __ch
fehlschlägt, hat __ch weiter einen unbestimmten Wert, die folgende Abfrage wäre also UB.
Laut Standard vielleicht, der Implementierer des GCC weiß aber ganz genau (und wir ebenfalls), dass der Computer nicht wirklich die Festplatte löschen wird, sondern bloß gegen irgendeinen Müllwert vergleicht. Da er beide Pfade abgedeckt hat (entweder ist das Zeichen zufällig '(' oder eben nicht), kann also nichts passieren.

Du bist also gcc-Entwickler? Für mich sieht das erher so aus als ob der Compiler auf die Idee kommen könnte, den Extraktor zu inlinen und dann zu entscheiden, dass der Kontrollpfad, der Fehlschlag anzeigt, nie betreten werden wird, folglich eliminiert werden kann, weil ja andernfalls UB die Folge wäre.
Wenn das bis jetzt noch nicht passiert ist, dann vielleicht nur, weil noch niemand die richtige Kombination aus -O99 -funroll-everything und -finline-limit=infinite gefunden hat.

I.d.R. wird der Code sicher trotzdem funktionieren, allerdings wird valgrind damit nicht glücklich.

valgrind defineirt aber nicht, was ein Fehler ist oder nicht, sondern im Zweifelsfalle die korrekte Funktion unter allen Umständen. Diese ist gegeben, sofern man den Code nur mit dem GCC (oder einem anderen nicht-geisteskranken Compiler) übersetzt.[/quote]Das kommentiere ich jetzt mal nicht.

SeppJ schrieb:

Die Zeichextraktoren verhalten sich bei Fehler anders als die arthmetischen Extraktoren, denn im Fehlerfall gibt num_get im Zweifel 0 zurück.

Hmm, eigentlich lese ich den Standard so, dass bei char-Typen keine Änderung erfolgt:

genau.

hustbaer

Das selbe Problem gibt's dann nochmal mit __re_x im äusseren else Zweig.

camper

hustbaer schrieb:

Das selbe Problem gibt's dann nochmal mit __re_x im äusseren else Zweig.

Das ist dann aber ein arithmetischer Extraktor, der num_get aufruft, und dabei kommt dann 0 heraus.
Ich habe jetzt erst mal mit

+      if (__is.fail())
+        {
+          __x = _Tp();
+        }
+      else if (__ch == '(')
-      if (__ch == '(')

gepatcht, und das beseitigt das Problem fürs Erste.
__ch zu intialisieren dürfte auch helfen.

hustbaer

camper schrieb:

hustbaer schrieb:

Das selbe Problem gibt's dann nochmal mit __re_x im äusseren else Zweig.

Das ist dann aber ein arithmetischer Extraktor, der num_get aufruft, und dabei kommt dann 0 heraus.

ich = Brett vorm Kopf
Das mit num_get hattest du ja schon geschrieben. Ich hab's auch gelesen und mir gedacht "aha, sehr interessant, jaja *mit-dem-kopf-nicke*".
10 Sekunden später war's anscheinend wieder weg *g*.

Naja, danke für die wiederholte Erklärung

Shade Of Mine

camper schrieb:

Für mich sieht das erher so aus als ob der Compiler auf die Idee kommen könnte, den Extraktor zu inlinen und dann zu entscheiden, dass der Kontrollpfad, der Fehlschlag anzeigt, nie betreten werden wird, folglich eliminiert werden kann, weil ja andernfalls UB die Folge wäre.

Wage ich zu bezweifeln.
Der Compiler "denkt" ja nicht in UB oder nicht UB. Weil UB ist ja nur ein Standardausdruck. UB gibt es ja nicht. Es ist immer definiert was passiert.

Auch wenn man uU alle Variablen kennen muss um es vorauszusagen.

camper

Shade Of Mine schrieb:

camper schrieb:

Für mich sieht das erher so aus als ob der Compiler auf die Idee kommen könnte, den Extraktor zu inlinen und dann zu entscheiden, dass der Kontrollpfad, der Fehlschlag anzeigt, nie betreten werden wird, folglich eliminiert werden kann, weil ja andernfalls UB die Folge wäre.

Wage ich zu bezweifeln.
Der Compiler "denkt" ja nicht in UB oder nicht UB. Weil UB ist ja nur ein Standardausdruck. UB gibt es ja nicht. Es ist immer definiert was passiert.

Auch wenn man uU alle Variablen kennen muss um es vorauszusagen.

Der Compiler "denkt" in impliziten Nebenbedingungen.
In

int x; cin >> x;
if ( x * x >= x )
   cout << x;
else
   cout << "Überlauf";

"weiß" der Compiler, dass die Bedingung überflüssig ist, und wird sie eliminieren.

Und - um auf den ursprünglichen Code zurückzukommen - durch statische Analyse zu entdecken, dass in einem Ausführungspfad auf eine nicht initialisierte Variable zugegriffen wird, ist in solchen relative einfachen Fällen durchaus möglich.

Ramanujan

Ein schlechts Beispiel. Was ist wenn x hinreichend groß ist und ein Overflow entsteht?

Und die Ausgabe Overflow ist auch blödsinn, weil bei x*x so zu sagen mehrere Overflows entstehen können