Skip to content

Commit

Permalink
Need to bug PostgreSQL core about spi.h
Browse files Browse the repository at this point in the history
Has a bunch of unused parameters in it that need annotation to compile
without warnings.
  • Loading branch information
jasonmp85 committed Jun 12, 2015
1 parent 7df5742 commit 12c8ef2
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pg_shard.c
Expand Up @@ -16,7 +16,10 @@
#include "funcapi.h"
#include "libpq-fe.h"
#include "miscadmin.h"
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wunused-parameter"
#include "plpgsql.h"
#pragma GCC diagnostic pop

#include "pg_shard.h"
#include "connection.h"
Expand Down

4 comments on commit 12c8ef2

@anarazel
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I think unused argument warnings are a really bad idea. It can be quite reasonable to add parameters that are currently unused when writing an API. And the other way round, when removing the need for a parameter in a widely used API, it's often not required to break existing users.

@jasonmp85
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We've had so few warnings for so long that I turned on -Werror recently, so this is more than avoiding an unused argument warning. It will fail the build.

At any rate, you can still have unused arguments, they just must be annotated as such with __attribute__((unused)) or whatnot.

What would you suggest: should I turn off unused argument warnings altogether (and if so, should I also turn off unused declaration warnings)? They do have a habit of catching variables or arguments I've written during development but ended up not needing later.

@anarazel
Copy link
Contributor

@anarazel anarazel commented on 12c8ef2 Jun 15, 2015 via email

Choose a reason for hiding this comment

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

@jasonmp85
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I assume you mean -Wno-unused-parameter. I pushed a change up as #122 and will merge it shortly. I'll rebase #121 on it so I don't need the __attribute__s in that change set.

Please sign in to comment.