MS Code Analyse



  • @manni66 und @wob Vielen Dank für eure Tips, das werde ich heute Abend ausprobieren.

    MfG Torsten



  • @manni66 sagte in MS Code Analyse:

    Gewöhne dir Rule of 0 an.

    QFT!



  • Selbststudium ist halt sehr schwierig 😞

    Erstmal vielen Dank für die Tips und die Links. Ich muss mir das im Detail mal anschauen, um es endlich zu verstehen.

    Ich habe den Code nun so weit abgespeckt:

    namespace StdLibs
    {
    	class Exception
    	{
    		public:
    			Exception() = default;
    			Exception(const Exception& _exception) = default;
    			Exception(Exception&& _exception) = default;
    			Exception(const std::string& _message);
    
    			virtual ~Exception() = default;
    
    			Exception& operator=(const Exception& _exception) = default;
    			Exception& operator=(Exception&& _exception) = default;
    
    			virtual const std::string& getMessage() const noexcept;
    
    		private:
    			std::string message;
    	};
    }
    

    und

    #include "exception.hpp"
    
    namespace StdLibs
    {
    	Exception::Exception(const std::string& _message)
    		: message(_message)
    	{
    	}
    
    	const std::string& Exception::getMessage() const noexcept
    	{
    		return message;
    	}
    }
    

    Die Warnung sind alle weg. Habt ihr jetzt noch weitere Ratschläge für mich?

    Kurze Frage noch zu unit tests. Die Tests für automatisch generierte Konstruktoren und Operatoren kann ich dann getrost weglassen, richtig?

    MfG Torsten



  • Naja. Wenn Du testen möchtest dass sich alles "automatisch richtig konstruiert" können die auch Sinn machen!



  • Die Dateiendung .hpp benutzt man häufig als Indikator, dass in der entsprechenden Datei funktionaler Code steht, z.B. bei templates. Gebräuchlich für normale Headerdateien ist die Endung .h.



  • @DocShoe Danke, für deinen Tipp.

    Sorry, dass ich nochmal stören muss. Aber ich habe es offensichtlich noch immer nicht ganz verstanden. Ich habe meine Klasse Exception, welche auch als Basisklasse dienen soll, wie folgt erweitert:

    #pragma once
    
    #include <string>
    
    namespace StdLibs
    {
    	class Exception
    	{
    		protected:
    			Exception() = default;
    
    		public:
    			Exception(const Exception& _exception) = default;
    			Exception(Exception&& _exception) = default;
    			Exception(const std::string& _message);
    
    			virtual ~Exception() = default;
    
    			Exception& operator=(const Exception& _exception) = default;
    			Exception& operator=(Exception&& _exception) = default;
    
    			virtual void setDetailedDescription(const std::string& _detailedDescription);
    
    			virtual const std::string& getMessage() const noexcept;
    			virtual const std::string& getDetailedDescription() const noexcept;
    
    		private:
    			std::string message;
    			std::string detailedDescription;
    	};
    }
    

    mit

    #include "exception.hpp"
    
    namespace StdLibs
    {
    	Exception::Exception(const std::string& _message)
    		: message(_message)
    	{
    	}
    
    	void Exception::setDetailedDescription(const std::string& _detailedDescription)
    	{
    		detailedDescription = _detailedDescription;
    	}
    
    	const std::string& Exception::getMessage() const noexcept
    	{
    		return message;
    	}
    
    	const std::string& Exception::getDetailedDescription() const noexcept
    	{
    		return detailedDescription;
    	}
    }
    

    In meinem Framework möchte ich nun eine WinAPI Exception implementieren:

    #pragma once
    
    #include "../../Standard libraries/Exception/exception.hpp"
    
    namespace Framework
    {
    	namespace Exceptions
    	{
    		class WinAPI : public StdLibs::Exception
    		{
    			public:
    				WinAPI();
    				WinAPI(const WinAPI& _winAPI) = default;
    				WinAPI(WinAPI&& _winAPI) = default;
    
    			public:
    				~WinAPI() = default;
    
    			private:
    				WinAPI& operator=(const WinAPI& _winAPI) = delete;
    				WinAPI& operator=(WinAPI&& _winAPI) = delete;
    		};
    	}
    }
    

    mit

    #include "winapi.hpp"
    
    #include <Windows.h>
    
    #include <string>
    
    namespace Framework
    {
    	namespace Exceptions
    	{
    		WinAPI::WinAPI()
    			: Exception("Something goes wrong with WinAPI")
    		{
    			//const DWORD _lastErrorId = GetLastError();
    			//LPSTR _lastErrorMessage = nullptr;
    			//const size_t _size = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
    			//	nullptr, _lastErrorId, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR)&_lastErrorMessage, 0, nullptr);
    
    			//setDetailedDescription(std::string(_lastErrorMessage, _size));
    			//LocalFree(_lastErrorMessage);
    		}
    	}
    }
    

    Die auskommentierten Zeilen bitte erstmal ignorieren.

    Ich bekomme beim Aufruf des Konstruktors Exception nun auch hier die Warnung C26455 Default constructor may not throw. Declare it 'noexcept' (f.6).

    Ich gehe doch recht in der Annahme, dass der Konstruktor Exception(const std::string& _message) der Basisklasse aufgerufen wird? Dieser kann eine Exception werfen, da die Zeichenkette kopiert wird. Ist das soweit richtig? Denn wenn ich den Konstruktor WinAPI() mit noexcept deklariere, bekomme ich erneut die Warnung, ich soll das noexcept entfernen, da der std::allocator() aufgerufen wird, der ja nun mal eine Exception werfen kann?

    Was mache ich denn hier falsch?

    MfG Torsten



  • @tormen_bln

    Die Warnung bezieht sich doch auf Exception::Exception(), nicht auf Exception::Exception(const std::string& _message), welcher von WinAPI aufgerufen wird.



  • Dieser Beitrag wurde gelöscht!


  • @Jockelx Hm... komme ich jetzt nicht mit. Die Warnung wird in Zeile 11 in der Quelltextdatei winapi.cpp angezeigt.

    MfG Torsten



  • @tormen_bln

    Ja und da wird doch offensichtlich der Konstruktor von Exception(std::string) aufgerufen und nicht Exception(). Letztere kann noexcept sein.



  • Kann ich so blöd sein? Wo soll ich denn noexcept hinschreiben?



  • @tormen_bln sagte in MS Code Analyse:

    1. C26455/C26439 ist keine harte Regel, sondern bloss was MS gut & richtig findet. Kann man so sehen, muss man aber nicht. Das "may not" im Text der Warning ist also übertrieben, die korrekte Formulierung wäre eher "should not".
    2. C26455/C26439 wird u.A. bei Default-Konstruktoren erzeugt wenn diese nicht noexcept markiert sind.
    3. Die Frage wo du - im Zusammenhang mit C26455/C26439 in Zeile 11 - in deinem Code noch noexcept hinschreiben sollte kann man mit "nirgends" beantworten. Denn: Der WinAPI Default-Konstruktor kann Exceptions werfen, da er einen std::string mit nicht-leerem Inhalt initialisieren muss. Was nicht noexcept geht.

    Wenn du die Warning vermeiden willst bleiben dir folgende Möglichkeiten:

    a) Entferne den WinAPI Default-Konstruktor. D.h. er muss nen Parameter bekommen der keinen Default-Wert hat. Der naheliegende Kandidat wäre der "last error" Wert. Den Aufruf von GetLastError im WinAPI Konstruktor halte ich sowieso für schlechten Stil. Grund: wenn man GetLastError nicht direkt sofort nach dem fehlgeschlagenen Aufruf macht, kann man kaum sicherstellen dass dazwischen nicht ein weiterer WinAPI Aufruf gemacht wurde, der evtl. den "last error" Wert ändert. D.h. man kann kaum sicher stellen dass man den richtigen Fehlercode bekommt. Den User zu zwingen GetLastError selbst aufzurufen könnte man also sogar als Verbesserung ansehen (ich sehe das so).
    Wenn du unbedingt einen Convenience-Helper möchtest der GetLastError aufruft und dir ein damit initialisiertes WinAPI Exception Objekt gibt kannst du das ja leicht machen, z.B. indem du WinAPI eine statische Funktion fromLastError gibst die ein WinAPI Objekt zurückgibt und ca. so aussehen könnte:

    class WinAPI ... {
        // ...
        static WinAPI fromLastError() {
            auto const errorCode = ::GetLastError(); // Das sollte auf jeden Fall die erste Anweisung in dieser
                                                     // Funktion bleiben, sonst besteht wieder die Gefahr dass du den falschen Wert bekommst
            return WinAPI{ errorCode };
        }
        // ...
    };
    

    b) Ändere deine Exception-Klasse so, dass die Fehlermeldung erst zusammengebaut wird wenn getMessage bzw. getDetailedDescription aufgerufen werden. Dann könntest du statt einen std::string zu initialisieren im WinAPI Default-Konstruktor einfach nur den Fehlercode im Objekt ablegen (was noexcept geht, ist ja bloss ein DWORD), und FormatMessage erst in getMessage bzw. getDetailedDescription aufrufen.

    Eine weitere Beobachtung: Exceptions sollten immer noexcept-kopierbar sein. Sonst kann es passieren dass beim Fangen einer Exception wieder eine Exception erzeugt wird, was dann aua ist. Die einfachste Möglichkeit das zu erreichen, ist alles was beim Kopieren Allokationen erfordert (bzw. allgemein alles was beim Kopieren Exceptions werfen kann) in eine kleine Hilfs-struct zu packen, und in der Exception-Klasse dann nur einen shared_ptr<HelperStruct const> zu halten. Da shared_ptr noexcept zu kopieren ist löst das das Problem. Bzw. wenn gar nichts gehalten werden muss was beim Kopieren werfen könnte (so wie hier wenn du Variante (b) wählst), gibt es sowieso kein Problem.

    Noch ein letzter Punkt: sei mit noexcept sehr vorsichtig. Wenn du an eine Funktion noexcept dranschreibst, dann garantierst du damit dass aus der Funktion keine Exceptions rausfliegen. Anders als bei const oder ähnlichen Dingen hilft dir der Compiler allerdings nicht dabei dieses Versprechen zu halten, indem er dir einen Fehler um die Ohren wirft wenn du noexcept wo dazuschreibst "wo es nicht hingehört". Statt dessen wird das Programm fehlerfrei übersetzt, und wenn während der Laufzeit dann eine Exception aus einer noexcept Funktion rausfliegen würde (weil du nicht aufgepasst hast), dann wird an der Stelle statt dessen einfach dein Programm abgebrochen - std::terminate. Schwupps-weg, einfach so.
    Möglicherweise wird dir der Static Analyzer bei solchen Fehlern eine Warnung geben (-> C26447), aber verlassen sollte man sich mMn. nicht unbedingt darauf.

    ps:
    c) Markiere deinen Default-Konstruktor als noexcept(false). Das sollte den Static Analyzer von Visual Studio wissen lassen dass du dir darüber im Klaren bist dass der Konstruktor nicht noexcept ist, und die Warnung sollte nicht mehr kommen. Zumindest steht das so hier https://developercommunity.visualstudio.com/content/problem/236762/c-code-analysis-warning-c26439-this-kind-of-functi.html beschrieben.



  • Dieser Beitrag wurde gelöscht!


  • @hustbaer Vielen, vielen Dank und sorry, dass meine Antwort so spät kommt. Das ist mal eine Aussage, mit der ich etwas anfangen kann. Auch die Tipps zum schlechten Stil, danke nochmals. Ich werde das alles umbauen und bin somit diese Warnungen erstmal los. Gibt ja noch weitere, die bearbeitet werden wollen 🙂

    Auch Dank an alle anderen, die versucht haben, mir zu helfen.

    MfG Torsten


Anmelden zum Antworten