Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not all returns in parsers are disallowed #3309

Merged
merged 6 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backends/bmv2/common/controlFlowGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class CFG final : public IHasDbPrint {
bool checkSame(const EdgeSet& other) const;
/// Check if this destination appears in this edgeset.
/// Importantly, a TableNode is a destination if it points to
/// the same table as an existin destination (pointer equality
/// the same table as an existing destination (pointer equality
/// is not enough).
bool isDestination(const CFG::Node* destination) const;
};
Expand Down
7 changes: 7 additions & 0 deletions frontends/p4/methodInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
#include "methodInstance.h"
#include "ir/ir.h"
#include "frontends/p4/typeChecking/typeChecker.h"
#include "frontends/p4/evaluator/substituteParameters.h"

namespace P4 {

Expand Down Expand Up @@ -124,6 +125,12 @@ MethodInstance::resolve(const IR::MethodCallExpression* mce, DeclarationLookup*
return nullptr; // unreachable
}

const IR::P4Action* ActionCall::specialize(ReferenceMap* refMap) const {
SubstituteParameters sp(refMap, &substitution, new TypeVariableSubstitution());
auto result = action->apply(sp);
return result->to<IR::P4Action>();
}

ConstructorCall*
ConstructorCall::resolve(const IR::ConstructorCallExpression* cce,
DeclarationLookup* refMap, TypeMap* typeMap) {
Expand Down
3 changes: 3 additions & 0 deletions frontends/p4/methodInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ class ActionCall final : public MethodInstance {
friend class MethodInstance;
public:
const IR::P4Action* action;
/// Generate a version of the action where the parameters in the
/// substitution have been replaced with the arguments.
const IR::P4Action* specialize(ReferenceMap* refMap) const;
};

/**
Expand Down
11 changes: 7 additions & 4 deletions frontends/p4/validateParsedProgram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,13 @@ void ValidateParsedProgram::postorder(const IR::SwitchStatement* statement) {

/// Return statements are not allowed in parsers
void ValidateParsedProgram::postorder(const IR::ReturnStatement* statement) {
auto inParser = findContext<IR::P4Parser>();
if (inParser != nullptr)
::error(ErrorType::ERR_INVALID,
"%1%: invalid statement. 'return' statements not allowed in parsers.", statement);
if (!findContext<IR::Function>()) {
auto inParser = findContext<IR::P4Parser>();
if (inParser != nullptr)
::error(ErrorType::ERR_INVALID,
"%1%: invalid statement. 'return' statements not allowed in parsers.",
statement);
}
}

/// Exit statements are not allowed in parsers or functions
Expand Down
2 changes: 1 addition & 1 deletion ir/expression.def
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class BoolLiteral : Literal {
class StringLiteral : Literal {
cstring value;
validate{ if (value.isNull()) BUG("null StringLiteral"); }
toString{ return cstring("\"") + value + "\""; }
toString{ return cstring("\"") + value.escapeJson() + "\""; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a one-off fix or is it needed elsewhere in the .def files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we handle string literals anywhere else.

StringLiteral(ID v) : Literal(v.srcInfo), value(v.name) {}
#emit
operator IR::ID() const { return IR::ID(srcInfo, value); }
Expand Down
6 changes: 6 additions & 0 deletions lib/cstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,9 @@ cstring cstring::toUpper() {
return ret;
}

cstring cstring::capitalize() {
std::string st = str;
st[0] = ::toupper(st[0]);
cstring ret = cstring::to_cstring(st);
return ret;
}
2 changes: 2 additions & 0 deletions lib/cstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ class cstring {

// convert the cstring to upper case
cstring toUpper();
// capitalize the first symbol
cstring capitalize();
};

inline bool operator==(const char *a, cstring b) { return b == a; }
Expand Down
10 changes: 10 additions & 0 deletions test/gtest/cstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ TEST(cstring, construct) {
EXPECT_FALSE(c.isNull());
}

TEST(cstring, toupper) {
cstring c = "simple";
cstring c1= "";

EXPECT_EQ(c.toUpper(), "SIMPLE");
EXPECT_EQ(c1.toUpper(), "");
EXPECT_EQ(c.capitalize(), "Simple");
EXPECT_EQ(c1.capitalize(), "");
}

TEST(cstring, compare) {
cstring c = "simple";
cstring c1 = "";
Expand Down
15 changes: 15 additions & 0 deletions testdata/p4_16_samples/issue3307.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
typedef bit t;
extern e {
e();
abstract t f(in t a);
}
parser MyParser(t tt) {
e() ee = {
t f(in t a) {
return a;
}
};
state start {
transition accept;
}
}
17 changes: 17 additions & 0 deletions testdata/p4_16_samples_outputs/issue3307-first.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
typedef bit<1> t;
extern e {
e();
abstract t f(in t a);
}

parser MyParser(t tt) {
e() ee = {
t f(in t a) {
return a;
}
};
state start {
transition accept;
}
}

6 changes: 6 additions & 0 deletions testdata/p4_16_samples_outputs/issue3307-frontend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
typedef bit<1> t;
extern e {
e();
abstract t f(in t a);
}

17 changes: 17 additions & 0 deletions testdata/p4_16_samples_outputs/issue3307.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
typedef bit<1> t;
extern e {
e();
abstract t f(in t a);
}

parser MyParser(t tt) {
e() ee = {
t f(in t a) {
return a;
}
};
state start {
transition accept;
}
}

1 change: 1 addition & 0 deletions testdata/p4_16_samples_outputs/issue3307.p4-stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[--Wwarn=missing] warning: Program does not contain a `main' module
5 changes: 2 additions & 3 deletions testdata/p4_16_samples_outputs/pragma-string-first.p4
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@name("original") const bit<1> b = 1w1;
@name("string \" with \" quotes") const bit<1> c = 1w1;
@name("string with
newline") const bit<1> d = 1w1;
@name("string \\\" with \\\" quotes") const bit<1> c = 1w1;
@name("string with\nnewline") const bit<1> d = 1w1;
@name("string with quoted newline") const bit<1> e = 1w1;
@name("8-bit string ⟶") const bit<1> f = 1w1;
5 changes: 2 additions & 3 deletions testdata/p4_16_samples_outputs/pragma-string.p4
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@name("original") const bit<1> b = 1;
@name("string \" with \" quotes") const bit<1> c = 1;
@name("string with
newline") const bit<1> d = 1;
@name("string \\\" with \\\" quotes") const bit<1> c = 1;
@name("string with\nnewline") const bit<1> d = 1;
@name("string with quoted newline") const bit<1> e = 1;
@name("8-bit string ⟶") const bit<1> f = 1;
5 changes: 1 addition & 4 deletions testdata/p4_16_samples_outputs/specIssue884-first.p4
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ struct meta_t {
control Ingress(inout headers_t hdr, inout meta_t meta) {
Hash<bit<32>>(algo = HashAlgorithm_t.CRC32) hash1;
Hash<bit<32>>(poly = CRCPolynomial<bit<16>>(coeff = 16w0x107)) hash2;
LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100
0100011
0010110
0001001") hamming;
LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100\n 0100011\n 0010110\n 0001001") hamming;
Hash<bit<32>>(matrix = hamming) hash3;
Hash<bit<32>>(formula = "magic_formula()") hash4;
apply {
Expand Down
5 changes: 1 addition & 4 deletions testdata/p4_16_samples_outputs/specIssue884-frontend.p4
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ control Ingress(inout headers_t hdr, inout meta_t meta) {
@name("Ingress.hash1") Hash<bit<32>>(algo = HashAlgorithm_t.CRC32) hash1_0;
@name("Ingress.tmp") CRCPolynomial<bit<16>>(coeff = 16w0x107) tmp;
@name("Ingress.hash2") Hash<bit<32>>(poly = tmp) hash2_0;
@name("Ingress.hamming") LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100
0100011
0010110
0001001") hamming_0;
@name("Ingress.hamming") LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100\n 0100011\n 0010110\n 0001001") hamming_0;
@name("Ingress.hash3") Hash<bit<32>>(matrix = hamming_0) hash3_0;
@name("Ingress.hash4") Hash<bit<32>>(formula = "magic_formula()") hash4_0;
apply {
Expand Down
5 changes: 1 addition & 4 deletions testdata/p4_16_samples_outputs/specIssue884-midend.p4
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ control Ingress(inout headers_t hdr, inout meta_t meta) {
@name("Ingress.hash1") Hash<bit<32>>(algo = HashAlgorithm_t.CRC32) hash1_0;
@name("Ingress.tmp") CRCPolynomial<bit<16>>(coeff = 16w0x107) tmp;
@name("Ingress.hash2") Hash<bit<32>>(poly = tmp) hash2_0;
@name("Ingress.hamming") LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100
0100011
0010110
0001001") hamming_0;
@name("Ingress.hamming") LCGMatrix<bit<8>, bit<10>>(8w4, 10w7, "1000100\n 0100011\n 0010110\n 0001001") hamming_0;
@name("Ingress.hash3") Hash<bit<32>>(matrix = hamming_0) hash3_0;
@name("Ingress.hash4") Hash<bit<32>>(formula = "magic_formula()") hash4_0;
apply {
Expand Down
5 changes: 1 addition & 4 deletions testdata/p4_16_samples_outputs/specIssue884.p4
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ struct meta_t {
control Ingress(inout headers_t hdr, inout meta_t meta) {
Hash<bit<32>>(algo = HashAlgorithm_t.CRC32) hash1;
Hash<bit<32>>(poly = CRCPolynomial(coeff = 16w0x107)) hash2;
LCGMatrix(8w4, 10w7, "1000100
0100011
0010110
0001001") hamming;
LCGMatrix(8w4, 10w7, "1000100\n 0100011\n 0010110\n 0001001") hamming;
Hash<bit<32>>(matrix = hamming) hash3;
Hash<bit<32>>(formula = "magic_formula()") hash4;
apply {
Expand Down
5 changes: 2 additions & 3 deletions testdata/p4_16_samples_outputs/string_anno-first.p4
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@name("original") const bit<1> b = 1w1;
@name("string \" with \" quotes") const bit<1> c = 1w1;
@name("string with
newline") const bit<1> d = 1w1;
@name("string \\\" with \\\" quotes") const bit<1> c = 1w1;
@name("string with\nnewline") const bit<1> d = 1w1;
@name("string with quoted newline") const bit<1> e = 1w1;
@name("8-bit string ⟶") const bit<1> f = 1w1;
5 changes: 2 additions & 3 deletions testdata/p4_16_samples_outputs/string_anno.p4
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@name("original") const bit<1> b = 1;
@name("string \" with \" quotes") const bit<1> c = 1;
@name("string with
newline") const bit<1> d = 1;
@name("string \\\" with \\\" quotes") const bit<1> c = 1;
@name("string with\nnewline") const bit<1> d = 1;
@name("string with quoted newline") const bit<1> e = 1;
@name("8-bit string ⟶") const bit<1> f = 1;