[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG23c39f9a9e14: [clang][Interp] Diagnose unknown parameter values (authored by tbaeder). Changed prior to commit: https://reviews.llvm.org/D156509?vs=545193&id=556845#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/Interp.cpp clang/lib/AST/Interp/Interp.h clang/lib/AST/Interp/Opcodes.td clang/test/AST/Interp/c.c clang/test/AST/Interp/functions.cpp clang/test/AST/Interp/literals.cpp Index: clang/test/AST/Interp/literals.cpp === --- clang/test/AST/Interp/literals.cpp +++ clang/test/AST/Interp/literals.cpp @@ -1088,3 +1088,31 @@ int array[(intptr_t)(char*)0]; // ref-warning {{variable length array folded to constant array}} \ // expected-warning {{variable length array folded to constant array}} } + +namespace InvalidDeclRefs { + bool b00; // ref-note {{declared here}} \ +// expected-note {{declared here}} + static_assert(b00, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{read of non-const variable}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{read of non-const variable}} + + float b01; // ref-note {{declared here}} \ + // expected-note {{declared here}} + static_assert(b01, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{read of non-constexpr variable}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{read of non-constexpr variable}} + + extern const int b02; // ref-note {{declared here}} \ +// expected-note {{declared here}} + static_assert(b02, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{initializer of 'b02' is unknown}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{initializer of 'b02' is unknown}} + + /// FIXME: This should also be diagnosed in the new interpreter. + int b03 = 3; // ref-note {{declared here}} + static_assert(b03, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{read of non-const variable}} +} Index: clang/test/AST/Interp/functions.cpp === --- clang/test/AST/Interp/functions.cpp +++ clang/test/AST/Interp/functions.cpp @@ -1,5 +1,9 @@ // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -std=c++14 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify %s // RUN: %clang_cc1 -verify=ref %s +// RUN: %clang_cc1 -std=c++14 -verify=ref %s +// RUN: %clang_cc1 -std=c++20 -verify=ref %s constexpr void doNothing() {} constexpr int gimme5() { @@ -307,3 +311,24 @@ } static_assert((foo(),1) == 1, ""); } + +namespace InvalidReclRefs { + void param(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} +static_assert(true ? true : b, ""); + } + +#if __cplusplus >= 202002L + consteval void param2(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} + } +#endif +} Index: clang/test/AST/Interp/c.c === --- clang/test/AST/Interp/c.c +++ clang/test/AST/Interp/c.c @@ -3,8 +3,6 @@ // RUN: %clang_cc1 -verify=ref -std=c11 %s // RUN: %clang_cc1 -pedantic -verify=pedantic-ref -std=c11 %s -/// expected-no-diagnostics - _Static_assert(1, ""); _Static_assert(0 != 1, ""); _Static_assert(1.0 == 1.0, ""); // pedantic-ref-warning {{not an integer constant expression}} \ @@ -26,3 +24,15 @@ _Static_assert(b == 3, ""); // peda
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:550-554 + if (isa(D)) { +S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D; +S.Note(D->getLocation(), diag::note_declared_at) << D->getSourceRange(); +return false; + } aaron.ballman wrote: > No need for this given all code paths return false. > > However, what should we do for calls to this for something other than a > `ParmVarDecl`? Should we issue a generic note so the constant expression > fails but we get notified to the missing cases? I think the `return false;` is basically fine, it will show the usual "is not an integral constant expression" error message, and if there's no other note, we can investigate and improve diagnostics. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
tbaeder updated this revision to Diff 545193. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/Interp.cpp clang/lib/AST/Interp/Interp.h clang/lib/AST/Interp/Opcodes.td clang/test/AST/Interp/functions.cpp Index: clang/test/AST/Interp/functions.cpp === --- clang/test/AST/Interp/functions.cpp +++ clang/test/AST/Interp/functions.cpp @@ -1,5 +1,9 @@ // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -std=c++14 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify %s // RUN: %clang_cc1 -verify=ref %s +// RUN: %clang_cc1 -std=c++14 -verify=ref %s +// RUN: %clang_cc1 -std=c++20 -verify=ref %s constexpr void doNothing() {} constexpr int gimme5() { @@ -300,3 +304,24 @@ } static_assert((foo(),1) == 1, ""); } + +namespace InvalidReclRefs { + void param(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} +static_assert(true ? true : b, ""); + } + +#if __cplusplus >= 202002L + consteval void param2(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} + } +#endif +} Index: clang/lib/AST/Interp/Opcodes.td === --- clang/lib/AST/Interp/Opcodes.td +++ clang/lib/AST/Interp/Opcodes.td @@ -54,6 +54,7 @@ def ArgCastKind : ArgType { let Name = "CastKind"; } def ArgCallExpr : ArgType { let Name = "const CallExpr *"; } def ArgOffsetOfExpr : ArgType { let Name = "const OffsetOfExpr *"; } +def ArgDeclRef : ArgType { let Name = "const DeclRefExpr *"; } //===--===// // Classes of types instructions operate on. @@ -649,3 +650,7 @@ def InvalidCast : Opcode { let Args = [ArgCastKind]; } + +def InvalidDeclRef : Opcode { + let Args = [ArgDeclRef]; +} Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -174,6 +174,9 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result, APFloat::opStatus Status); +/// Checks why the given DeclRefExpr is invalid. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR); + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte); @@ -1897,6 +1900,12 @@ return false; } +inline bool InvalidDeclRef(InterpState &S, CodePtr OpPC, + const DeclRefExpr *DR) { + assert(DR); + return CheckDeclRef(S, OpPC, DR); +} + template ::T> inline bool OffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E) { std::vector ArrayIndices; Index: clang/lib/AST/Interp/Interp.cpp === --- clang/lib/AST/Interp/Interp.cpp +++ clang/lib/AST/Interp/Interp.cpp @@ -541,6 +541,20 @@ return true; } +/// We aleady know the given DeclRefExpr is invalid for some reason, +/// now figure out why and print appropriate diagnostics. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) { + const ValueDecl *D = DR->getDecl(); + const SourceInfo &E = S.Current->getSource(OpPC); + + if (isa(D)) { +S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D; +S.Note(D->getLocation(), diag::note_declared_at) << D->getSourceRange(); + } + + return false; +} + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte) { Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -2499,7 +2499,7 @@ return this->emitGetPtrThisField(Offset, E); } - return false; + return this->emitInvalidDeclRef(E, E); } template ___ cfe-commits mailing list
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:550-554 + if (isa(D)) { +S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D; +S.Note(D->getLocation(), diag::note_declared_at) << D->getSourceRange(); +return false; + } No need for this given all code paths return false. However, what should we do for calls to this for something other than a `ParmVarDecl`? Should we issue a generic note so the constant expression fails but we get notified to the missing cases? Comment at: clang/test/AST/Interp/functions.cpp:305-312 + void param(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} +static_assert(true ? true : b); I'd like another test along the lines of: ``` consteval void param(bool b) { static_assert(b); // Same error despite this being a consteval function where `b`'s value is always going to be known at compile time } ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
tbaeder updated this revision to Diff 545028. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156509/new/ https://reviews.llvm.org/D156509 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/Interp.cpp clang/lib/AST/Interp/Interp.h clang/lib/AST/Interp/Opcodes.td clang/test/AST/Interp/functions.cpp Index: clang/test/AST/Interp/functions.cpp === --- clang/test/AST/Interp/functions.cpp +++ clang/test/AST/Interp/functions.cpp @@ -300,3 +300,14 @@ } static_assert((foo(),1) == 1, ""); } + +namespace InvalidReclRefs { + void param(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} +static_assert(true ? true : b); + } +} Index: clang/lib/AST/Interp/Opcodes.td === --- clang/lib/AST/Interp/Opcodes.td +++ clang/lib/AST/Interp/Opcodes.td @@ -54,6 +54,7 @@ def ArgCastKind : ArgType { let Name = "CastKind"; } def ArgCallExpr : ArgType { let Name = "const CallExpr *"; } def ArgOffsetOfExpr : ArgType { let Name = "const OffsetOfExpr *"; } +def ArgDeclRef : ArgType { let Name = "const DeclRefExpr *"; } //===--===// // Classes of types instructions operate on. @@ -649,3 +650,7 @@ def InvalidCast : Opcode { let Args = [ArgCastKind]; } + +def InvalidDeclRef : Opcode { + let Args = [ArgDeclRef]; +} Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -174,6 +174,9 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result, APFloat::opStatus Status); +/// Checks why the given DeclRefExpr is invalid. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR); + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte); @@ -1897,6 +1900,12 @@ return false; } +inline bool InvalidDeclRef(InterpState &S, CodePtr OpPC, + const DeclRefExpr *DR) { + assert(DR); + return CheckDeclRef(S, OpPC, DR); +} + template ::T> inline bool OffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E) { std::vector ArrayIndices; Index: clang/lib/AST/Interp/Interp.cpp === --- clang/lib/AST/Interp/Interp.cpp +++ clang/lib/AST/Interp/Interp.cpp @@ -541,6 +541,21 @@ return true; } +/// We aleady know the given DeclRefExpr is invalid for some reason, +/// now figure out why and print appropriate diagnostics. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) { + const ValueDecl *D = DR->getDecl(); + const SourceInfo &E = S.Current->getSource(OpPC); + + if (isa(D)) { +S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D; +S.Note(D->getLocation(), diag::note_declared_at) << D->getSourceRange(); +return false; + } + + return false; +} + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte) { Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -2501,7 +2501,7 @@ return this->emitGetPtrThisField(Offset, E); } - return false; + return this->emitInvalidDeclRef(E, E); } template ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156509: [clang][Interp] Diagnose unknown parameter values
tbaeder created this revision. tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D156509 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/Interp.cpp clang/lib/AST/Interp/Interp.h clang/lib/AST/Interp/Opcodes.td clang/test/AST/Interp/functions.cpp Index: clang/test/AST/Interp/functions.cpp === --- clang/test/AST/Interp/functions.cpp +++ clang/test/AST/Interp/functions.cpp @@ -300,3 +300,14 @@ } static_assert((foo(),1) == 1, ""); } + +namespace InvalidReclRefs { + void param(bool b) { // ref-note {{declared here}} \ + // expected-note {{declared here}} +static_assert(b); // ref-error {{not an integral constant expression}} \ + // ref-note {{function parameter 'b' with unknown value}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{function parameter 'b' with unknown value}} +static_assert(true ? true : b); + } +} Index: clang/lib/AST/Interp/Opcodes.td === --- clang/lib/AST/Interp/Opcodes.td +++ clang/lib/AST/Interp/Opcodes.td @@ -54,6 +54,7 @@ def ArgCastKind : ArgType { let Name = "CastKind"; } def ArgCallExpr : ArgType { let Name = "const CallExpr *"; } def ArgOffsetOfExpr : ArgType { let Name = "const OffsetOfExpr *"; } +def ArgDeclRef : ArgType { let Name = "const DeclRefExpr *"; } //===--===// // Classes of types instructions operate on. @@ -649,3 +650,7 @@ def InvalidCast : Opcode { let Args = [ArgCastKind]; } + +def InvalidDeclRef : Opcode { + let Args = [ArgDeclRef]; +} Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -174,6 +174,9 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result, APFloat::opStatus Status); +/// Checks why the given DeclRefExpr is invalid. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR); + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte); @@ -1897,6 +1900,12 @@ return false; } +inline bool InvalidDeclRef(InterpState &S, CodePtr OpPC, + const DeclRefExpr *DR) { + assert(DR); + return CheckDeclRef(S, OpPC, DR); +} + template ::T> inline bool OffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E) { std::vector ArrayIndices; Index: clang/lib/AST/Interp/Interp.cpp === --- clang/lib/AST/Interp/Interp.cpp +++ clang/lib/AST/Interp/Interp.cpp @@ -541,6 +541,20 @@ return true; } +/// We aleady know the given DeclRefExpr is invalid for some reason, +/// now figure out why and print appropriate diagnostics. +bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) { + const ValueDecl *D = DR->getDecl(); + const SourceInfo &E = S.Current->getSource(OpPC); + + if (isa(D)) { +S.FFDiag(E, diag::note_constexpr_function_param_value_unknown) << D; +S.Note(D->getLocation(), diag::note_declared_at); + } + + return false; +} + bool CheckBitcast(InterpState &S, CodePtr OpPC, unsigned IndeterminateBits, bool TargetIsUCharOrByte) { Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -2501,7 +2501,7 @@ return this->emitGetPtrThisField(Offset, E); } - return false; + return this->emitInvalidDeclRef(E, E); } template ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits