[Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia]

Summary

Fixing the assertion due to absence of source location for implicitly defined types (using addImplicitTypedef()). During Sema checks the source location is being expected and therefore an assertion is triggered.

The change is not specific to OpenCL. But it is particular common for OpenCL types to be declared implicitly in Clang to support the mode without standard header.

Link to the bug reported: https://bugs.llvm.org//show_bug.cgi?id=25404

[ahatanak]

lib/Sema/SemaDecl.cpp

2157 ↗

(On Diff #93126)

Is it necessary to check whether New is implicit? I was just wondering when or how an implicit definition would redefine a typedef.

[bader]

Anastasia, could you generate patch with full context, please?

[Anastasia]

Updated with the full diff.

[Anastasia]

lib/Sema/SemaDecl.cpp

2157 ↗

(On Diff #93126)

I had a thought on it too, and I am not sure actually. This can happen if we implicitly define something from the standard headers. But I believe the Sema initialization should always happen before parsing the standard header or even loading them from the PCH. So I guess this shouldn't ever happen really? Perhaps, I should just remove this?

[Anastasia]

lib/Sema/SemaDecl.cpp

2157 ↗

(On Diff #93126)

Actually in case of implicit typedefs we don't seem to follow this program path at all. So I am removing this.

[Anastasia]

Removed check of implicit for new type.

[bader]

lib/Sema/SemaDecl.cpp

2157 ↗

(On Diff #93126)

So something like this will also work?

typedef float float4 __attribute((ext_vector_type(4)));
typedef float4 atomic_int;

test/SemaOpenCL/types.cl

6 ↗

(On Diff #93226)

Can we check that -Wtypedef-redefinition will emit a warning for this expression?
This typedef seems to be unnecessary since clang implicitly defines atomic_flag for OpenCL. User is not supposed to re-define it, so warning would be helpful here.

[Anastasia]

lib/Sema/SemaDecl.cpp

2157 ↗

(On Diff #93126)

An error will be given for this because the definitions are conflicting (incompatible).

test/SemaOpenCL/types.cl

6 ↗

(On Diff #93226)

A warning is given only with -Wsystem-headers in this case. I don't modify this change here. But I can add a test if necessary. Verify wouldn't work here though because a note about the old location of the typedef will be given without source information. But I can add a FileCheck test.

[bader]

I think my topics are not related to the bug 25404 and can be handled separately.
Thanks!

cfe/trunk/lib/Sema/SemaDecl.cpp

void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
   if (getLangOpts().Modules || getLangOpts().C11)
     return;
 
   // If we have a redefinition of a typedef in C, emit a warning.  This warning
   // is normally mapped to an error, but can be controlled with
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-      (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+      // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+      (Old->isImplicit() ||
+       Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
        Context.getSourceManager().isInSystemHeader(New->getLocation())))
     return;
 
   Diag(New->getLocation(), diag::ext_redefinition_of_typedef)
     << New->getDeclName();
   Diag(Old->getLocation(), diag::note_previous_definition);
 }
 

cfe/trunk/test/SemaOpenCL/types.cl

+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;