r302966 - Remove unused tracking of owning module for MacroInfo objects.
Vitaly Buka via cfe-commits
cfe-commits at lists.llvm.org
Wed May 17 13:43:21 PDT 2017
On Tue, May 16, 2017 at 12:19 PM Richard Smith <richard at metafoo.co.uk>
wrote:
> On 16 May 2017 at 11:54, Vitaly Buka <vitalybuka at google.com> wrote:
>
>> The patch breaks this test
>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1349/steps/check-clang%20msan/logs/stdio
>>
>
> Given the nature of this change, that's a surprise, but anything's
> possible. How sure are you that it was this change?
>
>
I bisected to the change locally.
> Script:
>> --
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/./bin/clang
>> -cc1 -internal-isystem
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/lib/clang/5.0.0/include
>> -nostdsysteminc -verify -fms-extensions -Wmicrosoft
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c
>> not
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/./bin/clang
>> -cc1 -internal-isystem
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/lib/clang/5.0.0/include
>> -nostdsysteminc -P -E -fms-extensions
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c
>> |
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/./bin/FileCheck
>> -strict-whitespace
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c
>> --
>> Exit Code: 1
>>
>> Command Output (stderr):
>> --
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:35:1:
>> error: pasting formed '(baz', an invalid preprocessing token
>> bar(q) // expected-warning {{type specifier missing}} expected-error
>> {{invalid preprocessing token}} expected-error {{parameter list without
>> types}}
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:34:20:
>> note: expanded from macro 'bar'
>> #define bar(y) foo(##baz(y))
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:42:1:
>> error: pasting formed '1a-', an invalid preprocessing token
>> collapse_spaces(1a, b2, 3c, d4) // expected-error 4 {{invalid
>> preprocessing token}} expected-error {{expected function body}}
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:41:43:
>> note: expanded from macro 'collapse_spaces'
>> #define collapse_spaces(a, b, c, d) str(a ## - ## b ## - ## c ## d)
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:42:1:
>> error: pasting formed '-b2', an invalid preprocessing token
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:41:48:
>> note: expanded from macro 'collapse_spaces'
>> #define collapse_spaces(a, b, c, d) str(a ## - ## b ## - ## c ## d)
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:42:1:
>> error: pasting formed 'b2-', an invalid preprocessing token
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:41:53:
>> note: expanded from macro 'collapse_spaces'
>> #define collapse_spaces(a, b, c, d) str(a ## - ## b ## - ## c ## d)
>> ^
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:42:1:
>> error: pasting formed '-3c', an invalid preprocessing token
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:41:58:
>> note: expanded from macro 'collapse_spaces'
>> #define collapse_spaces(a, b, c, d) str(a ## - ## b ## - ## c ## d)
>> ^
>> 5 errors generated.
>> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Preprocessor/macro_paste_msextensions.c:44:11:
>> error: expected string not found in input
>> // CHECK: "1a-b2-3cd4"
>> ^
>> <stdin>:34:1: note: scanning from here
>> "1a-b2- 3cd4"
>> ^
>>
>>
>> On Mon, May 15, 2017 at 10:28 AM Jordan Rose via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Hi, Richard. Swift was using this information in order to put imported
>>> macros in a particular context. It wouldn't surprise me to hear that we
>>> were doing it wrong, and that there's a better way to go from a macro back
>>> to a module, but is there a recommended replacement?
>>>
>>> Thanks,
>>> Jordan
>>>
>>>
>>> > On May 12, 2017, at 16:40, Richard Smith via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>> >
>>> > Author: rsmith
>>> > Date: Fri May 12 18:40:52 2017
>>> > New Revision: 302966
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=302966&view=rev
>>> > Log:
>>> > Remove unused tracking of owning module for MacroInfo objects.
>>> >
>>> > Modified:
>>> > cfe/trunk/include/clang/Lex/MacroInfo.h
>>> > cfe/trunk/include/clang/Lex/Preprocessor.h
>>> > cfe/trunk/lib/Lex/MacroInfo.cpp
>>> > cfe/trunk/lib/Lex/PPDirectives.cpp
>>> > cfe/trunk/lib/Lex/Preprocessor.cpp
>>> > cfe/trunk/lib/Serialization/ASTReader.cpp
>>> > cfe/trunk/lib/Serialization/ASTWriter.cpp
>>> >
>>> > Modified: cfe/trunk/include/clang/Lex/MacroInfo.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/include/clang/Lex/MacroInfo.h (original)
>>> > +++ cfe/trunk/include/clang/Lex/MacroInfo.h Fri May 12 18:40:52 2017
>>> > @@ -105,9 +105,6 @@ class MacroInfo {
>>> > /// \brief Must warn if the macro is unused at the end of
>>> translation unit.
>>> > bool IsWarnIfUnused : 1;
>>> >
>>> > - /// \brief Whether this macro info was loaded from an AST file.
>>> > - bool FromASTFile : 1;
>>> > -
>>> > /// \brief Whether this macro was used as header guard.
>>> > bool UsedForHeaderGuard : 1;
>>> >
>>> > @@ -264,34 +261,16 @@ public:
>>> > IsDisabled = true;
>>> > }
>>> >
>>> > - /// \brief Determine whether this macro info came from an AST file
>>> (such as
>>> > - /// a precompiled header or module) rather than having been parsed.
>>> > - bool isFromASTFile() const { return FromASTFile; }
>>> > -
>>> > /// \brief Determine whether this macro was used for a header guard.
>>> > bool isUsedForHeaderGuard() const { return UsedForHeaderGuard; }
>>> >
>>> > void setUsedForHeaderGuard(bool Val) { UsedForHeaderGuard = Val; }
>>> >
>>> > - /// \brief Retrieve the global ID of the module that owns this
>>> particular
>>> > - /// macro info.
>>> > - unsigned getOwningModuleID() const {
>>> > - if (isFromASTFile())
>>> > - return *(const unsigned *)(this + 1);
>>> > -
>>> > - return 0;
>>> > - }
>>> > -
>>> > void dump() const;
>>> >
>>> > private:
>>> > unsigned getDefinitionLengthSlow(const SourceManager &SM) const;
>>> >
>>> > - void setOwningModuleID(unsigned ID) {
>>> > - assert(isFromASTFile());
>>> > - *(unsigned *)(this + 1) = ID;
>>> > - }
>>> > -
>>> > friend class Preprocessor;
>>> > };
>>> >
>>> >
>>> > Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
>>> > +++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri May 12 18:40:52 2017
>>> > @@ -644,14 +644,6 @@ class Preprocessor {
>>> > /// of that list.
>>> > MacroInfoChain *MIChainHead;
>>> >
>>> > - struct DeserializedMacroInfoChain {
>>> > - MacroInfo MI;
>>> > - unsigned OwningModuleID; // MUST be immediately after the
>>> MacroInfo object
>>> > - // so it can be accessed by
>>> MacroInfo::getOwningModuleID().
>>> > - DeserializedMacroInfoChain *Next;
>>> > - };
>>> > - DeserializedMacroInfoChain *DeserialMIChainHead;
>>> > -
>>> > void updateOutOfDateIdentifier(IdentifierInfo &II) const;
>>> >
>>> > public:
>>> > @@ -1669,10 +1661,6 @@ public:
>>> > /// \brief Allocate a new MacroInfo object with the provided
>>> SourceLocation.
>>> > MacroInfo *AllocateMacroInfo(SourceLocation L);
>>> >
>>> > - /// \brief Allocate a new MacroInfo object loaded from an AST file.
>>> > - MacroInfo *AllocateDeserializedMacroInfo(SourceLocation L,
>>> > - unsigned SubModuleID);
>>> > -
>>> > /// \brief Turn the specified lexer token into a fully checked and
>>> spelled
>>> > /// filename, e.g. as an operand of \#include.
>>> > ///
>>> > @@ -1764,9 +1752,6 @@ private:
>>> > /// macro name.
>>> > void updateModuleMacroInfo(const IdentifierInfo *II, ModuleMacroInfo
>>> &Info);
>>> >
>>> > - /// \brief Allocate a new MacroInfo object.
>>> > - MacroInfo *AllocateMacroInfo();
>>> > -
>>> > DefMacroDirective *AllocateDefMacroDirective(MacroInfo *MI,
>>> > SourceLocation Loc);
>>> > UndefMacroDirective *AllocateUndefMacroDirective(SourceLocation
>>> UndefLoc);
>>> >
>>> > Modified: cfe/trunk/lib/Lex/MacroInfo.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/lib/Lex/MacroInfo.cpp (original)
>>> > +++ cfe/trunk/lib/Lex/MacroInfo.cpp Fri May 12 18:40:52 2017
>>> > @@ -29,7 +29,6 @@ MacroInfo::MacroInfo(SourceLocation DefL
>>> > IsUsed(false),
>>> > IsAllowRedefinitionsWithoutWarning(false),
>>> > IsWarnIfUnused(false),
>>> > - FromASTFile(false),
>>> > UsedForHeaderGuard(false) {
>>> > }
>>> >
>>> > @@ -137,7 +136,6 @@ LLVM_DUMP_METHOD void MacroInfo::dump()
>>> > if (IsAllowRedefinitionsWithoutWarning)
>>> > Out << " allow_redefinitions_without_warning";
>>> > if (IsWarnIfUnused) Out << " warn_if_unused";
>>> > - if (FromASTFile) Out << " imported";
>>> > if (UsedForHeaderGuard) Out << " header_guard";
>>> >
>>> > Out << "\n #define <macro>";
>>> >
>>> > Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
>>> > +++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri May 12 18:40:52 2017
>>> > @@ -54,35 +54,12 @@ using namespace clang;
>>> > // Utility Methods for Preprocessor Directive Handling.
>>> >
>>> //===----------------------------------------------------------------------===//
>>> >
>>> > -MacroInfo *Preprocessor::AllocateMacroInfo() {
>>> > - MacroInfoChain *MIChain = BP.Allocate<MacroInfoChain>();
>>> > - MIChain->Next = MIChainHead;
>>> > +MacroInfo *Preprocessor::AllocateMacroInfo(SourceLocation L) {
>>> > + auto *MIChain = new (BP) MacroInfoChain{L, MIChainHead};
>>> > MIChainHead = MIChain;
>>> > return &MIChain->MI;
>>> > }
>>> >
>>> > -MacroInfo *Preprocessor::AllocateMacroInfo(SourceLocation L) {
>>> > - MacroInfo *MI = AllocateMacroInfo();
>>> > - new (MI) MacroInfo(L);
>>> > - return MI;
>>> > -}
>>> > -
>>> > -MacroInfo *Preprocessor::AllocateDeserializedMacroInfo(SourceLocation
>>> L,
>>> > - unsigned
>>> SubModuleID) {
>>> > - static_assert(alignof(MacroInfo) >= sizeof(SubModuleID),
>>> > - "alignment for MacroInfo is less than the ID");
>>> > - DeserializedMacroInfoChain *MIChain =
>>> > - BP.Allocate<DeserializedMacroInfoChain>();
>>> > - MIChain->Next = DeserialMIChainHead;
>>> > - DeserialMIChainHead = MIChain;
>>> > -
>>> > - MacroInfo *MI = &MIChain->MI;
>>> > - new (MI) MacroInfo(L);
>>> > - MI->FromASTFile = true;
>>> > - MI->setOwningModuleID(SubModuleID);
>>> > - return MI;
>>> > -}
>>> > -
>>> > DefMacroDirective *Preprocessor::AllocateDefMacroDirective(MacroInfo
>>> *MI,
>>> >
>>> SourceLocation Loc) {
>>> > return new (BP) DefMacroDirective(MI, Loc);
>>> >
>>> > Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
>>> > +++ cfe/trunk/lib/Lex/Preprocessor.cpp Fri May 12 18:40:52 2017
>>> > @@ -88,7 +88,7 @@ Preprocessor::Preprocessor(std::shared_p
>>> > CurDirLookup(nullptr), CurLexerKind(CLK_Lexer),
>>> > CurLexerSubmodule(nullptr), Callbacks(nullptr),
>>> > CurSubmoduleState(&NullSubmoduleState), MacroArgCache(nullptr),
>>> > - Record(nullptr), MIChainHead(nullptr),
>>> DeserialMIChainHead(nullptr) {
>>> > + Record(nullptr), MIChainHead(nullptr) {
>>> > OwnsHeaderSearch = OwnsHeaders;
>>> >
>>> > CounterValue = 0; // __COUNTER__ starts at 0.
>>> > @@ -169,11 +169,6 @@ Preprocessor::~Preprocessor() {
>>> > std::fill(TokenLexerCache, TokenLexerCache + NumCachedTokenLexers,
>>> nullptr);
>>> > CurTokenLexer.reset();
>>> >
>>> > - while (DeserializedMacroInfoChain *I = DeserialMIChainHead) {
>>> > - DeserialMIChainHead = I->Next;
>>> > - I->~DeserializedMacroInfoChain();
>>> > - }
>>> > -
>>> > // Free any cached MacroArgs.
>>> > for (MacroArgs *ArgList = MacroArgCache; ArgList;)
>>> > ArgList = ArgList->deallocate();
>>> >
>>> > Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
>>> > +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri May 12 18:40:52 2017
>>> > @@ -1534,9 +1534,8 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo
>>> > return Macro;
>>> >
>>> > unsigned NextIndex = 1; // Skip identifier ID.
>>> > - SubmoduleID SubModID = getGlobalSubmoduleID(F,
>>> Record[NextIndex++]);
>>> > SourceLocation Loc = ReadSourceLocation(F, Record, NextIndex);
>>> > - MacroInfo *MI = PP.AllocateDeserializedMacroInfo(Loc, SubModID);
>>> > + MacroInfo *MI = PP.AllocateMacroInfo(Loc);
>>> > MI->setDefinitionEndLoc(ReadSourceLocation(F, Record,
>>> NextIndex));
>>> > MI->setIsUsed(Record[NextIndex++]);
>>> > MI->setUsedForHeaderGuard(Record[NextIndex++]);
>>> >
>>> > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=302966&r1=302965&r2=302966&view=diff
>>> >
>>> ==============================================================================
>>> > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
>>> > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri May 12 18:40:52 2017
>>> > @@ -2413,7 +2413,6 @@ void ASTWriter::WritePreprocessor(const
>>> > }
>>> >
>>> > AddIdentifierRef(Name, Record);
>>> > -
>>> Record.push_back(inferSubmoduleIDFromLocation(MI->getDefinitionLoc()));
>>> > AddSourceLocation(MI->getDefinitionLoc(), Record);
>>> > AddSourceLocation(MI->getDefinitionEndLoc(), Record);
>>> > Record.push_back(MI->isUsed());
>>> >
>>> >
>>> > _______________________________________________
>>> > cfe-commits mailing list
>>> > cfe-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170517/b963a1f0/attachment-0001.html>
More information about the cfe-commits
mailing list