コンパイルエラーが直せません。

Question

選択ソートの実行時間を計測するためのプログラムを書いているのですが、コンパイルエラーが直すことができません。

以下がソースコードです。

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define MAXN 1000000
int inputdata();
int selectionsort();
int printdata();
int A[MAXN],n,m;
int main(){
  inputdata();
  selectionsort(1,n);
  printdata();
}

int selectionsort(p,q)
int p,q;
{
  int i,j,a,cmin;
  clock_t start, end;
  start = clock();
  for(j = p; j <= q; j++){
    cmin=j;
    for(i=j+1; i<=q; i++)
      if(A[cmin]>A[i]) cmin=i;
    swap(j,cmin);
  }
  end = clock();
  m=end-start;
}

int inputdata(){
  int i;

  srand((unsigned)time(NULL));
  printf("出力する乱数の個数を入力===>");
  scanf("%d",&n);
  printf("\n");
  printf("-------------入力データ--------------\n");
  for(i=1; i<=n; i++){
    A[i] = rand()%10000;
    printf("A[%d]=%5d    ",i,A[i]);
    if(i%4 == 0){
      printf("\n");
    }

  }
  printf("\n");
}

int printdata(){
  int i;
  printf("\n");
  printf("--------------ソート済みデータ------------------\n");
  for(i=1; i <= n; i++){
    printf("A[%d]=%5d    ",i,A[i]);
    if(i%4 == 0){
      printf("\n");
    }
  }
  printf("\n");
  printf("ソートの処理時間は%f秒です。 \n",(double)m / CLOCKS_PER_SEC);
}

int swap(i,j)
int i,j;
{
  int temp;

  temp=A[i];
  A[i]=A[j];
  A[j]=temp;
}
コード

このソースをコンパイルすると以下のようなエラーが出てしまいます。

warning: implicit declaration of function 'swap' is invalid in C99
[-Wimplicit-function-declaration]
swap(j,cmin);
^
Selectionsort2.c:30:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Selectionsort2.c:49:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Selectionsort2.c:63:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Selectionsort2.c:73:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
5 warnings generated.

どう改善したらよいか教えてください。
どうかよろしくお願いいたします。

Answer 1

もっとも単純な対処法は

-Wimplicit-function-declaration

を取ってしまうことでしょう。

ただ、なんらかの理由があってワーニングオプションをつけていると思うので、
妥当な範囲でもっとも簡単な対処法は、 swap 関数の記述を最初に（使用している箇所より前に）持ってくることでしょう。

エラーの意味は、swap() 関数のプロトタイプ宣言がないということだと思います。

-Wimplicit-function-declaration

は、そのチェック（プロトタイプ宣言のし忘れ）をさせるためのオプションです。

もう少し手間のかかる方法としては、swap() 関数に関するすべての記述の前に以下のようなプロトタイプ宣言を書くことです。

int swap(int i,int j);

最初に書いた関数の記述を前に持ってくる方法はこの方法の一種の代替えです。
（ちょっと調べてみましたが、私が使っている gcc では引数のチェックは、この形式の宣言がないとおこなわれませんね）

外部のファイルから swap 関数を使用する場合は、ヘッダファイルなどでこの記述を共有する必要があり、関数を前に書くやり方はつかえません。

使い捨てのプログラム、またはswap() を同一ファイル内でしか使わないのであれば、関数を前に書くやり方でも構わないとは思いますが、ある程度長期的に使うツールないしライブラリであれば、きちんとプロトタイプ宣言をしたほうがいいでしょう。

あと、コンパイルオプション、特に -W系のワーニングオプションは（こういうことをチェックしたいという）意図を持って使うものです。
（違反していてもコンパイルができないわけでもなく、記述に間違いがなれば動作もする）
なので、状況によって取ってしまう（プロトタイプ宣言でのチェックなんかなくても俺は間違えない、そんなことより早く動かしたいという場合など）という選択もありえるわけです。

Comments

[sort_merge]

アドバイス本当にありがとうございました！
無事実行することができました！

Answer 2

すでにベストアンサーが決まっていますが、
私なりに全体を書き換えてみたものを示してみます。
主な変更点は、
グローバル変数をなるべく少なくする、配列の内容表示部分の重複をなくした
の２つです。

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define MAXN 100000
int inputdata();
double selectionsort();
void swap(int i, int j);
void printA(int n, char* message);

int A[MAXN];

int main(int argc, char** argv) {
  int n;
  double time;

  n = inputdata();

  printA(n, "-------------入力データ--------------");
  time = selectionsort(1, n);
  printA(n, "--------------ソート済みデータ------------------");

  printf("ソートの処理時間は%f秒です。 \n", time / CLOCKS_PER_SEC);
  return 0;
}

double selectionsort(int p, int q) {
  clock_t start, end;
  start = clock();
  for (int j = p; j <= q; j++) {
    int cmin = j;
    for (int i = j + 1; i <= q; i++) {
      if (A[cmin] > A[i]) {
        cmin = i;
      }
    }
    swap(j, cmin);
  }
  end = clock();
  return end - start;
}

int inputdata() {
  int n;
  srand((unsigned)time(NULL));
  printf("出力する乱数の個数を入力===>");
  scanf("%d", &n);
  printf("\n");
  for (int i = 1; i <= n; i++) {
    A[i] = rand() % 10000;
  }
  return n;
}

void swap(int i, int j) {
  int temp = A[i];
  A[i] = A[j];
  A[j] = temp;
}

void printA(int n, char* message) {
  printf("%s\n", message);
  for (int i = 1; i <= n; i++) {
    printf("A[%d]=%5d    ", i, A[i]);
    if (i % 5 == 0) {
      printf("\n");
    }
  }
  printf("\n");
}

実行例：

$ ./a.out
出力する乱数の個数を入力===>10

-------------入力データ--------------
A[1]= 7340    A[2]= 4195    A[3]= 5248    A[4]= 1943    A[5]= 6844    
A[6]= 1519    A[7]= 7825    A[8]= 5134    A[9]= 4426    A[10]= 1680    

--------------ソート済みデータ------------------
A[1]= 1519    A[2]= 1680    A[3]= 1943    A[4]= 4195    A[5]= 4426    
A[6]= 5134    A[7]= 5248    A[8]= 6844    A[9]= 7340    A[10]= 7825    

ソートの処理時間は0.000005秒です。 

Comments

[sort_merge]

ありがとうございます！
参考にさせていただきます！

Answer 3

こんにちは。

swap()を定義する前にselectionsort()で呼び出しているため、最初の警告が出ています。
単純にselectionsort()の定義の前にswap()関数の定義を移動すれば先頭の警告は消えます。

int swap(i,j)
int i,j;
{
  int temp;

  temp=A[i];
  A[i]=A[j];
  A[j]=temp;
}

int selectionsort(p,q)
int p,q;
{
  int i,j,a,cmin;
(以下略)

2番目以降の警告は、戻り値を指定しているのに何も返却していないことの警告です。
下記４箇所です。

int selectionsort(p,q)
int inputdata(){
int printdata(){
int swap(i,j)

上記の「int」は各関数がint型を返却する指定です。しかし、各関数はreturn文がなく、戻り値を返却していません。簡単な対処としては、全てのintをvoidへ書き換えれば警告は出なくなる筈です。

Comments

[sort_merge]

改善することができました！
本当にありがとうございました！

Answer 4

ちょっと見で気が付いた所だけですが・・
int swap(i,j)の宣言が呼び出し前に無い。
関数に戻り値が無いのでintの代わりにvoidをつけましょうｄ＾＾
int swap(i,j)
int i,j;
{
は古い書き方です。
通常void swap( int i, int j)
{

Comments

[sort_merge]

とても参考になりました！
本当にありがとうございました！

Answer 5

C言語では関数を使う前に宣言か定義がなければなりません。

int swap();

を最初の方の関数の宣言をしているところに追加してみてください。

Comments

[sort_merge]

コンパイルすることができました！
本当にありがとうございました！