llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.23k stars 12.07k forks source link

Syntax error when compiling C example with 32-bit address space #114535

Open ararmine opened 3 weeks ago

ararmine commented 3 weeks ago

Clang issues a syntax error for the following C example example: Tried with x86-64 clang 19.1.0 version.

  1. cat example.c

    typedef int (*__ptr32 fptr1)();
    typedef int (* __ptr32 fptr5)(fptr1);
    int f5 (int (int()));
    int f5 (fptr5 fp)
    {
    return 5;
    }
  2. clang example.c -fms-extensions

    example.c:4:5: error: conflicting types for 'f5'
    int f5 (fptr5 fp)
    ^
    example.c:3:5: note: previous declaration is here
    int f5 (int (int()));
    ^
    1 error generated.

NOTE: When compiling the example as a C++ source, there is no error.

x64 MSVC v.19 gives a warning for the C example:

example.c
<source>(4): warning C4028: formal parameter 1 different from declaration

I am not sure if this a bug.

llvmbot commented 2 weeks ago

@llvm/issue-subscribers-clang-frontend

Author: None (ararmine)

Clang issues a syntax error for the following C example example: Tried with **x86-64 clang 19.1.0** version. 1. cat example.c ``` typedef int (*__ptr32 fptr1)(); typedef int (* __ptr32 fptr5)(fptr1); int f5 (int (int())); int f5 (fptr5 fp) { return 5; } ``` 2. clang example.c -fms-extensions ``` example.c:4:5: error: conflicting types for 'f5' int f5 (fptr5 fp) ^ example.c:3:5: note: previous declaration is here int f5 (int (int())); ^ 1 error generated. ``` NOTE: When compiling the example as a C++ source, there is no error. **x64 MSVC v.19** gives a warning for the C example: ``` example.c <source>(4): warning C4028: formal parameter 1 different from declaration ``` I am not sure if this a bug.
AaronBallman commented 2 weeks ago

I think Clang's behavior is correct because the types are different; MSVC is being slightly more permissive and perhaps we want to follow suit for compatibility?

CC @compnerd @rnk @majnemer @zmodem for additional opinions

zygoloid commented 2 weeks ago

More data: in C++ mode, MSVC accepts:

typedef int (*__ptr32 fptr1)();
typedef int (* __ptr32 fptr5)(fptr1);
int f5 (int (int())) { return 4; }
int f5 (fptr5 fp) { return 5; }

and treats these declarations as two different functions. In C mode, MSVC accepts:

int f5 (int p);
int f5 (void *p) { return 5; }

with only a warning. So this has nothing to do with __ptr32 -- MSVC just allows prototypes to differ between declaration and definition in C (with a warning).

ararmine commented 2 weeks ago

Clang accepts a little simplified version of the initial example, in C mode:

typedef int (* __ptr32 fptr5)();
int f5 (int());
int f5 (fptr5 fp)
{
  return 0;
}

Shouldn't Clang issue a similar error for this case?

AaronBallman commented 2 weeks ago

Clang accepts a little simplified version of the initial example, in C mode:

typedef int (* __ptr32 fptr5)();
int f5 (int());
int f5 (fptr5 fp)
{
  return 0;
}

Shouldn't Clang issue a similar error for this case?

Yes, I believe it should because the first f5 has a type of int (*)(int (*)()) and the second f5 as a type of int (*)(int (* __ptr32)()) and those are different types (at least, on a non-64-bit build). MSDN is silent about just how this qualifier works, so I could be wrong.

compnerd commented 2 weeks ago

With the goal of MSVC compatibility, I think that it makes sense to actually mimic the MSVC behaviour and simply warn on the differing prototype/definition in C mode and treat the functions as overloads in C++ mode. This type of subtle difference is something which could prevent a smooth migration from MSVC to clang even though it is meant to be a drop-in replacement.

rnk commented 1 week ago

From a code health and correctness perspective, I think Clang's behavior is good. Mismatched declaration and definition prototypes can create all sorts of hard to diagnose UB, and fixing those declarations feels like worthwhile effort during any MSVC->Clang C codebase migration.

Our guiding policy has historically been to be MSVC compatible enough to compile system headers, and this feels like it would go beyond that.

AaronBallman commented 1 week ago

Hmm, this extension is confusing to me. MSVC does not appear to make the qualifier part of the type identity; otherwise I would expect this to be a valid overload set: https://godbolt.org/z/zjWohWaq4

So if this qualifier is part of the type identity, then I think Clang's behavior is correct and more useful than MSVC's behavior. However, because this qualifier seems to be... something else... it may be more defensible to follow MSVC's behavior.

zygoloid commented 1 week ago

Hmm, this extension is confusing to me. MSVC does not appear to make the qualifier part of the type identity; otherwise I would expect this to be a valid overload set: https://godbolt.org/z/zjWohWaq4

I guess MSVC is ignoring all top-level qualifiers when determining whether an overload is the same function (not just cv-qualifiers).