Shader_Ir: Address Feedback and clang format.

src/video_core/renderer_vulkan/vk_shader_decompiler.cpp

@@ -1646,34 +1646,34 @@ private:
 
 class ExprDecompiler {
 public:
-    ExprDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {}
+    explicit ExprDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {}
 
-    void operator()(VideoCommon::Shader::ExprAnd& expr) {
+    Id operator()(VideoCommon::Shader::ExprAnd& expr) {
         const Id type_def = decomp.GetTypeDefinition(Type::Bool);
         const Id op1 = Visit(expr.operand1);
         const Id op2 = Visit(expr.operand2);
-        current_id = decomp.Emit(decomp.OpLogicalAnd(type_def, op1, op2));
+        return decomp.Emit(decomp.OpLogicalAnd(type_def, op1, op2));
     }
 
-    void operator()(VideoCommon::Shader::ExprOr& expr) {
+    Id operator()(VideoCommon::Shader::ExprOr& expr) {
         const Id type_def = decomp.GetTypeDefinition(Type::Bool);
         const Id op1 = Visit(expr.operand1);
         const Id op2 = Visit(expr.operand2);
-        current_id = decomp.Emit(decomp.OpLogicalOr(type_def, op1, op2));
+        return decomp.Emit(decomp.OpLogicalOr(type_def, op1, op2));
     }
 
-    void operator()(VideoCommon::Shader::ExprNot& expr) {
+    Id operator()(VideoCommon::Shader::ExprNot& expr) {
         const Id type_def = decomp.GetTypeDefinition(Type::Bool);
         const Id op1 = Visit(expr.operand1);
-        current_id = decomp.Emit(decomp.OpLogicalNot(type_def, op1));
+        return decomp.Emit(decomp.OpLogicalNot(type_def, op1));
     }
 
-    void operator()(VideoCommon::Shader::ExprPredicate& expr) {
+    Id operator()(VideoCommon::Shader::ExprPredicate& expr) {
         const auto pred = static_cast<Tegra::Shader::Pred>(expr.predicate);
-        current_id = decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.predicates.at(pred)));
+        return decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.predicates.at(pred)));
     }
 
-    void operator()(VideoCommon::Shader::ExprCondCode& expr) {
+    Id operator()(VideoCommon::Shader::ExprCondCode& expr) {
         const Node cc = decomp.ir.GetConditionCode(expr.cc);
         Id target;
 
@@ -1690,35 +1690,28 @@ public:
         } else if (const auto flag = std::get_if<InternalFlagNode>(&*cc)) {
             target = decomp.internal_flags.at(static_cast<u32>(flag->GetFlag()));
         }
-        current_id = decomp.Emit(decomp.OpLoad(decomp.t_bool, target));
+        return decomp.Emit(decomp.OpLoad(decomp.t_bool, target));
     }
 
-    void operator()(VideoCommon::Shader::ExprVar& expr) {
-        current_id =
-            decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.flow_variables.at(expr.var_index)));
+    Id operator()(VideoCommon::Shader::ExprVar& expr) {
+        return decomp.Emit(decomp.OpLoad(decomp.t_bool, decomp.flow_variables.at(expr.var_index)));
     }
 
-    void operator()(VideoCommon::Shader::ExprBoolean& expr) {
-        current_id = expr.value ? decomp.v_true : decomp.v_false;
-    }
-
-    Id GetResult() {
-        return current_id;
+    Id operator()(VideoCommon::Shader::ExprBoolean& expr) {
+        return expr.value ? decomp.v_true : decomp.v_false;
     }
 
     Id Visit(VideoCommon::Shader::Expr& node) {
-        std::visit(*this, *node);
-        return current_id;
+        return std::visit(*this, *node);
     }
 
 private:
-    Id current_id;
     SPIRVDecompiler& decomp;
 };
 
 class ASTDecompiler {
 public:
-    ASTDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {}
+    explicit ASTDecompiler(SPIRVDecompiler& decomp) : decomp{decomp} {}
 
     void operator()(VideoCommon::Shader::ASTProgram& ast) {
         ASTNode current = ast.nodes.GetFirst();
@@ -1850,7 +1843,7 @@ public:
 
 private:
     SPIRVDecompiler& decomp;
-    Id current_loop_exit;
+    Id current_loop_exit{};
 };
 
 void SPIRVDecompiler::DecompileAST() {

src/video_core/shader/ast.cpp

@@ -442,8 +442,11 @@ void ASTManager::Decompile() {
     auto it = gotos.begin();
     while (it != gotos.end()) {
         const ASTNode goto_node = *it;
-        const u32 label_index = goto_node->GetGotoLabel();
-        const ASTNode label = labels[label_index];
+        const auto label_index = goto_node->GetGotoLabel();
+        if (!label_index) {
+            return;
+        }
+        const ASTNode label = labels[*label_index];
         if (!full_decompile) {
             // We only decompile backward jumps
             if (!IsBackwardsJump(goto_node, label)) {
@@ -498,8 +501,11 @@ void ASTManager::Decompile() {
             bool can_remove = true;
             ASTNode label = *it;
             for (const ASTNode goto_node : gotos) {
-                const u32 label_index = goto_node->GetGotoLabel();
-                ASTNode glabel = labels[label_index];
+                const auto label_index = goto_node->GetGotoLabel();
+                if (!label_index) {
+                    return;
+                }
+                ASTNode glabel = labels[*label_index];
                 if (glabel == label) {
                     can_remove = false;
                     break;

src/video_core/shader/ast.h

@@ -44,7 +44,7 @@ enum class ASTZipperType : u32 {
 
 class ASTZipper final {
 public:
-    ASTZipper();
+    explicit ASTZipper();
 
     void Init(ASTNode first, ASTNode parent);
 
@@ -71,74 +71,74 @@ public:
 
 class ASTProgram {
 public:
-    ASTProgram() : nodes{} {};
-    ASTZipper nodes;
+    explicit ASTProgram() = default;
+    ASTZipper nodes{};
 };
 
 class ASTIfThen {
 public:
-    ASTIfThen(Expr condition) : condition(condition), nodes{} {}
+    explicit ASTIfThen(Expr condition) : condition(condition) {}
     Expr condition;
-    ASTZipper nodes;
+    ASTZipper nodes{};
 };
 
 class ASTIfElse {
 public:
-    ASTIfElse() : nodes{} {}
-    ASTZipper nodes;
+    explicit ASTIfElse() = default;
+    ASTZipper nodes{};
 };
 
 class ASTBlockEncoded {
 public:
-    ASTBlockEncoded(u32 start, u32 end) : start{start}, end{end} {}
+    explicit ASTBlockEncoded(u32 start, u32 end) : start{start}, end{end} {}
     u32 start;
     u32 end;
 };
 
 class ASTBlockDecoded {
 public:
-    ASTBlockDecoded(NodeBlock& new_nodes) : nodes(std::move(new_nodes)) {}
+    explicit ASTBlockDecoded(NodeBlock& new_nodes) : nodes(std::move(new_nodes)) {}
     NodeBlock nodes;
 };
 
 class ASTVarSet {
 public:
-    ASTVarSet(u32 index, Expr condition) : index{index}, condition{condition} {}
+    explicit ASTVarSet(u32 index, Expr condition) : index{index}, condition{condition} {}
     u32 index;
     Expr condition;
 };
 
 class ASTLabel {
 public:
-    ASTLabel(u32 index) : index{index} {}
+    explicit ASTLabel(u32 index) : index{index} {}
     u32 index;
     bool unused{};
 };
 
 class ASTGoto {
 public:
-    ASTGoto(Expr condition, u32 label) : condition{condition}, label{label} {}
+    explicit ASTGoto(Expr condition, u32 label) : condition{condition}, label{label} {}
     Expr condition;
     u32 label;
 };
 
 class ASTDoWhile {
 public:
-    ASTDoWhile(Expr condition) : condition(condition), nodes{} {}
+    explicit ASTDoWhile(Expr condition) : condition(condition) {}
     Expr condition;
-    ASTZipper nodes;
+    ASTZipper nodes{};
 };
 
 class ASTReturn {
 public:
-    ASTReturn(Expr condition, bool kills) : condition{condition}, kills{kills} {}
+    explicit ASTReturn(Expr condition, bool kills) : condition{condition}, kills{kills} {}
     Expr condition;
     bool kills;
 };
 
 class ASTBreak {
 public:
-    ASTBreak(Expr condition) : condition{condition} {}
+    explicit ASTBreak(Expr condition) : condition{condition} {}
     Expr condition;
 };
 
@@ -177,11 +177,11 @@ public:
         return &data;
     }
 
-    ASTNode GetNext() {
+    ASTNode GetNext() const {
         return next;
     }
 
-    ASTNode GetPrevious() {
+    ASTNode GetPrevious() const {
         return previous;
     }
 
@@ -189,12 +189,12 @@ public:
         return *manager;
     }
 
-    u32 GetGotoLabel() const {
+    std::optional<u32> GetGotoLabel() const {
         auto inner = std::get_if<ASTGoto>(&data);
         if (inner) {
-            return inner->label;
+            return {inner->label};
         }
-        return -1;
+        return {};
     }
 
     Expr GetGotoCondition() const {
@@ -220,12 +220,12 @@ public:
         return true;
     }
 
-    u32 GetLabelIndex() const {
+    std::optional<u32> GetLabelIndex() const {
         auto inner = std::get_if<ASTLabel>(&data);
         if (inner) {
-            return inner->index;
+            return {inner->index};
         }
-        return -1;
+        return {};
     }
 
     Expr GetIfCondition() const {
@@ -290,7 +290,7 @@ private:
     friend class ASTZipper;
 
     ASTData data;
-    ASTNode parent;
+    ASTNode parent{};
     ASTNode next{};
     ASTNode previous{};
     ASTZipper* manager{};
@@ -327,13 +327,18 @@ public:
 
     void SanityCheck();
 
+    void Clear();
+
     bool IsFullyDecompiled() const {
         if (full_decompile) {
             return gotos.size() == 0;
         } else {
             for (ASTNode goto_node : gotos) {
-                u32 label_index = goto_node->GetGotoLabel();
-                ASTNode glabel = labels[label_index];
+                auto label_index = goto_node->GetGotoLabel();
+                if (!label_index) {
+                    return false;
+                }
+                ASTNode glabel = labels[*label_index];
                 if (IsBackwardsJump(goto_node, glabel)) {
                     return false;
                 }
@@ -346,8 +351,6 @@ public:
         return main_node;
     }
 
-    void Clear();
-
     u32 GetVariables() const {
         return variables;
     }
@@ -372,9 +375,7 @@ private:
     void MoveOutward(ASTNode goto_node);
 
     u32 NewVariable() {
-        u32 new_var = variables;
-        variables++;
-        return new_var;
+        return variables++;
     }
 
     bool full_decompile{};

src/video_core/shader/expr.h

@@ -28,7 +28,7 @@ using Expr = std::shared_ptr<ExprData>;
 
 class ExprAnd final {
 public:
-    ExprAnd(Expr a, Expr b) : operand1{a}, operand2{b} {}
+    explicit ExprAnd(Expr a, Expr b) : operand1{a}, operand2{b} {}
 
     bool operator==(const ExprAnd& b) const;
 
@@ -38,7 +38,7 @@ public:
 
 class ExprOr final {
 public:
-    ExprOr(Expr a, Expr b) : operand1{a}, operand2{b} {}
+    explicit ExprOr(Expr a, Expr b) : operand1{a}, operand2{b} {}
 
     bool operator==(const ExprOr& b) const;
 
@@ -48,7 +48,7 @@ public:
 
 class ExprNot final {
 public:
-    ExprNot(Expr a) : operand1{a} {}
+    explicit ExprNot(Expr a) : operand1{a} {}
 
     bool operator==(const ExprNot& b) const;
 
@@ -57,7 +57,7 @@ public:
 
 class ExprVar final {
 public:
-    ExprVar(u32 index) : var_index{index} {}
+    explicit ExprVar(u32 index) : var_index{index} {}
 
     bool operator==(const ExprVar& b) const {
         return var_index == b.var_index;
@@ -68,7 +68,7 @@ public:
 
 class ExprPredicate final {
 public:
-    ExprPredicate(u32 predicate) : predicate{predicate} {}
+    explicit ExprPredicate(u32 predicate) : predicate{predicate} {}
 
     bool operator==(const ExprPredicate& b) const {
         return predicate == b.predicate;
@@ -79,7 +79,7 @@ public:
 
 class ExprCondCode final {
 public:
-    ExprCondCode(ConditionCode cc) : cc{cc} {}
+    explicit ExprCondCode(ConditionCode cc) : cc{cc} {}
 
     bool operator==(const ExprCondCode& b) const {
         return cc == b.cc;
@@ -90,7 +90,7 @@ public:
 
 class ExprBoolean final {
 public:
-    ExprBoolean(bool val) : value{val} {}
+    explicit ExprBoolean(bool val) : value{val} {}
 
     bool operator==(const ExprBoolean& b) const {
         return value == b.value;