On Tue, May 16, 2017 at 12:19 PM Richard Smith <rich...@metafoo.co.uk> wrote:
> On 16 May 2017 at 11:54, Vitaly Buka <vitalyb...@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@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@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@lists.llvm.org >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits