Make isSystemFile() part of the parser options file. #4888
Conversation
fruffy
added
the
core
fruffy
changed the title
fruffy
force-pushed
the
fruffy/parser_options_path
branch
from
75fd21c
to
250162f
Compare
frontends/p4/toP4/toP4.cpp
Outdated
| if (file.startsWith(p4includePath)) return true; | ||
| return false; | ||
| } | ||
|
|
||
| cstring ToP4::ifSystemFile(const IR::Node *node) { |
There was a problem hiding this comment.
Since you are touching it, maybe change it to return std::optional<std::string> (or std::optional<cstring>? The null-distinct-from-empty behavior of cstring is a bit anoying and bad design in my opinion. I'm not sure how big impact this would have though...
frontends/p4/toP4/toP4.h
Outdated
| @@ -72,8 72,8 @@ class ToP4 : public Inspector, ResolutionContext { | |||
| BUG_CHECK(!listTerminators.empty(), "Empty listTerminators"); | |||
| listTerminators.pop_back(); | |||
| } | |||
| bool isSystemFile(cstring file); | |||
| cstring ifSystemFile(const IR::Node *node); // return file containing node if system file | |||
| // return file containing node if system file | |||
There was a problem hiding this comment.
| // return file containing node if system file | |
| /// @returns file containing node if system file |
Signed-off-by: fruffy <[email protected]>
Signed-off-by: fruffy <[email protected]>
fruffy
force-pushed
the
fruffy/parser_options_path
branch
from
250162f
to
8eb15c1
Compare
Signed-off-by: fruffy <[email protected]>
fruffy
force-pushed
the
fruffy/parser_options_path
branch
from
8eb15c1
to
ba23563
Compare
| sourceFile != nullptr) { | ||
| auto sourceFileOpt = ifSystemFile(a); | ||
| // Errors can come from multiple files | ||
| if (!a->is<IR::Type_Error>() && sourceFileOpt.has_value()) { |
There was a problem hiding this comment.
As a way of personal preference, I usually prefer casting optional to bool to check if there is something inside :)
There was a problem hiding this comment.
Couldn't this lead to ambiguity if the type of sourceFileOpt changes? Main reason why I always try make this explicit with has_value.
There was a problem hiding this comment.
Only if type would be convertible to bool. Note that we need explicit conversion for std::optional (and this is better compared to boost::optional that is implicitly convertible to bool). Anyway, as I said, this is mostly within personal preference :)
This function is static and checks the p4IncludePath which is part of the parser options file. It should be part of this file such that it can be used in other contexts.
Among others, this call can be used by the formatter or to determine whether a particular source info objects comes from a systems file.