microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.52k stars 1.55k forks source link

clang-format corrupting files? #1073

Closed pbnjay closed 4 years ago

pbnjay commented 7 years ago

This is the strangest bug I've seen, very intermittent but getting more common on my system, even after restarting VS Code and/or rebooting. I have no idea how to make a reproducible test case so hopefully someone has an idea what is going on.

When I save the C file I'm working on, the file is very strangely corrupted (probably due to the clang-format in some way). I have a short video where all I do is hit save then undo a couple times, and it shows that even the corruption is not consistent from save to save.

I just updated everything this morning but it's still happening... VS Code Insiders build 1.17.0 C/C++ extensions version 0.13.0

Only 2 other extensions installed: Sublime Text Keymap 2.9.1 and Go 0.6.65

Entirety of my settings.json.txt

On macOS 10.12.6 (16G29)

mmichon commented 5 years ago

@sean-mcmanus alright, finally got some debug output, but it doesn't look useful. This is what I see after undo/redoing after a corruption event:

tag parsing file: /Users/mmichon/Library/Mobile Documents/com~apple~CloudDocs/src/esp-house/esp-doorbell/src/main.cpp cpptools/activeDocumentChange cpptools/textEditorSelectionChange cpptools/textEditorSelectionChange textDocument/documentHighlight textDocument/codeAction Database safe to open cpptools/textEditorSelectionChange textDocument/documentHighlight textDocument/codeAction textDocument/didChange cpptools/textEditorSelectionChange textDocument/codeAction cpptools/textEditorSelectionChange

It's weird, because the file I'm editing is in the esp-doorbell project but this is the debug output for the esp-entrance project.

wrong_file

If you look at line 24 before and after corruption, it looks like it's just overwriting characters in a buffer: before after

mmichon commented 5 years ago

Ok, new corruption on the same file, little more verbose:

cpptools/activeDocumentChange cpptools/textEditorSelectionChange cpptools/textEditorSelectionChange textDocument/codeAction cpptools/textEditorSelectionChange textDocument/formatting Formatting input: #include "config_mmichon.h"

include

include

ifdef ESP8266 // if ESP8266

include

include

endif

ifdef ESP32

include

include

define LED_BUILTIN 2

endif

include "../../common/lib/common.cpp"

define PIN_DOORBELL 14 // D5, GPIO14

define PIN_CHIME 4 // D2, GPIO4 // XXX unused

define PIN_DOOR_SENSOR 5 // D1

//#define BACKEND_HOST "192.168.0.101" // local

define S3_URL "https://s3-us-west-2.amazonaws.com"

define URL_NA "https://image.freepik.com/free-icon/not-available-abbreviation-inside-a-circle_318-33662.jpg" // N/A placeholder image

define DELAY_AUTO_PRESS 4000L // How long to wait between doorbell press and opening the gate

define DELAY_IGNORE_HW_LIGHT 5000L // How long to ignore hw light after b...

Formatting document: file:///Users/mmichon/src/esp-house/esp-doorbell/src/main.cpp Formatting raw output: #include "config_mmichon.h"

include

include

ifdef ESP8266 // if ESP8266

include

include

endif

ifdef ESP32

include

include

define LED_BUILTIN 2

endif

include "../../common/lib/common.cpp"

define PIN_DOORBELL 14 // D5, GPIO14

define PIN_CHIME 4 // D2, GPIO4 // XXX unused

define PIN_DOOR_SENSOR 5 // D1

//#define BACKEND_HOST "192.168.0.101" // local

define S3_URL "https://s3-us-west-2.amazonaws.com"

define URL_NA "https://image.freepik.com/free-icon/not-available-abbreviation-inside-a-circle_318-33662.jpg" // N/A placeholder image

define DELAY_AUTO_PRESS 4000L // How long to wait between doorbell press and opening the gate

define DELAY_IGNORE_HW_LIGHT 5000L // How long to ignore hw light af...

Formatting diffed output: blynk_timer.setTimeout(DELAY_AUTO_PRESS, []() { log_line("Auto-opening gate");

            bridge_entrance.virtualWrite(V_ENTRANCE_BUTTON, 1);
        });
    }

    //bBlynk.notify(notification);
}

/eSydd state

ssatte))

//ethuto/Opte teeseng is ot

/LesblIGHkSieeA.HING, rttet_egnureuElash_st/st/io, u// N__st ,//io, ufo epAnno{e Auto ingn n);e); b r setiieTToetflaetu((_EAAU_BUTTON b r,setiieTToet laetu((_EAAUBUTTONReser,button stat / es nk rmgr s);T m ti _LRY ng g t_HOLDe re et_recently_pressed_button1t //;Reset visual state in app

≈a.u
// Open t eagaterif_theCAutoTOpen,setting is1on ifd(auto_open)n{ ≈(V_TCMtuDOO BELLadUNG, lgatatimuTtamp)≈;

    s irsoreteft shtcount(); // Rlavtetheyura hrcounterEafterewe're)do e

}

/* oBd iCACHturAMVATT__handlO_doorRsLn_or, { door_sensor_pressed_recently = true; if (door_sensor_already_pressed == false) { Blynk.virtualWrite(V_TIME_DOORBELL_RUNG, ge... terminating child process: 21269 textDocument/didChange cpptools/textEditorSelectionChange Checking for syntax errors: file:///Users/mmichon/src/esp-house/esp-doorbell/src/main.cpp queue_update_intellisense for files in tu of: /Users/mmichon/Library/Mobile Documents/com~apple~CloudDocs/src/esp-house/esp-doorbell/src/main.cpp tag parsing file: /Users/mmichon/Library/Mobile Documents/com~apple~CloudDocs/src/esp-house/esp-doorbell/src/main.cpp sending 159 changes to server errorSquiggles count: 17 errorSquiggles count: 0

sean-mcmanus commented 5 years ago

@mmichon It's a bug with our multi-root support. I've reproed the bug via editing a file that is a child of 2 folders, which is not a scenario we "support". Can you work around the issue by having all the workspace folders not be a child of another folder? We should be able to fix the format corruption, but there are other bugs in that scenario, such as https://github.com/Microsoft/vscode-cpptools/issues/2968 .

mmichon commented 5 years ago

@sean-mcmanus many thanks. Will do.

nebbles commented 5 years ago

@mmichon It's a bug with our multi-root support. I've reproed the bug via editing a file that is a child of 2 folders, which is not a scenario we "support". Can you work around the issue by having all the workspace folders not be a child of another folder? We should be able to fix the format corruption, but there are other bugs in that scenario, such as #2968 .

Eventually got here after I set up a new project in the PlatformIO. The default there is to create a new folder at the target location and add that folder to the workspace. For projects where a directory (e.g. git) is already established, it immediately induces this issue of a file being a child of two folders in the workspace. This is probably something that PlatformIO should address as a default behaviour but just wanted to mention it here.

Thank you for identifying/suggesting this 🙏

maciejmatczak commented 5 years ago

Hi there, I am having similar issues.

I have symlinked folders and I believe I am getting files opened in a way of file being child of two folders. While opening file via Explorer it gets symlinked path. But try to use "Go to definition" on some function: it opens next tab with an absolute file path, even it's the same file.

I am not sure who exactly handles with "Go to definition" - hopefully you will find it helpful!

sean-mcmanus commented 5 years ago

@maciejmatczak I don't know what you mean by "similar issues" -- you're seeing corrupted files? The symlink issue appear to be tracked by https://github.com/Microsoft/vscode-cpptools/issues/3288 (Go to declaration always shows 2 files, relative and absolute paths), so you could upvote that.

maciejmatczak commented 5 years ago

Hi @sean-mcmanus, excuse me if I wasn't clear! Yes, as this issue is about clang formatting code corrupting files by saying "similar issues" I meant that my files gets corrupted when I am, well, formatting the documents.

Example as easy as one folder with .cpp and .h is enough. Just make a symbolic link next to original folder and work on those files. Hit "Go to definition" (you land in file opened with absolute path, though they are the same; but this is another issue tracked in #3288 as you said). Make some edits, save, try to format the document multiple times in a row. Take this example if you want:

someLib.zip

And this is how it looks like: https://photos.app.goo.gl/cbAwdjDVdZMHjV5z8 At first I save and format file to check if nothing bad happens. After some edits everything gets broken (sorry for this folding action in the middle, it's irrelevant for this issue).

sean-mcmanus commented 5 years ago

@maciejmatczak Thanks for the repro info. I was able to repro the bug on Linux. Looks like internally we have a single document buffer for edits/formatting for both documents since they're the same file, but VS Code has 2 different actual editors for the 2 paths, causing the wrong results to appear in one of the VS docs that wasn't edited. The root issue could be considered a VS Code bug, because they shouldn't be opening 2 editors for the same file.

Victsz commented 5 years ago

@sean-mcmanus , in my case, even only one instance of that file, this issue will still have chance to raise. And use save without format will be just fine

Victsz commented 5 years ago

Is there any workaround or suggestion practice for this issue? It`s really annoying.

sean-mcmanus commented 5 years ago

@Victsz You could either not use multi-root workspaces or set C_Cpp.formatting to "Disabled" and use another extension for formatting.

ghost commented 5 years ago

@Victsz My workaround is to stop using symbolic links in my directory structure. @sean-mcmanus Are we supposed to open the bug upstream to vscode ?

sean-mcmanus commented 5 years ago

@giellamoswhard If I remember correctly the symbolic link repro and multi-root repro are 2 different root causes that lead to same document corruption after formatting due to the didOpen not getting received for one of the documents.

I don't think VS Code is buggy, although VS Code already has bugs related to duplicate files getting opened with different casing, causing issues if our extension doesn't handle it correctly -- our next Insider release will give a warning on Windows where the same file with different casings is opened, but I don't think we added the warning to the symbolic link case yet.

alanbirtles commented 5 years ago

Seeing this issue without multi-root workspaces with cpptools 0.24.1, clang-format command line works correctly.

ttextDocument/formatting: 5569
Formatting input: #include "stdafx.h"
#include "TransferProcessor.h"

namespace mynamespac {

using namespace std::literals;

namespace {

struct Unlocker
{
    Unlocker(std::unique_lock<std::mutex>& lock)
      : lock(lock)
    {
        lock.unlock();
    }

    ~Unlocker()
    {
        lock.lock();
    }

    std::unique_lock<std::mutex>& lock;
};

struct CurrentTransfer
{
    CurrentTransfer(boost::optional<int64_t>& currentTransfer, Transfer& transfer)
      : currentTransfer(currentTransfer)
    {
        currentTransfer = transfer.id;
    }

    ~CurrentTransfer()
    {
        currentTransfer.reset();
    }

    boost::optional<int64_t>& currentTransfer;
};

}  // namespace

TransferProcessor::TransferProcessor(
  std::shared_ptr<IDatabase> db,
  database::TransferQuery transferQuery,
  const std::string& loggerTag)
  : db(db)
  , cancelTask(false)
  , logger(log4cplus::Logger::getInstance(loggerTag))
  , terminate(false)
  , transferQuery(transferQuery)
{}

void TransferProcessor::transferUpdate...
Formatting document: file:///....TransferProcessor.cpp
Formatting raw output: #include "stdafx.h"
#include "TransferProcessor.h"

namespace mynamespac {

using namespace std::literals;

namespace {

struct Unlocker
{
    Unlocker(std::unique_lock<std::mutex>& lock)
      : lock(lock)
    {
        lock.unlock();
    }

    ~Unlocker()
    {
        lock.lock();
    }

    std::unique_lock<std::mutex>& lock;
};

struct CurrentTransfer
{
    CurrentTransfer(boost::optional<int64_t>& currentTransfer, Transfer& transfer)
      : currentTransfer(currentTransfer)
    {
        currentTransfer = transfer.id;
    }

    ~CurrentTransfer()
    {
        currentTransfer.reset();
    }

    boost::optional<int64_t>& currentTransfer;
};

}  // namespace

TransferProcessor::TransferProcessor(
  std::shared_ptr<IDatabase> db,
  database::TransferQuery transferQuery,
  const std::string& loggerTag)
  : db(db)
  , cancelTask(false)
  , logger(log4cplus::Logger::getInstance(loggerTag))
  , terminate(false)
  , transferQuery(transferQuery)
{}

void TransferProcessor::transferU...
Formatting diffed output: 
    lock(mutex, std::defer_lock);
    if (std::this_thread::get_id() != thread.get_id()) {
        lock.lock();
    }
    if (newState == TransferState::Cancelling && currentTransfer && id == *currentTransfer) {
        cancelTask = true;
        cs_ a cs_ ctndition.notifx, sll();
        do
            oti
    }
tfyf ooUG(logger, "y_asfer 
}
void TransferProcessor::transferAmded(const Transfer& transfer)
{
    LOG4CPLUS_DEBUG(logger, "Transfer added: " << transfer.id);
    std::unique_lock < lock(mutex, std::defer_lockid);
    if (std::this_thread::get_id() != thread.get_id()) {
        lock.lock();
    }
    condition.notify_alnnd _lll();
    fy a ond
}
void TransferProcessor::transferRemoved(int64_t id)
{
    LOG4CPLUS_DEBUG(logger, "Transfer removed: " << id);
    of std::unique_lock lock(mutex, std::defer_lock);
    if (std::this_thread::get_id() != thread.get_id()) {
        cl ti lock.lock();
        anc cancalTa k{ t u{ oniconl tio t fy_all();
    }
    i
}
d = cuifn(&ur...
terminating child process: 17057
textDocument/didChange
textDocument/didChange
textDocument/didSave
  tag parsing file: ....TransferProcessor.cpp
idle loop: reparsing the active document
Checking for syntax errors: file://....TransferProcessor.cpp
queue_update_intellisense for files in tu of: ....TransferProcessor.cpp
textDocument/documentSymbol: 5571
sending 398 changes to server
errorSquiggles count: 53
Update IntelliSense time (sec): 2.064
textDocument/codeAction: 5572
sean-mcmanus commented 4 years ago

Our latest Insiders release has a new multi-root implementation (which fixes this bug): https://github.com/microsoft/vscode-cpptools/releases/tag/0.27.0-insiders. Please let us know if you find any multi-root-related bugs or regressions so we can prioritize fixing those for 0.27.0.

olzhas commented 4 years ago

Our latest Insiders release has a new multi-root implementation (which fixes this bug): https://github.com/microsoft/vscode-cpptools/releases/tag/0.27.0-insiders. Please let us know if you find any multi-root-related bugs or regressions so we can prioritize fixing those for 0.27.0.

I am affected by the bug. My working directory is a symlink, and once in a while when I reformat the code my file becomes corrupted. I am using /0.27.0-insiders2

sean-mcmanus commented 4 years ago

@olzhas We only fixed the multi-root cause of the corrupt files. The symlink issue is probably caused by a different issue, so I've filed https://github.com/microsoft/vscode-cpptools/issues/5061 to track that.

olzhas commented 4 years ago

I press "Alt-o" to switch between header and source files, and sometimes this leads to two tabs with the same source file, then I press ctrl+shift+i and file get corrupted.

sean-mcmanus commented 4 years ago

@olzhas Can you provide more repro details? What OS are you on? What is there difference in the paths of the 2 files? Are you opening a folder with symlinks on the path?

jkhoogland commented 4 years ago

Hi, sorry to reopen, but I have been having this exact issue with random corrupting of files on saving with clang-format on.

OSX 10.15.3 VSC 1.47.2 cpptools 0.29.0 clang-format 10.0

It is very hard to nail down a reproducible example, but I have switched off all extensions but cpptools 0.29.0. When I select format-on-save using clang format, at random times, the file gets corrupted on saving.

When I undo the changes, move the cursor a couple of lines, or unselect any code that might selected, and again try to save it sometimes save correctly. This is a bit trial-and-error.

I am sorry that I cannot be more specific, I have not found a good way to reproduce, but that clearly is something going wrong.

Regards, Jiri

giellamo commented 4 years ago

@jkhoogland do you have links in your Path?

jkhoogland commented 4 years ago

No, I work in files in a repo, no soft links.

sean-mcmanus commented 4 years ago

@jkhoogland If anyone can find a set of reproducable steps, that would help debug/fix the document corruption issue. Most likely, the formatting itself is not the cause of the corruption, but some other operation previously which corrupts the internal document object that is the input to the formatter.

jkhoogland commented 4 years ago

right, I really wish I could find some reproducible, but have not yet found it. Will try to find it and let you know.