[cfe-commits] r158085 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/SourceManager.h include/clang/Parse/Parser.h lib/Basic/SourceManager.cpp lib/Lex/Preprocessor.cpp lib/Parse/ParseAST.cpp lib/Parse/Parser.cpp t
Meador Inge
meadori at gmail.com
Wed Jun 6 18:49:36 PDT 2012
On Wed, Jun 6, 2012 at 12:25 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> Author: jrose
> Date: Wed Jun 6 12:25:21 2012
> New Revision: 158085
>
> URL: http://llvm.org/viewvc/llvm-project?rev=158085&view=rev
> Log:
> Add pedantic warning -Wempty-translation-unit (C11 6.9p1).
>
> In standard C since C89, a 'translation-unit' is syntactically defined to have
> at least one "external-declaration", which is either a decl or a function
> definition. In Clang the latter gives us a declaration as well.
>
> The tricky bit about this warning is that our predefines can contain external
> declarations (__builtin_va_list and the 128-bit integer types). Therefore our
> AST parser now makes sure we have at least one declaration that doesn't come
> from the predefines buffer.
>
> Also, remove bogus warning about empty source files. This doesn't catch source
> files that only contain comments, and never fired anyway because of our
> predefines.
>
> PR12665 and <rdar://problem/9165548>
I agree with the decision to track the pre-defined decls as a
short-term solution. Longer term I still think injecting strings into
the preprocessor input like this is still gross because: (1)
'InitializePredefinedMacros' is for initializing, uh, macros and
__builtin_va_list is not a macro, (2) there isn't a clean separation
between all of the builtin types and the builtin preprocessor input,
(3) it causes issue like PR 12665 that we have to work around by doing
extra unnecessary tracking, and (4) there is special casing in
'Sema::ActOnTypedefNameDecl' to notify the AST context of the types
that can be removed.
Attached is a work in progress patch where I move the
__builtin_va_list type construction to the TargetInfo classes.
However, there are some drawbacks to this approach at this point in
time: (1) it is a breaking change for the AST serialization format and
(2) libclangBasic now depends on libclangAST thus introducing a cyclic
dependency.
Anyway, I am glad PR 12665 is fixed and I just wanted to share the
__builtin_va_list type construction work I have been playing with.
> Added:
> cfe/trunk/test/PCH/empty-with-headers.c
> cfe/trunk/test/Parser/completely-empty-header-file.h
> cfe/trunk/test/Parser/empty-translation-unit.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> cfe/trunk/include/clang/Basic/SourceManager.h
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/lib/Basic/SourceManager.cpp
> cfe/trunk/lib/Lex/Preprocessor.cpp
> cfe/trunk/lib/Parse/ParseAST.cpp
> cfe/trunk/lib/Parse/Parser.cpp
> cfe/trunk/test/Misc/warning-flags.c
> cfe/trunk/test/Parser/opencl-pragma.cl
> cfe/trunk/test/Preprocessor/undef-error.c
> cfe/trunk/test/Sema/c89-2.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Jun 6 12:25:21 2012
> @@ -20,7 +20,9 @@
>
> let CategoryName = "Parse Issue" in {
>
> -def ext_empty_source_file : Extension<"ISO C forbids an empty source file">;
> +def ext_empty_translation_unit : Extension<
> + "ISO C requires a translation unit to contain at least one declaration.">,
> + InGroup<DiagGroup<"empty-translation-unit">>;
> def warn_cxx98_compat_top_level_semi : Warning<
> "extra ';' outside of a function is incompatible with C++98">,
> InGroup<CXX98CompatPedantic>, DefaultIgnore;
>
> Modified: cfe/trunk/include/clang/Basic/SourceManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/SourceManager.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManager.h Wed Jun 6 12:25:21 2012
> @@ -584,6 +584,9 @@
> /// \brief The file ID for the precompiled preamble there is one.
> FileID PreambleFileID;
>
> + /// \brief The file ID for the preprocessor's predefines.
> + FileID PredefinesFileID;
> +
> // Statistics for -print-stats.
> mutable unsigned NumLinearScans, NumBinaryProbes;
>
> @@ -628,6 +631,14 @@
> MainFileID = createFileIDForMemBuffer(Buffer);
> return MainFileID;
> }
> +
> + /// \brief Create the FileID for a memory buffer that contains the
> + /// preprocessor's predefines.
> + FileID createPredefinesFileIDForMemBuffer(const llvm::MemoryBuffer *Buffer) {
> + assert(PredefinesFileID.isInvalid() && "PredefinesFileID already set!");
> + PredefinesFileID = createFileIDForMemBuffer(Buffer);
> + return PredefinesFileID;
> + }
>
> //===--------------------------------------------------------------------===//
> // MainFileID creation and querying methods.
> @@ -636,6 +647,9 @@
> /// getMainFileID - Returns the FileID of the main source file.
> FileID getMainFileID() const { return MainFileID; }
>
> + /// \brief Returns the FileID of the preprocessor predefines buffer.
> + FileID getPredefinesFileID() const { return PredefinesFileID; }
> +
> /// createMainFileID - Create the FileID for the main source file.
> FileID createMainFileID(const FileEntry *SourceFile,
> SrcMgr::CharacteristicKind Kind = SrcMgr::C_User) {
> @@ -1113,6 +1127,12 @@
> return getFileID(Loc) == getMainFileID();
> }
>
> + /// isFromPredefines - Returns true if the provided SourceLocation is
> + /// within the processor's predefines buffer.
> + bool isFromPredefines(SourceLocation Loc) const {
> + return getFileID(Loc) == getPredefinesFileID();
> + }
> +
> /// isInSystemHeader - Returns if a SourceLocation is in a system header.
> bool isInSystemHeader(SourceLocation Loc) const {
> return getFileCharacteristic(Loc) != SrcMgr::C_User;
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Wed Jun 6 12:25:21 2012
> @@ -730,6 +730,9 @@
> public:
> DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID);
> DiagnosticBuilder Diag(const Token &Tok, unsigned DiagID);
> + DiagnosticBuilder Diag(unsigned DiagID) {
> + return Diag(Tok, DiagID);
> + }
>
> private:
> void SuggestParentheses(SourceLocation Loc, unsigned DK,
>
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Wed Jun 6 12:25:21 2012
> @@ -407,6 +407,7 @@
>
> void SourceManager::clearIDTables() {
> MainFileID = FileID();
> + PredefinesFileID = FileID();
> LocalSLocEntryTable.clear();
> LoadedSLocEntryTable.clear();
> SLocEntryLoaded.clear();
>
> Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
> +++ cfe/trunk/lib/Lex/Preprocessor.cpp Wed Jun 6 12:25:21 2012
> @@ -425,7 +425,7 @@
> llvm::MemoryBuffer *SB =
> llvm::MemoryBuffer::getMemBufferCopy(Predefines, "<built-in>");
> assert(SB && "Cannot create predefined source buffer");
> - FileID FID = SourceMgr.createFileIDForMemBuffer(SB);
> + FileID FID = SourceMgr.createPredefinesFileIDForMemBuffer(SB);
> assert(!FID.isInvalid() && "Could not create FileID for predefines?");
>
> // Start parsing the predefines.
>
> Modified: cfe/trunk/lib/Parse/ParseAST.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseAST.cpp?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseAST.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseAST.cpp Wed Jun 6 12:25:21 2012
> @@ -12,6 +12,7 @@
> //===----------------------------------------------------------------------===//
>
> #include "clang/Parse/ParseAST.h"
> +#include "clang/Parse/ParseDiagnostic.h"
> #include "clang/Sema/Sema.h"
> #include "clang/Sema/CodeCompleteConsumer.h"
> #include "clang/Sema/SemaConsumer.h"
> @@ -77,27 +78,50 @@
> S.getPreprocessor().EnterMainSourceFile();
> P.Initialize();
> S.Initialize();
> -
> - if (ExternalASTSource *External = S.getASTContext().getExternalSource())
> +
> + // C11 6.9p1 says translation units must have at least one top-level
> + // declaration. C++ doesn't have this restriction. We also don't want to
> + // complain if we have a precompiled header, although technically if the PCH
> + // is empty we should still emit the (pedantic) diagnostic.
> + bool WarnForEmptyTU = !S.getLangOpts().CPlusPlus;
> + if (ExternalASTSource *External = S.getASTContext().getExternalSource()) {
> External->StartTranslationUnit(Consumer);
> -
> - bool Abort = false;
> + WarnForEmptyTU = false;
> + }
> +
> + // Clang's predefines contain top-level declarations for things like va_list,
> + // making it hard to tell if the /user's/ translation unit has at least one
> + // top-level declaration. So we parse cautiously, looking for a declaration
> + // that doesn't come from our predefines.
> + // Note that ParseTopLevelDecl returns 'true' at EOF.
> + SourceManager &SM = S.getSourceManager();
> Parser::DeclGroupPtrTy ADecl;
> -
> - while (!P.ParseTopLevelDecl(ADecl)) { // Not end of file.
> - // If we got a null return and something *was* parsed, ignore it. This
> - // is due to a top-level semicolon, an action override, or a parse error
> - // skipping something.
> + while (WarnForEmptyTU && !P.ParseTopLevelDecl(ADecl)) {
> if (ADecl) {
> - if (!Consumer->HandleTopLevelDecl(ADecl.get())) {
> - Abort = true;
> - break;
> + if (!Consumer->HandleTopLevelDecl(ADecl.get()))
> + return;
> + if (DeclGroupRef::iterator FirstDecl = ADecl.get().begin()) {
> + SourceLocation DeclLoc = (*FirstDecl)->getLocation();
> + WarnForEmptyTU = SM.isFromPredefines(DeclLoc);
> }
> }
> - };
> + }
>
> - if (Abort)
> - return;
> + // If we ended up seeing EOF before any top-level declarations, emit our
> + // diagnostic. Otherwise, parse the rest of the file normally.
> + if (WarnForEmptyTU) {
> + P.Diag(diag::ext_empty_translation_unit);
> + } else {
> + while (!P.ParseTopLevelDecl(ADecl)) { // Not end of file.
> + // If we got a null return and something *was* parsed, ignore it. This
> + // is due to a top-level semicolon, an action override, or a parse error
> + // skipping something.
> + if (ADecl) {
> + if (!Consumer->HandleTopLevelDecl(ADecl.get()))
> + return;
> + }
> + };
> + }
>
> // Process any TopLevelDecls generated by #pragma weak.
> for (SmallVector<Decl*,2>::iterator
>
> Modified: cfe/trunk/lib/Parse/Parser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/Parser.cpp (original)
> +++ cfe/trunk/lib/Parse/Parser.cpp Wed Jun 6 12:25:21 2012
> @@ -439,10 +439,6 @@
> // Prime the lexer look-ahead.
> ConsumeToken();
>
> - if (Tok.is(tok::eof) &&
> - !getLangOpts().CPlusPlus) // Empty source file is an extension in C
> - Diag(Tok, diag::ext_empty_source_file);
> -
> // Initialization for Objective-C context sensitive keywords recognition.
> // Referenced in Parser::ParseObjCTypeQualifierList.
> if (getLangOpts().ObjC1) {
>
> Modified: cfe/trunk/test/Misc/warning-flags.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/test/Misc/warning-flags.c (original)
> +++ cfe/trunk/test/Misc/warning-flags.c Wed Jun 6 12:25:21 2012
> @@ -17,7 +17,7 @@
>
> The list of warnings below should NEVER grow. It should gradually shrink to 0.
>
> -CHECK: Warnings without flags (242):
> +CHECK: Warnings without flags (241):
> CHECK-NEXT: ext_anonymous_struct_union_qualified
> CHECK-NEXT: ext_binary_literal
> CHECK-NEXT: ext_cast_fn_obj
> @@ -26,7 +26,6 @@
> CHECK-NEXT: ext_duplicate_declspec
> CHECK-NEXT: ext_ellipsis_exception_spec
> CHECK-NEXT: ext_empty_fnmacro_arg
> -CHECK-NEXT: ext_empty_source_file
> CHECK-NEXT: ext_enum_friend
> CHECK-NEXT: ext_enum_value_not_int
> CHECK-NEXT: ext_enumerator_list_comma
>
> Added: cfe/trunk/test/PCH/empty-with-headers.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/empty-with-headers.c?rev=158085&view=auto
> ==============================================================================
> --- cfe/trunk/test/PCH/empty-with-headers.c (added)
> +++ cfe/trunk/test/PCH/empty-with-headers.c Wed Jun 6 12:25:21 2012
> @@ -0,0 +1,27 @@
> +// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic-errors %s
> +// RUN: %clang_cc1 -fsyntax-only -std=c99 -emit-pch -o %t %s
> +// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic-errors -include-pch %t %s
> +
> +// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic-errors -DINCLUDED %s -verify
> +// This last one should warn for -Wempty-translation-unit (C99 6.9p1).
> +
> +#if defined(INCLUDED)
> +
> +// empty except for the prefix header
> +
> +#elif defined(HEADER)
> +
> +typedef int my_int;
> +#define INCLUDED
> +
> +#else
> +
> +#define HEADER
> +#include "empty-with-headers.c"
> +// empty except for the header
> +
> +#endif
> +
> +// This should only fire if the header is not included,
> +// either explicitly or as a prefix header.
> +// expected-error{{ISO C requires a translation unit to contain at least one declaration.}}
>
> Added: cfe/trunk/test/Parser/completely-empty-header-file.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/completely-empty-header-file.h?rev=158085&view=auto
> ==============================================================================
> (empty)
>
> Added: cfe/trunk/test/Parser/empty-translation-unit.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/empty-translation-unit.c?rev=158085&view=auto
> ==============================================================================
> --- cfe/trunk/test/Parser/empty-translation-unit.c (added)
> +++ cfe/trunk/test/Parser/empty-translation-unit.c Wed Jun 6 12:25:21 2012
> @@ -0,0 +1,10 @@
> +// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -W -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -x c++ -std=c++03 -pedantic-errors -W %s
> +
> +#include "completely-empty-header-file.h"
> +// no-warning -- an empty file is OK
> +
> +#define A_MACRO_IS_NOT_GOOD_ENOUGH 1
> +
> +// In C we should get this warning, but in C++ we shouldn't.
> +// expected-warning{{ISO C requires a translation unit to contain at least one declaration.}}
>
> Modified: cfe/trunk/test/Parser/opencl-pragma.cl
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/opencl-pragma.cl?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/test/Parser/opencl-pragma.cl (original)
> +++ cfe/trunk/test/Parser/opencl-pragma.cl Wed Jun 6 12:25:21 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
> +// RUN: %clang_cc1 %s -verify -pedantic -Wno-empty-translation-unit -fsyntax-only
>
> #pragma OPENCL EXTENSION cl_khr_fp16 : enable
>
>
> Modified: cfe/trunk/test/Preprocessor/undef-error.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/undef-error.c?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/test/Preprocessor/undef-error.c (original)
> +++ cfe/trunk/test/Preprocessor/undef-error.c Wed Jun 6 12:25:21 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 %s -pedantic-errors -verify
> +// RUN: %clang_cc1 %s -pedantic-errors -Wno-empty-translation-unit -verify
> // PR2045
>
> #define b
>
> Modified: cfe/trunk/test/Sema/c89-2.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c89-2.c?rev=158085&r1=158084&r2=158085&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/c89-2.c (original)
> +++ cfe/trunk/test/Sema/c89-2.c Wed Jun 6 12:25:21 2012
> @@ -1,4 +1,4 @@
> -/* RUN: %clang_cc1 %s -std=c89 -pedantic-errors -verify
> +/* RUN: %clang_cc1 %s -std=c89 -pedantic-errors -Wno-empty-translation-unit -verify
> */
>
> #if 1LL /* expected-error {{long long}} */
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
# Meador
-------------- next part --------------
A non-text attachment was scrubbed...
Name: va-list-builtin.patch
Type: application/octet-stream
Size: 28152 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120606/583b80a1/attachment.obj>
More information about the cfe-commits
mailing list