Stellen finden wo Returnwert an const-ref gebunden wird



  • Hi,
    ich hab da ne Klasse die ne value() Funktion sowie nen "operator T" hat, ala

    template <class T>
    class Foo {
    ...
    public:
         T const& value() const { return *m_ptr; }
         operator T const& () const { return *m_ptr; }
    };
    

    Beide werden an vielen vielen Stellen in einer richtig grossen Codebase verwendet.

    Jetzt muss ich die beiden Funktionen auf return by value umstellen

    template <class T>
    class Foo {
    ...
    public:
         T value() const { return *m_ptr; }
         operator T () const { return *m_ptr; }
    };
    

    Bevor ich das mache würde ich gerne Stellen finden wo die aktuelle Implementierung in dieser Art verwendet wird

    Bar const& b = foo.value(); // oder nur "= foo;"
        while (someLongRunningLoop()) {
            // Verwenden von b wobei wichtig ist dass der Wert sich ändert wenn der Wert int *m_ptr sich ändert.
        }
    

    Falls solche Stellen existieren müssten die nämlich angepasst werden. Und bei der schieren Menge an Code um die es geht möchte ich nicht einfach davon ausgehen dass es "eh keiner macht". (Und manuell inspizieren scheidet aus, dafür sind es zu viele Stellen wo die Funktion verwendet wird.)

    Weiss jemand eine Möglichkeit wie man solche Stellen finden könnte? Idealerweise durch Code-ändern und neu bauen und dann warnings oder error bekommen. Bzw. wenns ein eigenes Tool gäbe mit dem man sowas finden kann wäre ich auch interessiert.


  • Mod

    Definiere die Funktionen einfach als deleted und schau, wo es kracht?



  • Wenn zwei Methoden exakt dasselbe tun, dann ist eine davon überflüssig. In diesem Fall würde ich den Operator entfernen.

    an vielen vielen Stellen in einer richtig grossen Codebase

    Das ist bitter. Aber da muss man wohl oder übel durch. Das ist die Strafe dafür, dass man aus Bequemlichkeit das Bar const & b eingeführt hat, was nichts weiter als syntaktischer Zucker über foo.value() ist. An zuviel Zucker kann man sich eben den Magen verderben.

    Stell dein Buildsystem so ein, dass es die ganze Codebase bis zum Ende baut und nicht beim ersten Fehler aufhört. Und dann mach das hier

    Definiere die Funktionen einfach als deleted und schau, wo es kracht?

    und beiß dich durch.



  • Ich will nicht die Stellen finden wo der Conversion-Operator verwendet wird. Der ist hässlich aber kein Problem in dem Sinn dass dadurch irgendwas schief gehen würde.

    Ich will Stellen finden wo

    Bar const& b = foo.value();
    // ODER
    Bar const& b = foo;
    

    steht (weil das übel wäre), aber NICHT stellen wo

    Bar b = foo.value();
    // ODER
    Bar b = foo;
    

    steht.



  • Printe schrieb:

    Wenn zwei Methoden exakt dasselbe tun, dann ist eine davon überflüssig. In diesem Fall würde ich den Operator entfernen.

    Das weiss ich selbst, darum geht es nicht. Der Aufwand den Conversion-Operator zu entfernen wäre im Moment einfach Overkill.

    Printe schrieb:

    an vielen vielen Stellen in einer richtig grossen Codebase

    Das ist bitter. Aber da muss man wohl oder übel durch. Das ist die Strafe dafür, dass man aus Bequemlichkeit das Bar const & b eingeführt hat,

    Ich glaub du hast überhaupt nicht verstanden worum es geht. Und was bitter oder "die Strafe dafür" ist tut auch nix zur Sache. Ich hab den Code nicht geschrieben.

    Printe schrieb:

    was nichts weiter als syntaktischer Zucker über foo.value() ist. An zuviel Zucker kann man sich eben den Magen verderben.

    Was du nicht sagst 😮

    Printe schrieb:

    Stell dein Buildsystem so ein, dass es die ganze Codebase bis zum Ende baut und nicht beim ersten Fehler aufhört.

    Ja. Wenn du mir jetzt noch sagst wie das geht...
    Ich hab dazu hier im Forum glaub ich schon 2-3 mal nachgefragt -> keine Antwort.
    Das Build System ist Visual Studio 2017, Toolchain ist Visual Studio 2015. Und das bricht bei mir leider ein Projekt sofort beim 1. Fehler ab. (Die Solution baut danach weiter, aber das hilft mir kaum.)

    Printe schrieb:

    Und dann mach das hier

    Definiere die Funktionen einfach als deleted und schau, wo es kracht?

    und beiß dich durch.

    Ja, danke, ich kann selbst lesen was Arcoth geschrieben hat.
    Davon abgesehen weiss ich selbst wie man das macht. Das hilft mir bloss nix. Ich will nicht die Verwendung des Conversion-Operators finden, um den geht's nur nebenbei.
    Ich will auch nicht alle Stellen finden wo eine der beiden Funktionen aufgerufen wird. Weil das mindestens hunderte sind, vermutlich eher tausende. Und die alle manuell zu checken ob mit dem Ergebnis ne Reference initialisiert wird (=die Stellen die ich suche) oder der Wert bloss irgendwie anders verwendet wird (=die vielen vielen vielen vielen Stellen die mich nicht interessieren) ist das was ich vermeiden möchte.



  • Das Build System ist Visual Studio 2017, Toolchain ist Visual Studio 2015.

    google google ... Es scheint eine Projekteigenschaft zu geben, die "ContinueOnError" heißt. Wie du die auf true kriegst, musst du selber raustüfteln. Ich hab hier nur Linux.

    Und die alle manuell zu checken ob mit dem Ergebnis ne Reference initialisiert wird ...

    Vermutlich würde ich versuchen, diese Stellen mit ner Regex zu finden, irgendwas wie "const&BLA=BLUBB.value()". Viel Handarbeit bleibts trotzdem.

    Das ist übrigens einer der Gründe, warum dieser T-Operator große Kacke ist und dringend entfernt gehört: Kaum möglich, nach sowas im Code zu suchen

    Ich glaub du hast überhaupt nicht verstanden worum es geht.

    Vielleicht. Trotzdem gern geschehn.


  • Administrator

    Kannst du das nicht mit der Suchfunktion und etwas Regex lösen? Wird natürlich recht aufwendig, wenn du alle Sonderfälle abdecken willst. Aber ansonsten müsste es doch irgendetwas in diese Richtung sein:
    /Bar\s+const&\s+[a-zA-Z][a-zA-Z0-9]*\s*=\s*[a-zA-Z][a-zA-Z0-9]*\.value\(\)/

    Leider sind alle meine Ideen, um das über Code zu lösen/finden bisher gescheitert.



  • Müsste man das nicht über ein Plugin für clang tidy machen können?

    Und ich würde Klassen nicht Foo nennen, das sieht echt unprofessionell aus 😉



  • Mechanics schrieb:

    Müsste man das nicht über ein Plugin für clang tidy machen können?

    Und ich würde Klassen nicht Foo nennen, das sieht echt unprofessionell aus 😉

    Wie sonst? Das deutsche 08/15 geht nicht, kein Buchstabe/Unterstrich am Anfang, Operator im Bezeichner 😃

    *duckundweg*



  • @Dravere
    Das könnte ich probieren wenn ...
    - da nicht der elendige operator T const& wäre über den geschätzt 80% der Zugriffe erfolgen
    - wir nicht zig andere Funktionen in anderen Klassen hätten die value heissen

    Und natürlich wäre das nicht sicher, denn die Regex "übersieht" ja einiges (Referenz-Typ als blah::const_reference, Zuweisungen mit Zeilenumbruch drin usw.). Wobei das noch gerade "OK" wäre, 100%ige Sicherheit wäre zwar schön, aber nicht unbedingt erforderlich.

    Dravere schrieb:

    Leider sind alle meine Ideen, um das über Code zu lösen/finden bisher gescheitert.

    Meine leider auch, weswegen ich hier frag(t)e 🙂


  • Mod

    Also soweit ich verstehe, handelt es sich um Stellen, in denen das Foo<T> an eine Referenz T const& gebunden wird?

    operator T() ;
      operator T const&() volatile = delete;
    

    Es wird der zweite Operator ausgewählt, wenn wir eine Referenz binden, siehe [dcl.init.ref]/(5.1.2). Sonst wird der andere mangels volatile präferiert. Demo.

    Der obige Code ist auch sicher in dem Sinne, dass volatile Objekte trotzdem einen Fehler generieren, es gibt also eine false negatives.

    Edit: Ich denke, ähnliches kann man für value() Aufrufe anwenden, wenn value stattdessen einen Proxy zurückgibt, der wiederum den obigen Trick einsetzt.



  • Arcoth, danke!!! 🙂
    Einself!

    Ich kannte die genauen Regeln dafür nichtmal, da wäre ich im Leben nicht draufgekommen.

    🙂 🙂 🙂



  • Hachja, wäre schön gewesen, geht bloss leider nicht, weil halt auch sowas vorkommt:

    #include <string>
    
    struct A {
      operator std::string() const;
      operator std::string const&() const volatile = delete;
    };
    
    int main() {
      A a;
      std::string b1 = a;
      b1 = a; // use of deleted function operator std::string const&
    }
    

    Trotzdem nochmal danke!



  • hustbaer schrieb:

    Ich will Stellen finden wo

    Bar const& b = foo.value();
    // ODER
    Bar const& b = foo;
    

    steht (weil das übel wäre)

    Das ist eigentlich nur übel, wenn foo sehr kurzlebig ist und mit Abschluss dieser Zeile wieder zerstört wird. Denn dann hättest Du eine baumelnde Referenz b . Geht es darum, das zu vermeiden?

    Wenn Du die Methode so anpasst, dass es das Bar "by value" zurück gibt, wäre die Sache mit der Referenz b kein Problem mehr. Dafür gibt es eine Regel im Standard, die die Lebenszeit eines temporären Objekts verlängern kann. Siehe https://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/

    Vielleicht kann man generell so ein Static Analyzer anpassen, um solche Stellen zu finden. Google hat ja auch automatisiert solche Situationen

    void sink(string const&);
    void bar(string const& x) {
        sink(x.c_str());
    }
    

    gefunden, wo eine unnötige Konvertierung (string -> c_str -> string) stattfindet. Ich finde aber die Information, wie sie das genau gemacht haben, nicht mehr. Ich würde mir mal clang-tidy angucken, um zu schauen, wie einfach es ist, den so anzupassen, dass er die gewünschten Stellen für Dich findet.



  • Nein, darum geht's nicht. Worum es geht steht dagegen im Kopfbeitrag.


Log in to reply