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

Document the RemoveUnusedDeclarations pass. #548

Merged

Conversation

cole-barefoot
Copy link
Contributor

No description provided.

@cole-barefoot cole-barefoot requested a review from mihaibudiu May 1, 2017 18:45
class RemoveAllUnusedDeclarations : public PassManager {
public:
explicit RemoveAllUnusedDeclarations(ReferenceMap* refMap, bool warn = false) {
CHECK_NULL(refMap);
std::set<const IR::Node*> *warned = nullptr;

// TODO: The @warned set is discarded. Is it necessary, or can it be replaced
Copy link
Contributor

Choose a reason for hiding this comment

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

The set is not discarded, it is used as an argument for the RemoveUnusedDeclarations below.
It is the same set for all iterations of the PassRepeated, thus it cannot be replaced with a bool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see. This avoids issuing multiple warnings for the same unreferenced extern definition, which is the only thing that can both trigger a warning and persist across multiple rounds.

I'll remove the TODO and add a comment to this effect. Thanks for pointing it out.

* - IR::Type_StructLike
*
* If @warned is non-null, unused IR::P4Table and IR::Declaration_Instance
* nodes are stored in @warned if they are unused and removed by this pass.
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition, a warning message is emitted only if the declaration was not previously in warned -- that is in fact the main point of warned -- to suppress duplicate warnings

@cole-barefoot cole-barefoot force-pushed the document_RemoveUnusedDeclarations branch from fbdefe2 to a9ca136 Compare May 1, 2017 21:43
@cole-barefoot
Copy link
Contributor Author

Thanks @mbudiu-vmw and @ChrisDodd. I updated the comments to (a) remove the TODO, which was wrong, and (b) clarify the behavior of the warning list.

@cole-barefoot cole-barefoot force-pushed the document_RemoveUnusedDeclarations branch from a9ca136 to 44a4dfe Compare May 1, 2017 22:25
…o make giveWarning() private, since it is only used inside the pass.
…O comment). Based on feedback from @mbudiu-vmw and @ChrisDodd.
@cole-barefoot cole-barefoot force-pushed the document_RemoveUnusedDeclarations branch from 44a4dfe to 1b19336 Compare May 2, 2017 00:35
@cole-barefoot cole-barefoot merged commit 98ca53b into p4lang:master May 2, 2017
@cole-barefoot cole-barefoot deleted the document_RemoveUnusedDeclarations branch May 2, 2017 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants